From 2e6a020e4a4e28db500c20bf2bfc185074b27ebf Mon Sep 17 00:00:00 2001 From: crazyhzm Date: Tue, 19 Dec 2023 14:11:56 +0800 Subject: [PATCH] Revert dubbo module refactor Signed-off-by: crazyhzm --- .../rpc/cluster/support/MockInvokerTest.java | 71 +++++++++---------- .../common/logger/log4j/Log4jLogger.java | 5 -- .../common/logger/log4j2/Log4j2Logger.java | 5 ++ dubbo-rpc/dubbo-rpc-api/pom.xml | 5 -- .../dubbo/rpc/filter/AccessLogFilterTest.java | 42 ++--------- 5 files changed, 45 insertions(+), 83 deletions(-) diff --git a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/MockInvokerTest.java b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/MockInvokerTest.java index cc752951e297..ae0509ffadc2 100644 --- a/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/MockInvokerTest.java +++ b/dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/support/MockInvokerTest.java @@ -36,29 +36,27 @@ class MockInvokerTest { @Test void testParseMockValue() throws Exception { - Assertions.assertNull(org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("null")); - Assertions.assertNull(org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("empty")); + Assertions.assertNull(MockInvoker.parseMockValue("null")); + Assertions.assertNull(MockInvoker.parseMockValue("empty")); - Assertions.assertTrue((Boolean) org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("true")); - Assertions.assertFalse((Boolean) org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("false")); + Assertions.assertTrue((Boolean) MockInvoker.parseMockValue("true")); + Assertions.assertFalse((Boolean) MockInvoker.parseMockValue("false")); - Assertions.assertEquals(123, org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("123")); - Assertions.assertEquals("foo", org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("foo")); - Assertions.assertEquals("foo", org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("\"foo\"")); - Assertions.assertEquals("foo", org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("\'foo\'")); + Assertions.assertEquals(123, MockInvoker.parseMockValue("123")); + Assertions.assertEquals("foo", MockInvoker.parseMockValue("foo")); + Assertions.assertEquals("foo", MockInvoker.parseMockValue("\"foo\"")); + Assertions.assertEquals("foo", MockInvoker.parseMockValue("\'foo\'")); - Assertions.assertEquals(new HashMap<>(), org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("{}")); - Assertions.assertEquals(new ArrayList<>(), org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("[]")); - Assertions.assertEquals( - "foo", org.apache.dubbo.rpc.support.MockInvoker.parseMockValue("foo", new Type[] {String.class})); + Assertions.assertEquals(new HashMap<>(), MockInvoker.parseMockValue("{}")); + Assertions.assertEquals(new ArrayList<>(), MockInvoker.parseMockValue("[]")); + Assertions.assertEquals("foo", MockInvoker.parseMockValue("foo", new Type[] {String.class})); } @Test void testInvoke() { URL url = URL.valueOf("remote://1.2.3.4/" + String.class.getName()); url = url.addParameter(MOCK_KEY, "return "); - org.apache.dubbo.rpc.support.MockInvoker mockInvoker = - new org.apache.dubbo.rpc.support.MockInvoker(url, String.class); + MockInvoker mockInvoker = new MockInvoker(url, String.class); RpcInvocation invocation = new RpcInvocation(); invocation.setMethodName("getSomething"); @@ -71,8 +69,7 @@ void testGetDefaultObject() { final Class demoServiceAClass = DemoServiceA.class; URL url = URL.valueOf("remote://1.2.3.4/" + demoServiceAClass.getName()); url = url.addParameter(MOCK_KEY, "force:true"); - org.apache.dubbo.rpc.support.MockInvoker mockInvoker = - new org.apache.dubbo.rpc.support.MockInvoker(url, demoServiceAClass); + MockInvoker mockInvoker = new MockInvoker(url, demoServiceAClass); RpcInvocation invocation = new RpcInvocation(); invocation.setMethodName("methodA"); @@ -82,7 +79,7 @@ void testGetDefaultObject() { final Class demoServiceBClass = DemoServiceB.class; url = URL.valueOf("remote://1.2.3.4/" + demoServiceBClass.getName()); url = url.addParameter(MOCK_KEY, "force:true"); - mockInvoker = new org.apache.dubbo.rpc.support.MockInvoker(url, demoServiceBClass); + mockInvoker = new MockInvoker(url, demoServiceBClass); invocation = new RpcInvocation(); invocation.setMethodName("methodB"); Assertions.assertEquals(new HashMap<>(), mockInvoker.invoke(invocation).getObjectAttachments()); @@ -91,7 +88,7 @@ void testGetDefaultObject() { @Test void testInvokeThrowsRpcException1() { URL url = URL.valueOf("remote://1.2.3.4/" + String.class.getName()); - org.apache.dubbo.rpc.support.MockInvoker mockInvoker = new org.apache.dubbo.rpc.support.MockInvoker(url, null); + MockInvoker mockInvoker = new MockInvoker(url, null); Assertions.assertThrows(RpcException.class, () -> mockInvoker.invoke(new RpcInvocation())); } @@ -100,8 +97,7 @@ void testInvokeThrowsRpcException1() { void testInvokeThrowsRpcException2() { URL url = URL.valueOf("remote://1.2.3.4/" + String.class.getName()); url = url.addParameter(MOCK_KEY, "fail"); - org.apache.dubbo.rpc.support.MockInvoker mockInvoker = - new org.apache.dubbo.rpc.support.MockInvoker(url, String.class); + MockInvoker mockInvoker = new MockInvoker(url, String.class); RpcInvocation invocation = new RpcInvocation(); invocation.setMethodName("getSomething"); @@ -112,8 +108,7 @@ void testInvokeThrowsRpcException2() { void testInvokeThrowsRpcException3() { URL url = URL.valueOf("remote://1.2.3.4/" + String.class.getName()); url = url.addParameter(MOCK_KEY, "throw"); - org.apache.dubbo.rpc.support.MockInvoker mockInvoker = - new org.apache.dubbo.rpc.support.MockInvoker(url, String.class); + MockInvoker mockInvoker = new MockInvoker(url, String.class); RpcInvocation invocation = new RpcInvocation(); invocation.setMethodName("getSomething"); @@ -130,24 +125,24 @@ void testGetThrowable() { void testGetMockObject() { Assertions.assertEquals( "", - org.apache.dubbo.rpc.support.MockInvoker.getMockObject( + MockInvoker.getMockObject( ApplicationModel.defaultModel().getExtensionDirector(), "java.lang.String", String.class)); Assertions.assertThrows( IllegalStateException.class, - () -> org.apache.dubbo.rpc.support.MockInvoker.getMockObject( + () -> MockInvoker.getMockObject( ApplicationModel.defaultModel().getExtensionDirector(), "true", String.class)); Assertions.assertThrows( IllegalStateException.class, - () -> org.apache.dubbo.rpc.support.MockInvoker.getMockObject( + () -> MockInvoker.getMockObject( ApplicationModel.defaultModel().getExtensionDirector(), "default", String.class)); Assertions.assertThrows( IllegalStateException.class, - () -> org.apache.dubbo.rpc.support.MockInvoker.getMockObject( + () -> MockInvoker.getMockObject( ApplicationModel.defaultModel().getExtensionDirector(), "java.lang.String", Integer.class)); Assertions.assertThrows( IllegalStateException.class, - () -> org.apache.dubbo.rpc.support.MockInvoker.getMockObject( + () -> MockInvoker.getMockObject( ApplicationModel.defaultModel().getExtensionDirector(), "java.io.Serializable", Serializable.class)); @@ -155,17 +150,17 @@ void testGetMockObject() { @Test void testNormalizeMock() { - Assertions.assertNull(org.apache.dubbo.rpc.support.MockInvoker.normalizeMock(null)); - - Assertions.assertEquals("", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("")); - Assertions.assertEquals("", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("fail:")); - Assertions.assertEquals("", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("force:")); - Assertions.assertEquals("throw", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("throw")); - Assertions.assertEquals("default", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("fail")); - Assertions.assertEquals("default", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("force")); - Assertions.assertEquals("default", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("true")); - Assertions.assertEquals("default", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("default")); - Assertions.assertEquals("return null", org.apache.dubbo.rpc.support.MockInvoker.normalizeMock("return")); + Assertions.assertNull(MockInvoker.normalizeMock(null)); + + Assertions.assertEquals("", MockInvoker.normalizeMock("")); + Assertions.assertEquals("", MockInvoker.normalizeMock("fail:")); + Assertions.assertEquals("", MockInvoker.normalizeMock("force:")); + Assertions.assertEquals("throw", MockInvoker.normalizeMock("throw")); + Assertions.assertEquals("default", MockInvoker.normalizeMock("fail")); + Assertions.assertEquals("default", MockInvoker.normalizeMock("force")); + Assertions.assertEquals("default", MockInvoker.normalizeMock("true")); + Assertions.assertEquals("default", MockInvoker.normalizeMock("default")); + Assertions.assertEquals("return null", MockInvoker.normalizeMock("return")); Assertions.assertEquals("return null", MockInvoker.normalizeMock("return null")); } } diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j/Log4jLogger.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j/Log4jLogger.java index ed621224ce7d..9c1073aa9c23 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j/Log4jLogger.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j/Log4jLogger.java @@ -162,9 +162,4 @@ public boolean isWarnEnabled() { public boolean isErrorEnabled() { return logger.isEnabledFor(Level.ERROR); } - - // test purpose only - public org.apache.log4j.Logger getLogger() { - return logger; - } } diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j2/Log4j2Logger.java b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j2/Log4j2Logger.java index f0a1ab773d66..8701b181fed5 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j2/Log4j2Logger.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/logger/log4j2/Log4j2Logger.java @@ -150,4 +150,9 @@ public boolean isWarnEnabled() { public boolean isErrorEnabled() { return logger.isErrorEnabled(); } + + // test purpose only + public org.apache.logging.log4j.Logger getLogger() { + return logger; + } } diff --git a/dubbo-rpc/dubbo-rpc-api/pom.xml b/dubbo-rpc/dubbo-rpc-api/pom.xml index f7c8272edf30..40d072ed2699 100644 --- a/dubbo-rpc/dubbo-rpc-api/pom.xml +++ b/dubbo-rpc/dubbo-rpc-api/pom.xml @@ -56,10 +56,5 @@ log4j-slf4j-impl test - - log4j - log4j - test - diff --git a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/AccessLogFilterTest.java b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/AccessLogFilterTest.java index 6b96546cb3c4..4a326b477ec6 100644 --- a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/AccessLogFilterTest.java +++ b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/AccessLogFilterTest.java @@ -19,8 +19,8 @@ import org.apache.dubbo.common.URL; import org.apache.dubbo.common.logger.ErrorTypeAwareLogger; import org.apache.dubbo.common.logger.LoggerFactory; -import org.apache.dubbo.common.logger.log4j.Log4jLogger; -import org.apache.dubbo.common.logger.support.FailsafeLogger; +import org.apache.dubbo.common.utils.DubboAppender; +import org.apache.dubbo.common.utils.LogUtil; import org.apache.dubbo.rpc.Invocation; import org.apache.dubbo.rpc.Invoker; import org.apache.dubbo.rpc.support.AccessLogData; @@ -31,19 +31,11 @@ import java.util.Map; import java.util.Queue; -import org.apache.log4j.Appender; -import org.apache.log4j.Level; -import org.apache.log4j.spi.LoggingEvent; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; /** * AccessLogFilterTest.java @@ -51,14 +43,6 @@ class AccessLogFilterTest { private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger("mock.dubbo.access.log"); - private static org.apache.log4j.Logger realLogger; - - @BeforeAll - public static void setupLogger() { - FailsafeLogger failsafeLogger = (FailsafeLogger) logger; - Log4jLogger log4j2Logger = (Log4jLogger) failsafeLogger.getLogger(); - realLogger = log4j2Logger.getLogger(); - } AccessLogFilter accessLogFilter = new AccessLogFilter(); @@ -85,52 +69,40 @@ public void testDefault() throws NoSuchFieldException, IllegalAccessException { @Test void testCustom() { + DubboAppender.doStart(); ErrorTypeAwareLogger originalLogger = AccessLogFilter.logger; long originalInterval = AccessLogFilter.getInterval(); - Appender appender = mock(Appender.class); AccessLogFilter.setInterval(500); AccessLogFilter.logger = logger; AccessLogFilter customAccessLogFilter = new AccessLogFilter(); try { - realLogger.addAppender(appender); URL url = URL.valueOf("test://test:11/test?accesslog=custom-access.log"); Invoker invoker = new MyInvoker<>(url); Invocation invocation = new MockInvocation(); customAccessLogFilter.invoke(invoker, invocation); sleep(); - ArgumentCaptor argument = ArgumentCaptor.forClass(LoggingEvent.class); - verify(appender, times(2)).doAppend(argument.capture()); - assertEquals(Level.WARN, argument.getAllValues().get(1).getLevel()); - assertTrue(argument.getAllValues() - .get(1) - .getRenderedMessage() - .contains("Change of accesslog file path not allowed")); + assertEquals(1, LogUtil.findMessage("Change of accesslog file path not allowed")); } finally { customAccessLogFilter.destroy(); - realLogger.removeAppender(appender); + DubboAppender.clear(); AccessLogFilter.logger = originalLogger; AccessLogFilter.setInterval(originalInterval); } - Appender appender2 = mock(Appender.class); AccessLogFilter.setInterval(500); AccessLogFilter.logger = logger; AccessLogFilter customAccessLogFilter2 = new AccessLogFilter(); try { - realLogger.addAppender(appender2); URL url2 = URL.valueOf("test://test:11/test?accesslog=custom-access.log&accesslog.fixed.path=false"); Invoker invoker = new MyInvoker<>(url2); Invocation invocation = new MockInvocation(); customAccessLogFilter2.invoke(invoker, invocation); sleep(); - ArgumentCaptor argument = ArgumentCaptor.forClass(LoggingEvent.class); - verify(appender2, times(2)).doAppend(argument.capture()); - assertEquals(Level.WARN, argument.getAllValues().get(1).getLevel()); - assertTrue(argument.getAllValues().get(1).getRenderedMessage().contains("Accesslog file path changed to")); + assertEquals(1, LogUtil.findMessage("Accesslog file path changed to")); } finally { customAccessLogFilter2.destroy(); - realLogger.removeAppender(appender2); + DubboAppender.clear(); AccessLogFilter.logger = originalLogger; AccessLogFilter.setInterval(originalInterval); }