From 2ab4657d76ac35c6050789482b932a8fbf8dcf99 Mon Sep 17 00:00:00 2001 From: Stefaan Dutry Date: Mon, 21 May 2018 20:28:32 +0200 Subject: [PATCH 1/6] remove TimerInterceptor Part of removing the profiling layer Issue: * WW-4779 --- .../xwork2/interceptor/TimerInterceptor.java | 251 ------------------ core/src/main/resources/struts-default.xml | 1 - core/src/main/resources/xwork-default.xml | 1 - .../xwork2/config/ConfigurationTest.java | 4 +- .../XmlConfigurationProviderActionsTest.java | 6 +- ...urationProviderInterceptorsSpringTest.java | 10 +- ...ConfigurationProviderInterceptorsTest.java | 28 +- .../interceptor/TimerInterceptorTest.java | 163 ------------ .../spring/SpringObjectFactoryTest.java | 8 +- .../xwork2/config/providers/xwork- test.xml | 4 +- ...rk-test-actions-packagedefaultclassref.xml | 4 +- .../config/providers/xwork-test-actions.xml | 4 +- .../xwork-test-interceptor-defaultref.xml | 4 +- .../xwork-test-interceptor-inheritance.xml | 4 +- .../xwork-test-interceptor-params.xml | 4 +- .../xwork-test-interceptors-basic.xml | 4 +- .../xwork-test-interceptors-spring.xml | 2 +- core/src/test/resources/xwork-proxyinvoke.xml | 2 - .../src/test/resources/xwork-test-default.xml | 2 - .../test/resources/xwork-test-validation.xml | 2 - 20 files changed, 42 insertions(+), 466 deletions(-) delete mode 100644 core/src/main/java/com/opensymphony/xwork2/interceptor/TimerInterceptor.java delete mode 100644 core/src/test/java/com/opensymphony/xwork2/interceptor/TimerInterceptorTest.java diff --git a/core/src/main/java/com/opensymphony/xwork2/interceptor/TimerInterceptor.java b/core/src/main/java/com/opensymphony/xwork2/interceptor/TimerInterceptor.java deleted file mode 100644 index 3530cb78f..000000000 --- a/core/src/main/java/com/opensymphony/xwork2/interceptor/TimerInterceptor.java +++ /dev/null @@ -1,251 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package com.opensymphony.xwork2.interceptor; - -import com.opensymphony.xwork2.ActionInvocation; -import org.apache.commons.lang3.StringUtils; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -/** - * - * This interceptor logs the amount of time in milliseconds. In order for this interceptor to work properly, the - * logging framework must be set to at least the INFO level. - * This interceptor relies on the Commons Logging API to - * report its execution-time value. - * - * - * - * - * - * - * The parameters above enables us to log all action execution times in our own logfile. - * - * - * - * - * This interceptor can be extended to provide custom message format. Users should override the - * invokeUnderTiming method. - * - * - * 
- * 
- * <!-- records only the action's execution time -->
- * <action name="someAction" class="com.examples.SomeAction">
- *     <interceptor-ref name="completeStack"/>
- *     <interceptor-ref name="timer"/>
- *     <result name="success">good_result.ftl</result>
- * </action>
- *
- * <!-- records action's execution time as well as other interceptors-->
- * <action name="someAction" class="com.examples.SomeAction">
- *     <interceptor-ref name="timer"/>
- *     <interceptor-ref name="completeStack"/>
- *     <result name="success">good_result.ftl</result>
- * </action>
- * 
- *
- * - * This second example uses our own log category at debug level. - * - * 
- * 
- * <!-- records only the action's execution time -->
- * <action name="someAction" class="com.examples.SomeAction">
- *     <interceptor-ref name="completeStack"/>
- *     <interceptor-ref name="timer">
- *         <param name="logLevel">debug</param>
- *         <param name="logCategory">com.mycompany.myapp.actiontime</param>
- *     <interceptor-ref/>
- *     <result name="success">good_result.ftl</result>
- * </action>
- *
- * <!-- records action's execution time as well as other interceptors-->
- * <action name="someAction" class="com.examples.SomeAction">
- *     <interceptor-ref name="timer"/>
- *     <interceptor-ref name="completeStack"/>
- *     <result name="success">good_result.ftl</result>
- * </action>
- * 
- *
- * - * @author Jason Carreira - * @author Claus Ibsen - * - * @deprecated will be dropped with next major release (2.6) - */ -@Deprecated -public class TimerInterceptor extends AbstractInterceptor { - - private static final Logger LOG = LogManager.getLogger(TimerInterceptor.class); - - protected Logger categoryLogger; - protected String logCategory; - protected String logLevel; - - public String getLogCategory() { - return logCategory; - } - - public void setLogCategory(String logCatgory) { - this.logCategory = logCatgory; - } - - public String getLogLevel() { - return logLevel; - } - - public void setLogLevel(String logLevel) { - this.logLevel = logLevel; - } - - @Override - public String intercept(ActionInvocation invocation) throws Exception { - if (! shouldLog()) { - return invocation.invoke(); - } else { - return invokeUnderTiming(invocation); - } - } - - /** - * Is called to invoke the action invocation and time the execution time. - * - * @param invocation the action invocation. - * @return the result of the action execution. - * @throws Exception can be thrown from the action. - */ - protected String invokeUnderTiming(ActionInvocation invocation) throws Exception { - long startTime = System.currentTimeMillis(); - String result = invocation.invoke(); - long executionTime = System.currentTimeMillis() - startTime; - - StringBuilder message = new StringBuilder(100); - message.append("Executed action ["); - String namespace = invocation.getProxy().getNamespace(); - if (StringUtils.isNotBlank(namespace)) { - message.append(namespace).append("/"); - } - message.append(invocation.getProxy().getActionName()); - message.append("!"); - message.append(invocation.getProxy().getMethod()); - message.append("] took ").append(executionTime).append(" ms."); - - doLog(getLoggerToUse(), message.toString()); - - return result; - } - - /** - * Determines if we should log the time. - * - * @return true to log, false to not log. - */ - protected boolean shouldLog() { - // default check first - if (logLevel == null && logCategory == null) { - return LOG.isInfoEnabled(); - } - - // okay user have set some parameters - return isLoggerEnabled(getLoggerToUse(), logLevel); - } - - /** - * Get's the logger to use. - * - * @return the logger to use. - */ - protected Logger getLoggerToUse() { - if (logCategory != null) { - if (categoryLogger == null) { - // init category logger - categoryLogger = LogManager.getLogger(logCategory); - if (logLevel == null) { - logLevel = "info"; // use info as default if not provided - } - } - return categoryLogger; - } - - return LOG; - } - - /** - * Performs the actual logging. - * - * @param logger the provided logger to use. - * @param message the message to log. - */ - protected void doLog(Logger logger, String message) { - if (logLevel == null) { - logger.info(message); - return; - } - - if ("debug".equalsIgnoreCase(logLevel)) { - logger.debug(message); - } else if ("info".equalsIgnoreCase(logLevel)) { - logger.info(message); - } else if ("warn".equalsIgnoreCase(logLevel)) { - logger.warn(message); - } else if ("error".equalsIgnoreCase(logLevel)) { - logger.error(message); - } else if ("fatal".equalsIgnoreCase(logLevel)) { - logger.fatal(message); - } else if ("trace".equalsIgnoreCase(logLevel)) { - logger.trace(message); - } else { - throw new IllegalArgumentException("LogLevel [" + logLevel + "] is not supported"); - } - } - - /** - * Is the given logger enalbed at the given level? - * - * @param logger the logger. - * @param level the level to check if isXXXEnabled. - * @return true if enabled, false if not. - */ - private static boolean isLoggerEnabled(Logger logger, String level) { - if ("debug".equalsIgnoreCase(level)) { - return logger.isDebugEnabled(); - } else if ("info".equalsIgnoreCase(level)) { - return logger.isInfoEnabled(); - } else if ("warn".equalsIgnoreCase(level)) { - return logger.isWarnEnabled(); - } else if ("error".equalsIgnoreCase(level)) { - return logger.isErrorEnabled(); - } else if ("fatal".equalsIgnoreCase(level)) { - return logger.isFatalEnabled(); - } else if ("trace".equalsIgnoreCase(level)) { - return logger.isTraceEnabled(); - } else { - throw new IllegalArgumentException("LogLevel [" + level + "] is not supported"); - } - } - -} diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 204c5f1c0..85f67bc9b 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -226,7 +226,6 @@ - diff --git a/core/src/main/resources/xwork-default.xml b/core/src/main/resources/xwork-default.xml index eb843bdfa..9e7f68dec 100644 --- a/core/src/main/resources/xwork-default.xml +++ b/core/src/main/resources/xwork-default.xml @@ -36,7 +36,6 @@ - diff --git a/core/src/test/java/com/opensymphony/xwork2/config/ConfigurationTest.java b/core/src/test/java/com/opensymphony/xwork2/config/ConfigurationTest.java index 424bfde17..ee2e39461 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/ConfigurationTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/ConfigurationTest.java @@ -283,7 +283,7 @@ public class ConfigurationTest extends XWorkTestCase { assertNotNull(proxy); proxy = actionProxyFactory.createActionProxy("multipleInheritance", "testMultipleInheritance", null, null); assertNotNull(proxy); - assertEquals(5, proxy.getConfig().getInterceptors().size()); + assertEquals(4, proxy.getConfig().getInterceptors().size()); assertEquals(2, proxy.getConfig().getResults().size()); } catch (Exception e) { e.printStackTrace(); @@ -294,7 +294,7 @@ public class ConfigurationTest extends XWorkTestCase { public void testPackageExtension() { try { ActionProxy proxy = actionProxyFactory.createActionProxy("/foo/bar", "Bar", null, null); - assertEquals(5, proxy.getConfig().getInterceptors().size()); + assertEquals(4, proxy.getConfig().getInterceptors().size()); } catch (Exception e) { e.printStackTrace(); fail(); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java index 033dd08b8..f702757d8 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java @@ -23,9 +23,9 @@ import com.opensymphony.xwork2.SimpleAction; import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.config.ConfigurationProvider; import com.opensymphony.xwork2.config.entities.*; -import com.opensymphony.xwork2.interceptor.TimerInterceptor; import com.opensymphony.xwork2.mock.MockInterceptor; import com.opensymphony.xwork2.mock.MockResult; +import org.apache.struts2.interceptor.NoOpInterceptor; import java.util.ArrayList; import java.util.HashMap; @@ -68,8 +68,8 @@ public class XmlConfigurationProviderActionsTest extends ConfigurationTestBase { params.put("bar", "24"); results.put("success", new ResultConfig.Builder("success", MockResult.class.getName()).build()); - InterceptorConfig timerInterceptorConfig = new InterceptorConfig.Builder("timer", TimerInterceptor.class.getName()).build(); - interceptors.add(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptorConfig, new HashMap()))); + InterceptorConfig noopInterceptorConfig = new InterceptorConfig.Builder("noop", NoOpInterceptor.class.getName()).build(); + interceptors.add(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptorConfig, new HashMap()))); ActionConfig fooAction = new ActionConfig.Builder("", "Foo", SimpleAction.class.getName()) .addParams(params) diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsSpringTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsSpringTest.java index 6296fe68e..2f17a0de0 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsSpringTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsSpringTest.java @@ -23,7 +23,7 @@ import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.config.ConfigurationProvider; import com.opensymphony.xwork2.config.entities.InterceptorConfig; import com.opensymphony.xwork2.config.entities.PackageConfig; -import com.opensymphony.xwork2.interceptor.TimerInterceptor; +import org.apache.struts2.interceptor.NoOpInterceptor; import org.springframework.beans.MutablePropertyValues; import org.springframework.context.support.StaticApplicationContext; @@ -39,13 +39,13 @@ import java.util.Map; */ public class XmlConfigurationProviderInterceptorsSpringTest extends ConfigurationTestBase { - InterceptorConfig timerInterceptor = new InterceptorConfig.Builder("timer", TimerInterceptor.class.getName()).build(); + InterceptorConfig noopInterceptor = new InterceptorConfig.Builder("noop", NoOpInterceptor.class.getName()).build(); ObjectFactory objectFactory; StaticApplicationContext sac; public void testInterceptorsLoadedFromSpringApplicationContext() throws ConfigurationException { - sac.registerSingleton("timer-interceptor", TimerInterceptor.class, new MutablePropertyValues()); + sac.registerSingleton("noop-interceptor", NoOpInterceptor.class, new MutablePropertyValues()); final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-interceptors-spring.xml"; @@ -64,8 +64,8 @@ public class XmlConfigurationProviderInterceptorsSpringTest extends Configuratio assertEquals(1, interceptorConfigs.size()); // assertions for interceptors - InterceptorConfig seen = (InterceptorConfig) interceptorConfigs.get("timer"); - assertEquals("timer-interceptor", seen.getClassName()); + InterceptorConfig seen = (InterceptorConfig) interceptorConfigs.get("noop"); + assertEquals("noop-interceptor", seen.getClassName()); } @Override diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java index 9cb449218..70bfe23f3 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java @@ -25,8 +25,8 @@ import com.opensymphony.xwork2.config.ConfigurationProvider; import com.opensymphony.xwork2.config.RuntimeConfiguration; import com.opensymphony.xwork2.config.entities.*; import com.opensymphony.xwork2.interceptor.LoggingInterceptor; -import com.opensymphony.xwork2.interceptor.TimerInterceptor; import com.opensymphony.xwork2.mock.MockInterceptor; +import org.apache.struts2.interceptor.NoOpInterceptor; import java.util.ArrayList; import java.util.HashMap; @@ -44,7 +44,7 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB InterceptorConfig loggingInterceptor = new InterceptorConfig.Builder("logging", LoggingInterceptor.class.getName()).build(); InterceptorConfig mockInterceptor = new InterceptorConfig.Builder("mock", MockInterceptor.class.getName()).build(); - InterceptorConfig timerInterceptor = new InterceptorConfig.Builder("timer", TimerInterceptor.class.getName()).build(); + InterceptorConfig noopInterceptor = new InterceptorConfig.Builder("noop", NoOpInterceptor.class.getName()).build(); ObjectFactory objectFactory; @Override @@ -68,16 +68,16 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB // the default interceptor stack InterceptorStackConfig defaultStack = new InterceptorStackConfig.Builder("defaultStack") - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .addInterceptor(new InterceptorMapping("test", objectFactory.buildInterceptor(mockInterceptor, params))) .build(); // the derivative interceptor stack InterceptorStackConfig derivativeStack = new InterceptorStackConfig.Builder("derivativeStack") - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) - .addInterceptor(new InterceptorMapping("test", objectFactory.buildInterceptor(mockInterceptor, params))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("test", objectFactory.buildInterceptor(mockInterceptor, params))) .addInterceptor(new InterceptorMapping("logging", objectFactory.buildInterceptor(loggingInterceptor, new HashMap()))) - .build(); + .build(); // execute the configuration provider.init(configuration); @@ -90,7 +90,7 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB assertEquals(5, interceptorConfigs.size()); // assertions for interceptors - assertEquals(timerInterceptor, interceptorConfigs.get("timer")); + assertEquals(noopInterceptor, interceptorConfigs.get("noop")); assertEquals(loggingInterceptor, interceptorConfigs.get("logging")); assertEquals(paramsInterceptor, interceptorConfigs.get("test")); @@ -114,13 +114,13 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB .build(); ActionConfig actionWithDefaultRef = new ActionConfig.Builder("", "ActionWithDefaultRef", SimpleAction.class.getName()) - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .build(); // sub package // this should inherit ActionConfig actionWithNoRef = new ActionConfig.Builder("", "ActionWithNoRef", SimpleAction.class.getName()) - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .build(); interceptors = new ArrayList<>(); @@ -144,7 +144,7 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB // expectations - the inherited interceptor stack InterceptorStackConfig inheritedStack = new InterceptorStackConfig.Builder("subDefaultStack") - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .build(); ConfigurationProvider provider = buildConfigurationProvider("com/opensymphony/xwork2/config/providers/xwork-test-interceptor-inheritance.xml"); @@ -160,8 +160,8 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB // expectations - the inherited interceptor stack inheritedStack = new InterceptorStackConfig.Builder("subSubDefaultStack") - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .build(); PackageConfig subSubPkg = configuration.getPackageConfig("subSubPackage"); @@ -178,7 +178,7 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB params.put("expectedFoo", "expectedFooValue"); InterceptorStackConfig defaultStack = new InterceptorStackConfig.Builder("defaultStack") - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .addInterceptor(new InterceptorMapping("test", objectFactory.buildInterceptor(mockInterceptor, params))) .build(); @@ -195,7 +195,7 @@ public class XmlConfigurationProviderInterceptorsTest extends ConfigurationTestB interceptorParams.put("foo", "foo123"); InterceptorStackConfig defaultStack2 = new InterceptorStackConfig.Builder("defaultStack") - .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addInterceptor(new InterceptorMapping("noop", objectFactory.buildInterceptor(noopInterceptor, new HashMap()))) .addInterceptor(new InterceptorMapping("test", objectFactory.buildInterceptor(mockInterceptor, interceptorParams))) .build(); diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/TimerInterceptorTest.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/TimerInterceptorTest.java deleted file mode 100644 index 7e02e9780..000000000 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/TimerInterceptorTest.java +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package com.opensymphony.xwork2.interceptor; - -import com.opensymphony.xwork2.SimpleFooAction; -import com.opensymphony.xwork2.XWorkTestCase; -import com.opensymphony.xwork2.mock.MockActionInvocation; -import com.opensymphony.xwork2.mock.MockActionProxy; -import org.apache.logging.log4j.Logger; - -/** - * Unit test for {@link TimerInterceptor}. - * - * @author Claus Ibsen - */ -public class TimerInterceptorTest extends XWorkTestCase { - - private MyTimerInterceptor interceptor; - private MockActionInvocation mai; - private MockActionProxy ap; - - - public void testTimerInterceptor() throws Exception { - TimerInterceptor real = new TimerInterceptor(); - real.init(); - real.intercept(mai); - real.destroy(); - } - - public void testInvalidLogLevel() throws Exception { - TimerInterceptor real = new TimerInterceptor(); - real.setLogLevel("xxxx"); - real.init(); - try { - real.intercept(mai); - fail("Should not have reached this point."); - } catch (IllegalArgumentException e) { - // success - } - } - - public void testDefault() throws Exception { - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - } - - public void testNoNamespace() throws Exception { - ap.setNamespace(null); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myAction!execute] took ")); - } - - public void testInputMethod() throws Exception { - ap.setMethod("input"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!input] took ")); - } - - public void testTraceLevel() throws Exception { - interceptor.setLogLevel("trace"); - interceptor.intercept(mai); - assertNull(interceptor.message); // no default logging at trace level - assertEquals("trace", interceptor.getLogLevel()); - } - - public void testDebugLevel() throws Exception { - interceptor.setLogLevel("debug"); - interceptor.intercept(mai); - assertNull(interceptor.message); // no default logging at debug level - } - - public void testInfoLevel() throws Exception { - interceptor.setLogLevel("info"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - } - - public void testWarnLevel() throws Exception { - interceptor.setLogLevel("warn"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - } - - public void testErrorLevel() throws Exception { - interceptor.setLogLevel("error"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - } - - public void testFatalLevel() throws Exception { - interceptor.setLogLevel("fatal"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - } - - public void testLogCategory() throws Exception { - interceptor.setLogCategory("com.mycompany.myapp.actiontiming"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - } - - public void testLogCategoryLevel() throws Exception { - interceptor.setLogCategory("com.mycompany.myapp.actiontiming"); - interceptor.setLogLevel("error"); - interceptor.intercept(mai); - assertTrue(interceptor.message.startsWith("Executed action [myApp/myAction!execute] took ")); - assertEquals("com.mycompany.myapp.actiontiming", interceptor.getLogCategory()); - } - - @Override - protected void setUp() throws Exception { - super.setUp(); - interceptor = new MyTimerInterceptor(); - interceptor.init(); - - mai = new MockActionInvocation(); - ap = new MockActionProxy(); - ap.setActionName("myAction"); - ap.setNamespace("myApp"); - ap.setMethod("execute"); - mai.setAction(new SimpleFooAction()); - mai.setProxy(ap); - } - - @Override - protected void tearDown() throws Exception { - super.tearDown(); - interceptor.destroy(); - ap = null; - mai = null; - } - - private class MyTimerInterceptor extends TimerInterceptor { - - private Logger logger; - private String message; - - @Override - protected void doLog(Logger logger, String message) { - super.doLog(logger, message); - - this.logger = logger; - this.message = message; - } - } - -} diff --git a/core/src/test/java/com/opensymphony/xwork2/spring/SpringObjectFactoryTest.java b/core/src/test/java/com/opensymphony/xwork2/spring/SpringObjectFactoryTest.java index 776bd791b..7c71660bc 100644 --- a/core/src/test/java/com/opensymphony/xwork2/spring/SpringObjectFactoryTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/spring/SpringObjectFactoryTest.java @@ -26,12 +26,12 @@ import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.interceptor.Interceptor; import com.opensymphony.xwork2.interceptor.ModelDrivenInterceptor; -import com.opensymphony.xwork2.interceptor.TimerInterceptor; import com.opensymphony.xwork2.test.StubConfigurationProvider; import com.opensymphony.xwork2.util.location.LocatableProperties; import com.opensymphony.xwork2.validator.Validator; import com.opensymphony.xwork2.validator.validators.ExpressionValidator; import com.opensymphony.xwork2.validator.validators.RequiredStringValidator; +import org.apache.struts2.interceptor.NoOpInterceptor; import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.autoproxy.BeanNameAutoProxyCreator; import org.springframework.aop.interceptor.DebugInterceptor; @@ -126,12 +126,12 @@ public class SpringObjectFactoryTest extends XWorkTestCase { } public void testObtainInterceptorBySpringName() throws Exception { - sac.registerSingleton("timer-interceptor", TimerInterceptor.class, new MutablePropertyValues()); + sac.registerSingleton("noop-interceptor", NoOpInterceptor.class, new MutablePropertyValues()); - InterceptorConfig iConfig = new InterceptorConfig.Builder("timer", "timer-interceptor").build(); + InterceptorConfig iConfig = new InterceptorConfig.Builder("noop", "noop-interceptor").build(); Interceptor interceptor = objectFactory.buildInterceptor(iConfig, new HashMap()); - assertEquals(TimerInterceptor.class, interceptor.getClass()); + assertEquals(NoOpInterceptor.class, interceptor.getClass()); } public void testObtainResultBySpringName() throws Exception { diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork- test.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork- test.xml index 9a684cbcd..50bc3dab7 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork- test.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork- test.xml @@ -33,14 +33,14 @@ - + fooDefault - + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions-packagedefaultclassref.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions-packagedefaultclassref.xml index 66d4d086b..6b0008b2c 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions-packagedefaultclassref.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions-packagedefaultclassref.xml @@ -33,14 +33,12 @@ - fooDefault - @@ -59,4 +57,4 @@ - \ No newline at end of file + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions.xml index 9a684cbcd..50bc3dab7 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-actions.xml @@ -33,14 +33,14 @@ - + fooDefault - + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-defaultref.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-defaultref.xml index e71320e40..3902c3b33 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-defaultref.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-defaultref.xml @@ -28,11 +28,11 @@ - + - + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-inheritance.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-inheritance.xml index 0ed913030..a20a87358 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-inheritance.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-inheritance.xml @@ -28,10 +28,10 @@ - + - + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-params.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-params.xml index 1a3400957..4943bbce0 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-params.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptor-params.xml @@ -28,13 +28,13 @@ - + fooDefault - + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-basic.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-basic.xml index d5722456e..ac52fc3f9 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-basic.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-basic.xml @@ -28,14 +28,14 @@ - + expectedFoo - + diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-spring.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-spring.xml index 7db93d40e..bc6e93bfa 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-spring.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-interceptors-spring.xml @@ -28,7 +28,7 @@ - + diff --git a/core/src/test/resources/xwork-proxyinvoke.xml b/core/src/test/resources/xwork-proxyinvoke.xml index 53baa495d..2ce698dc2 100644 --- a/core/src/test/resources/xwork-proxyinvoke.xml +++ b/core/src/test/resources/xwork-proxyinvoke.xml @@ -37,7 +37,6 @@ - @@ -56,7 +55,6 @@ - diff --git a/core/src/test/resources/xwork-test-default.xml b/core/src/test/resources/xwork-test-default.xml index e9f222bc1..bdcc91ba1 100644 --- a/core/src/test/resources/xwork-test-default.xml +++ b/core/src/test/resources/xwork-test-default.xml @@ -33,7 +33,6 @@ - @@ -53,7 +52,6 @@ - diff --git a/core/src/test/resources/xwork-test-validation.xml b/core/src/test/resources/xwork-test-validation.xml index df5dd9fbe..ca60347b8 100644 --- a/core/src/test/resources/xwork-test-validation.xml +++ b/core/src/test/resources/xwork-test-validation.xml @@ -34,7 +34,6 @@ - @@ -55,7 +54,6 @@ - From 1045836778466d9105c6ebf074e45bc0b36d01c9 Mon Sep 17 00:00:00 2001 From: Stefaan Dutry Date: Mon, 21 May 2018 21:01:56 +0200 Subject: [PATCH 2/6] remove ObjectProfiler Part of removing the profiling layer Issue: * WW-4779 --- .../xwork2/util/profiling/ObjectProfiler.java | 177 ------------------ 1 file changed, 177 deletions(-) delete mode 100644 core/src/main/java/com/opensymphony/xwork2/util/profiling/ObjectProfiler.java diff --git a/core/src/main/java/com/opensymphony/xwork2/util/profiling/ObjectProfiler.java b/core/src/main/java/com/opensymphony/xwork2/util/profiling/ObjectProfiler.java deleted file mode 100644 index 286d82bdf..000000000 --- a/core/src/main/java/com/opensymphony/xwork2/util/profiling/ObjectProfiler.java +++ /dev/null @@ -1,177 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Copyright (c) 2002-2003, Atlassian Software Systems Pty Ltd All rights reserved. - * - * Redistribution and use in source and binary forms, with or without modification, - * are permitted provided that the following conditions are met: - * - * * Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation and/or - * other materials provided with the distribution. - * * Neither the name of Atlassian Software Systems Pty Ltd nor the names of - * its contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR - * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package com.opensymphony.xwork2.util.profiling; - -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.lang.reflect.Proxy; - -/** - * @author Scott Farquhar - * @deprecated will be dropped with next major release (2.6) - */ -@Deprecated -public class ObjectProfiler { - - /** - *

- * Given a class, and an interface that it implements, return a proxied version of the class that implements - * the interface. - *

- * - *

- * The usual use of this is to profile methods from Factory objects: - *

- * - * 
-     * public PersistenceManager getPersistenceManager()
-     * {
-     *   return new DefaultPersistenceManager();
-     * }
-     *
-     * instead write:
-     * public PersistenceManager getPersistenceManager()
-     * {
-     *   return ObjectProfiler.getProfiledObject(PersistenceManager.class, new DefaultPersistenceManager());
-     * }
-     *
- * - *

- * A side effect of this is that you will no longer be able to downcast to DefaultPersistenceManager. This is probably a *good* thing. - *

- * - * @param interfaceClazz The interface to implement. - * @param o The object to proxy - * @return A proxied object, or the input object if the interfaceClazz wasn't an interface. - */ - public static Object getProfiledObject(Class interfaceClazz, Object o) { - //if we are not active - then do nothing - if (!UtilTimerStack.isActive()) { - return o; - } - - //this should always be true - you shouldn't be passing something that isn't an interface - if (interfaceClazz.isInterface()) { - InvocationHandler timerHandler = new TimerInvocationHandler(o); - return Proxy.newProxyInstance(interfaceClazz.getClassLoader(), - new Class[]{interfaceClazz}, timerHandler); - } else { - return o; - } - } - - /** - * A profiled call {@link Method#invoke(java.lang.Object, java.lang.Object[])}. If {@link UtilTimerStack#isActive() } - * returns false, then no profiling is performed. - * - * @param target target method - * @param value value - * @param args arguments - * - * @return target object - * - * @throws IllegalAccessException in case of access errors - * @throws InvocationTargetException in case of invocation errors - */ - public static Object profiledInvoke(Method target, Object value, Object[] args) throws IllegalAccessException, InvocationTargetException { - //if we are not active - then do nothing - if (!UtilTimerStack.isActive()) { - return target.invoke(value, args); - } - - String logLine = getTrimmedClassName(target) + "." + target.getName() + "()"; - - UtilTimerStack.push(logLine); - try { - Object returnValue = target.invoke(value, args); - - //if the return value is an interface then we should also proxy it! - if (returnValue != null && target.getReturnType().isInterface()) { - InvocationHandler timerHandler = new TimerInvocationHandler(returnValue); - return Proxy.newProxyInstance(returnValue.getClass().getClassLoader(), - new Class[]{target.getReturnType()}, timerHandler); - } else { - return returnValue; - } - } finally { - UtilTimerStack.pop(logLine); - } - } - - /** - * Given a method, get the Method name, with no package information. - * - * @param method method - * - * @return method name, with no package information - */ - public static String getTrimmedClassName(Method method) { - String classname = method.getDeclaringClass().getName(); - return classname.substring(classname.lastIndexOf('.') + 1); - } - -} - -/** - * @deprecated will be dropped with next major release (2.6) - */ -@Deprecated -class TimerInvocationHandler implements InvocationHandler { - protected Object target; - - public TimerInvocationHandler(Object target) { - if (target == null) { - throw new IllegalArgumentException("Target Object passed to timer cannot be null"); - } - this.target = target; - } - - public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { - return ObjectProfiler.profiledInvoke(method, target, args); - } - -} From 14c0f47d840a049e88e9774156df687635b18aee Mon Sep 17 00:00:00 2001 From: Stefaan Dutry Date: Mon, 21 May 2018 21:30:56 +0200 Subject: [PATCH 3/6] remove UtilTimerStack Part of removing the profiling layer Issue: * WW-4779 --- .../xwork2/DefaultActionInvocation.java | 34 +- .../xwork2/DefaultActionProxy.java | 9 +- .../xwork2/util/profiling/UtilTimerStack.java | 492 ------------------ .../apache/struts2/dispatcher/Dispatcher.java | 4 - .../ProfilingActivationInterceptor.java | 104 ---- core/src/main/resources/struts-default.xml | 1 - .../util/profiling/UtilTimerStackTest.java | 136 ----- .../struts2/rest/RestActionInvocation.java | 5 - ...ecorator2NewStrutsFreemarkerDecorator.java | 5 - 9 files changed, 7 insertions(+), 783 deletions(-) delete mode 100644 core/src/main/java/com/opensymphony/xwork2/util/profiling/UtilTimerStack.java delete mode 100644 core/src/main/java/org/apache/struts2/interceptor/ProfilingActivationInterceptor.java delete mode 100644 core/src/test/java/com/opensymphony/xwork2/util/profiling/UtilTimerStackTest.java diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java index 77acc3cdd..1c40be511 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java @@ -30,7 +30,6 @@ import com.opensymphony.xwork2.interceptor.WithLazyParams; import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.ValueStackFactory; -import com.opensymphony.xwork2.util.profiling.UtilTimerStack; import ognl.MethodFailedException; import ognl.NoSuchPropertyException; import org.apache.logging.log4j.LogManager; @@ -239,8 +238,6 @@ public class DefaultActionInvocation implements ActionInvocation { public String invoke() throws Exception { String profileKey = "invoke: "; try { - UtilTimerStack.push(profileKey); - if (executed) { throw new IllegalStateException("Action has already executed"); } @@ -249,16 +246,13 @@ public class DefaultActionInvocation implements ActionInvocation { if (interceptors.hasNext()) { final InterceptorMapping interceptorMapping = interceptors.next(); String interceptorMsg = "interceptorMapping: " + interceptorMapping.getName(); - UtilTimerStack.push(interceptorMsg); try { Interceptor interceptor = interceptorMapping.getInterceptor(); if (interceptor instanceof WithLazyParams) { interceptor = lazyParamInjector.injectParams(interceptor, interceptorMapping.getParams(), invocationContext); } resultCode = interceptor.intercept(DefaultActionInvocation.this); - } finally { - UtilTimerStack.pop(interceptorMsg); - } + } finally {} } else { resultCode = invokeActionOnly(); } @@ -283,11 +277,8 @@ public class DefaultActionInvocation implements ActionInvocation { String _profileKey = "preResultListener: "; try { - UtilTimerStack.push(_profileKey); listener.beforeResult(this, resultCode); - } finally { - UtilTimerStack.pop(_profileKey); - } + } finally {} } } @@ -303,10 +294,7 @@ public class DefaultActionInvocation implements ActionInvocation { } return resultCode; - } - finally { - UtilTimerStack.pop(profileKey); - } + } finally {} } public String invokeActionOnly() throws Exception { @@ -317,7 +305,6 @@ public class DefaultActionInvocation implements ActionInvocation { // load action String timerKey = "actionCreate: " + proxy.getActionName(); try { - UtilTimerStack.push(timerKey); action = objectFactory.buildAction(proxy.getActionName(), proxy.getNamespace(), proxy.getConfig(), contextMap); } catch (InstantiationException e) { throw new XWorkException("Unable to instantiate Action!", e, proxy.getConfig()); @@ -338,9 +325,7 @@ public class DefaultActionInvocation implements ActionInvocation { gripe += (((" -- " + e.getMessage()) != null) ? e.getMessage() : " [no message in exception]"); throw new XWorkException(gripe, e, proxy.getConfig()); - } finally { - UtilTimerStack.pop(timerKey); - } + } finally {} if (actionEventListener != null) { action = actionEventListener.prepare(action, stack); @@ -390,7 +375,6 @@ public class DefaultActionInvocation implements ActionInvocation { String timerKey = "executeResult: " + getResultCode(); try { - UtilTimerStack.push(timerKey); if (result != null) { result.execute(this); } else if (resultCode != null && !Action.NONE.equals(resultCode)) { @@ -401,9 +385,7 @@ public class DefaultActionInvocation implements ActionInvocation { LOG.debug("No result returned for action {} at {}", getAction().getClass().getName(), proxy.getConfig().getLocation()); } } - } finally { - UtilTimerStack.pop(timerKey); - } + } finally {} } public void init(ActionProxy proxy) { @@ -451,8 +433,6 @@ public class DefaultActionInvocation implements ActionInvocation { String timerKey = "invokeAction: " + proxy.getActionName(); try { - UtilTimerStack.push(timerKey); - Object methodResult; try { methodResult = ognlUtil.callMethod(methodName + "()", getStack().getContext(), action); @@ -497,9 +477,7 @@ public class DefaultActionInvocation implements ActionInvocation { } else { throw e; } - } finally { - UtilTimerStack.pop(timerKey); - } + } finally {} } /** diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java index 87b9b7f90..d1bfe320c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java @@ -22,7 +22,6 @@ import com.opensymphony.xwork2.config.Configuration; import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.config.entities.ActionConfig; import com.opensymphony.xwork2.inject.Inject; -import com.opensymphony.xwork2.util.profiling.UtilTimerStack; import org.apache.commons.text.StringEscapeUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; @@ -152,14 +151,11 @@ public class DefaultActionProxy implements ActionProxy, Serializable { String profileKey = "execute: "; try { - UtilTimerStack.push(profileKey); - retCode = invocation.invoke(); } finally { if (cleanupContext) { ActionContext.setContext(nestedContext); } - UtilTimerStack.pop(profileKey); } return retCode; @@ -185,7 +181,6 @@ public class DefaultActionProxy implements ActionProxy, Serializable { protected void prepare() { String profileKey = "create DefaultActionProxy: "; try { - UtilTimerStack.push(profileKey); config = configuration.getRuntimeConfiguration().getActionConfig(namespace, actionName); if (config == null && unknownHandlerManager.hasUnknownHandlers()) { @@ -202,9 +197,7 @@ public class DefaultActionProxy implements ActionProxy, Serializable { } else { throw new ConfigurationException(prepareNotAllowedErrorMessage()); } - } finally { - UtilTimerStack.pop(profileKey); - } + } finally {} } protected String prepareNotAllowedErrorMessage() { diff --git a/core/src/main/java/com/opensymphony/xwork2/util/profiling/UtilTimerStack.java b/core/src/main/java/com/opensymphony/xwork2/util/profiling/UtilTimerStack.java deleted file mode 100644 index 87b577748..000000000 --- a/core/src/main/java/com/opensymphony/xwork2/util/profiling/UtilTimerStack.java +++ /dev/null @@ -1,492 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Copyright (c) 2002-2003, Atlassian Software Systems Pty Ltd All rights reserved. - * - * Redistribution and use in source and binary forms, with or without modification, - * are permitted provided that the following conditions are met: - * - * * Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation and/or - * other materials provided with the distribution. - * * Neither the name of Atlassian Software Systems Pty Ltd nor the names of - * its contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR - * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package com.opensymphony.xwork2.util.profiling; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - - -/** - *

A timer stack.

- * - *

- * Struts2 profiling aspects involves the following: - *

- * - *
    - *
  • ActionContextCleanUp
    • - *
  • FreemarkerPageFilter
    • - *
  • DispatcherFilter - *
      - *
    • Dispatcher - *
        - *
      • creation of DefaultActionProxy - *
          - *
        • creation of DefaultActionInvocation - *
            - *
          • creation of Action
            • - *
          - *
          • - *
        - *
        • - *
      • execution of DefaultActionProxy - *
          - *
        • invocation of DefaultActionInvocation - *
            - *
          • invocation of Interceptors
            • - *
          • invocation of Action
            • - *
          • invocation of PreResultListener
            • - *
          • invocation of Result
            • - *
          - *
          • - *
        - *
        • - *
      - *
      • - *
    - *
    • - *
- * - * - * - * - * - *

- * XWork2 profiling aspects involves the following: - *

- * - *
    - *
  • creation of DefaultActionProxy - *
      - *
    • creation of DefaultActionInvocation
      • - *
    - *
    • - *
  • creation of Action
    • - *
  • execution of DefaultActionProxy - *
      - *
    • invocation of DefaultActionInvocation - *
        - *
      • invocation of Interceptors
        • - *
      • invocation of Action
        • - *
      • invocation of PreResultListener
        • - *
      • invocation of Result
        • - *
      - *
      • - *
    - *
    • - *
- * - * - * - * - * - *

- * Activating / Deactivating of the profiling feature could be done through: - *

- * - * - * - * - *

System properties:

- * 
- * 
- *
- *  -Dxwork.profile.activate=true
- *
- * 
- *
- * - * - *

- * This could be done in the container startup script eg. CATALINA_OPTS in catalina.sh - * (tomcat) or using 'java -Dxwork.profile.activate=true -jar start.jar' (jetty) - *

- * - * - * - *

Code :

- * 
- * 
- *
- *  UtilTimerStack.setActivate(true);
- *
- * 
- *
- * - *

- * This could be done in a static block, in a Spring bean with lazy-init='false', - * in a Servlet with init-on-startup as some numeric value, in a Filter or - * Listener's init method etc. - *

- * - * - *

- * Parameter: - *

- * - * 
- * 
- *
- * <action ... >
- *  ...
- *  <interceptor-ref name="profiling">
- *      <param name="profilingKey">profiling</param>
- *  </interceptor-ref>
- *  ...
- * </action>
- *
- * or
- *
- * <action .... >
- * ...
- *  <interceptor-ref name="profiling" />
- * ...
- * </action>
- *
- * through url
- *
- * http://host:port/context/namespace/someAction.action?profiling=true
- *
- * through code
- *
- * ActionContext.getContext().getParameters().put("profiling", "true);
- *
- * 
- *
- * - * - * - *

- * To use profiling activation through parameter, one will need to pass in through - * the 'profiling' parameter (which is the default) and could be changed through - * the param tag in the interceptor-ref. - *

- * - * - *

Warning:

- * - * - *

- * Profiling activation through a parameter requires the following: - *

- * - *
    - *
  • Profiling interceptor in interceptor stack
    • - *
  • dev mode on (struts.devMode=true in struts.properties) - *
- * - * - * - * - *

- * One could filter out the profile logging by having a System property as follows. With this - * 'xwork.profile.mintime' property, one could only log profile information when its execution time - * exceed those specified in 'xwork.profile.mintime' system property. If no such property is specified, - * it will be assumed to be 0, hence all profile information will be logged. - *

- * - * - * - * 
- * 
- *
- *  -Dxwork.profile.mintime=10000
- *
- * 
- *
- * - * - *

- * One could extend the profiling feature provided by Struts2 in their web application as well. - *

- * - * - * 
- * 
- *
- *    String logMessage = "Log message";
- *    UtilTimerStack.push(logMessage);
- *    try {
- *        // do some code
- *    }
- *    finally {
- *        UtilTimerStack.pop(logMessage); // this needs to be the same text as above
- *    }
- *
- * 
- *
- *

- * or - *

- * 
- * 
- *
- *   String result = UtilTimerStack.profile("purchaseItem: ",
- *       new UtilTimerStack.ProfilingBlock<String>() {
- *            public String doProfiling() {
- *               // do some code
- *               return "Ok";
- *            }
- *       });
- *
- * 
- *
- * - * - * - *

- * Profiled result is logged using commons-logging under the logger named - * 'com.opensymphony.xwork2.util.profiling.UtilTimerStack'. Depending on the underlying logging implementation - * say if it is Log4j, one could direct the log to appear in a different file, being emailed to someone or have - * it stored in the db. - *

- * - * - * - * @deprecated will be dropped with next major release (2.6) - */ -@Deprecated -public class UtilTimerStack { - - // A reference to the current ProfilingTimerBean - protected static ThreadLocal current = new ThreadLocal<>(); - - /** - * System property that controls whether this timer should be used or not. Set to "true" activates - * the timer. Set to "false" to deactivate. - */ - public static final String ACTIVATE_PROPERTY = "xwork.profile.activate"; - - /** - * System property that controls the min time, that if exceeded will cause a log (at INFO level) to be - * created. - */ - public static final String MIN_TIME = "xwork.profile.mintime"; - - private static final Logger LOG = LogManager.getLogger(UtilTimerStack.class); - - /** - * Initialized in a static block, it can be changed at runtime by calling setActive(...) - */ - private static boolean active; - - static { - active = "true".equalsIgnoreCase(System.getProperty(ACTIVATE_PROPERTY)); - } - - /** - * Create and start a performance profiling with the name given. Deal with - * profile hierarchy automatically, so caller don't have to be concern about it. - * - * @param name profile name - */ - public static void push(String name) { - if (!isActive()) { - return; - } - - //create a new timer and start it - ProfilingTimerBean newTimer = new ProfilingTimerBean(name); - newTimer.setStartTime(); - - //if there is a current timer - add the new timer as a child of it - ProfilingTimerBean currentTimer = (ProfilingTimerBean) current.get(); - if (currentTimer != null) { - currentTimer.addChild(newTimer); - } - - //set the new timer to be the current timer - current.set(newTimer); - } - - /** - * End a performance profiling with the name given. Deal with - * profile hierarchy automatically, so caller don't have to be concern about it. - * - * @param name profile name - */ - public static void pop(String name) { - if (!isActive()) { - return; - } - - ProfilingTimerBean currentTimer = current.get(); - - //if the timers are matched up with each other (ie push("a"); pop("a")); - if (currentTimer != null && name != null && name.equals(currentTimer.getResource())) { - currentTimer.setEndTime(); - ProfilingTimerBean parent = currentTimer.getParent(); - //if we are the root timer, then print out the times - if (parent == null) { - printTimes(currentTimer); - current.set(null); //for those servers that use thread pooling - } else { - current.set(parent); - } - } else { - //if timers are not matched up, then print what we have, and then print warning. - if (currentTimer != null) { - printTimes(currentTimer); - current.set(null); //prevent printing multiple times - LOG.warn("Unmatched Timer. Was expecting {}, instead got {}", currentTimer.getResource(), name); - } - } - } - - /** - * Do a log (at INFO level) of the time taken for this particular profiling. - * - * @param currentTimer profiling timer bean - */ - private static void printTimes(ProfilingTimerBean currentTimer) { - LOG.info(currentTimer.getPrintable(getMinTime())); - } - - /** - * Get the min time for this profiling, it searches for a System property - * 'xwork.profile.mintime' and default to 0. - * - * @return min time for this profiling - */ - private static long getMinTime() { - try { - return Long.parseLong(System.getProperty(MIN_TIME, "0")); - } catch (NumberFormatException e) { - return -1; - } - } - - /** - * Determine if profiling is being activated, by searching for a system property - * 'xwork.profile.activate', default to false (profiling is off). - * - * @return true, if active, false otherwise. - */ - public static boolean isActive() { - return active; - } - - /** - * @param active Turn profiling on or off. - */ - public static void setActive(boolean active) { - if (active) { - System.setProperty(ACTIVATE_PROPERTY, "true"); - } else { - System.clearProperty(ACTIVATE_PROPERTY); - } - UtilTimerStack.active = active; - } - - - /** - *

- * A convenience method that allows block of code subjected to profiling to be executed - * and avoid the need of coding boiler code that does pushing (UtilTimeBean.push(...)) and - * poping (UtilTimerBean.pop(...)) in a try ... finally ... block. - *

- * - *

- * Example of usage: - *

- * - * 
-     * 	 // we need a returning result
-     *   String result = UtilTimerStack.profile("purchaseItem: ",
-     *       new UtilTimerStack.ProfilingBlock<String>() {
-     *            public String doProfiling() {
-     *               getMyService().purchaseItem(....)
-     *               return "Ok";
-     *            }
-     *       });
-     *
- * - * or - * - * 
-     *   // we don't need a returning result
-     *   UtilTimerStack.profile("purchaseItem: ",
-     *       new UtilTimerStack.ProfilingBlock<String>() {
-     *            public String doProfiling() {
-     *               getMyService().purchaseItem(....)
-     *               return null;
-     *            }
-     *       });
-     *
- * - * @param any return value if there's one. - * @param name profile name - * @param block code block subjected to profiling - * @return T - * @throws Exception in case of any errors - */ - public static T profile(String name, ProfilingBlock block) throws Exception { - UtilTimerStack.push(name); - try { - return block.doProfiling(); - } finally { - UtilTimerStack.pop(name); - } - } - - /** - * A callback interface where code subjected to profile is to be executed. This eliminates the need - * of coding boiler code that does pushing (UtilTimerBean.push(...)) and poping (UtilTimerBean.pop(...)) - * in a try ... finally ... block. - * - * @param type - */ - public static interface ProfilingBlock { - - /** - * Method that execute the code subjected to profiling. - * - * @return profiles Type - * @throws Exception in case of any errors - */ - T doProfiling() throws Exception; - } -} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 1281533c9..c372410de 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -34,7 +34,6 @@ import com.opensymphony.xwork2.util.ValueStackFactory; import com.opensymphony.xwork2.util.location.LocatableProperties; import com.opensymphony.xwork2.util.location.Location; import com.opensymphony.xwork2.util.location.LocationUtils; -import com.opensymphony.xwork2.util.profiling.UtilTimerStack; import org.apache.commons.lang3.LocaleUtils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; @@ -583,7 +582,6 @@ public class Dispatcher { String timerKey = "Handling request from Dispatcher"; try { - UtilTimerStack.push(timerKey); String namespace = mapping.getNamespace(); String name = mapping.getName(); String method = mapping.getMethod(); @@ -623,8 +621,6 @@ public class Dispatcher { } else { throw new ServletException(e); } - } finally { - UtilTimerStack.pop(timerKey); } } diff --git a/core/src/main/java/org/apache/struts2/interceptor/ProfilingActivationInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ProfilingActivationInterceptor.java deleted file mode 100644 index d1a21964b..000000000 --- a/core/src/main/java/org/apache/struts2/interceptor/ProfilingActivationInterceptor.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.struts2.interceptor; - -import com.opensymphony.xwork2.ActionInvocation; -import com.opensymphony.xwork2.inject.Inject; -import com.opensymphony.xwork2.interceptor.AbstractInterceptor; -import com.opensymphony.xwork2.util.profiling.UtilTimerStack; -import org.apache.commons.lang3.BooleanUtils; -import org.apache.struts2.StrutsConstants; -import org.apache.struts2.dispatcher.Parameter; - -/** - * - * - * Allows profiling to be enabled or disabled via request parameters, when - * devMode is enabled. - * - * - * - * - * - * - *
    - *
  • profilingKey
    • - *
- * - * - * - * - * - * none - * - * - * - * 
- * 
- * // to change the profiling key
- * <action ...>
- *   ...
- *   <interceptor-ref name="profiling">
- *      <param name="profilingKey">profilingKey</param>
- *   </interceptor-ref>
- *   ...
- * </action>
- * 
- *
- * - * @version $Date$ $Id$ - */ -public class ProfilingActivationInterceptor extends AbstractInterceptor { - - private String profilingKey = "profiling"; - private boolean devMode; - - /** - * @return the profilingKey - */ - public String getProfilingKey() { - return profilingKey; - } - - /** - * @param profilingKey the profilingKey to set - */ - public void setProfilingKey(String profilingKey) { - this.profilingKey = profilingKey; - } - - @Inject(StrutsConstants.STRUTS_DEVMODE) - public void setDevMode(String mode) { - this.devMode = BooleanUtils.toBoolean(mode); - } - - @Override - public String intercept(ActionInvocation invocation) throws Exception { - if (devMode) { - Parameter val = invocation.getInvocationContext().getParameters().get(profilingKey); - if (val.isDefined()) { - String sval = val.getValue(); - boolean enable = BooleanUtils.toBoolean(sval); - UtilTimerStack.setActive(enable); - invocation.getInvocationContext().getParameters().remove(profilingKey); - } - } - return invocation.invoke(); - } -} diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 85f67bc9b..e3d3b072f 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -233,7 +233,6 @@ - diff --git a/core/src/test/java/com/opensymphony/xwork2/util/profiling/UtilTimerStackTest.java b/core/src/test/java/com/opensymphony/xwork2/util/profiling/UtilTimerStackTest.java deleted file mode 100644 index f7c0b71fb..000000000 --- a/core/src/test/java/com/opensymphony/xwork2/util/profiling/UtilTimerStackTest.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package com.opensymphony.xwork2.util.profiling; - -import junit.framework.TestCase; - -/** - * @author tmjee - * @version $Date$ $Id$ - */ -public class UtilTimerStackTest extends TestCase { - - protected String activateProp; - protected String minTimeProp; - - - public void testActivateInactivate() throws Exception { - UtilTimerStack.setActive(true); - assertTrue(UtilTimerStack.isActive()); - UtilTimerStack.setActive(false); - assertFalse(UtilTimerStack.isActive()); - } - - - public void testPushPop() throws Exception { - UtilTimerStack.push("p1"); - Thread.sleep(1050); - ProfilingTimerBean bean = UtilTimerStack.current.get(); - assertTrue(bean.startTime > 0); - UtilTimerStack.pop("p1"); - assertTrue(bean.totalTime > 1000); - } - - - public void testProfileCallback() throws Exception { - - MockProfilingBlock block = new MockProfilingBlock() { - @Override - public String performProfiling() throws Exception { - Thread.sleep(1050); - return "OK"; - } - }; - String result = UtilTimerStack.profile("p1", block); - assertEquals(result, "OK"); - assertNotNull(block.getProfilingTimerBean()); - assertTrue(block.getProfilingTimerBean().totalTime >= 1000); - - } - - - public void testProfileCallbackThrowsException() throws Exception { - try { - UtilTimerStack.profile("p1", - new UtilTimerStack.ProfilingBlock() { - public String doProfiling() throws Exception { - throw new RuntimeException("test"); - } - }); - fail("exception should have been thrown"); - } - catch (Exception e) { - assertTrue(true); - } - } - - - @Override - protected void setUp() throws Exception { - super.setUp(); - - activateProp = System.getProperty(UtilTimerStack.ACTIVATE_PROPERTY); - minTimeProp = System.getProperty(UtilTimerStack.MIN_TIME); - - System.setProperty(UtilTimerStack.ACTIVATE_PROPERTY, "true"); - UtilTimerStack.setActive(true); - System.setProperty(UtilTimerStack.MIN_TIME, "0"); - } - - - @Override - protected void tearDown() throws Exception { - - if (activateProp != null) { - System.setProperty(UtilTimerStack.ACTIVATE_PROPERTY, activateProp); - } else { - System.clearProperty(UtilTimerStack.ACTIVATE_PROPERTY); - } - if (minTimeProp != null) { - System.setProperty(UtilTimerStack.MIN_TIME, minTimeProp); - } else { - System.clearProperty(UtilTimerStack.ACTIVATE_PROPERTY); - } - - - activateProp = null; - minTimeProp = null; - - super.tearDown(); - } - - - public abstract class MockProfilingBlock implements UtilTimerStack.ProfilingBlock { - - private ProfilingTimerBean bean; - - public T doProfiling() throws Exception { - bean = UtilTimerStack.current.get(); - return performProfiling(); - } - - public ProfilingTimerBean getProfilingTimerBean() { - return bean; - } - - public abstract T performProfiling() throws Exception; - } -} - - diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java index 6fbfa4aae..a6ad038a4 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java @@ -24,7 +24,6 @@ import com.opensymphony.xwork2.config.entities.ActionConfig; import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.ValidationAware; -import com.opensymphony.xwork2.util.profiling.UtilTimerStack; import org.apache.commons.lang3.BooleanUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -164,8 +163,6 @@ public class RestActionInvocation extends DefaultActionInvocation { protected void processResult() throws Exception { String timerKey = "processResult: " + getResultCode(); try { - UtilTimerStack.push(timerKey); - HttpServletRequest request = ServletActionContext.getRequest(); HttpServletResponse response = ServletActionContext.getResponse(); @@ -191,8 +188,6 @@ public class RestActionInvocation extends DefaultActionInvocation { LOG.debug("Result not processed because the status code is not modified."); } - } finally { - UtilTimerStack.pop(timerKey); } } diff --git a/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsFreemarkerDecorator.java b/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsFreemarkerDecorator.java index 174ae2a81..50970f913 100644 --- a/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsFreemarkerDecorator.java +++ b/plugins/sitemesh/src/main/java/org/apache/struts2/sitemesh/OldDecorator2NewStrutsFreemarkerDecorator.java @@ -24,7 +24,6 @@ import com.opensymphony.sitemesh.compatability.Content2HTMLPage; import com.opensymphony.xwork2.ActionContext; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; -import com.opensymphony.xwork2.util.profiling.UtilTimerStack; import freemarker.template.Configuration; import freemarker.template.SimpleHash; import freemarker.template.Template; @@ -72,8 +71,6 @@ public class OldDecorator2NewStrutsFreemarkerDecorator extends OldDecorator2NewS } try { - UtilTimerStack.push(timerKey); - // get the configuration and template Configuration config = freemarkerManager.getConfiguration(servletContext); Template template = config.getTemplate(oldDecorator.getPage(), getLocale(ctx.getActionInvocation(), config)); // WW-1181 @@ -98,8 +95,6 @@ public class OldDecorator2NewStrutsFreemarkerDecorator extends OldDecorator2NewS String msg = "Error applying decorator to request: " + request.getRequestURL() + "?" + request.getQueryString() + " with message:" + e.getMessage(); LOG.error(msg, e); throw new ServletException(msg, e); - } finally { - UtilTimerStack.pop(timerKey); } } From 6c0d719a8a2d7f56b00f9373ac1ea1b660c2310b Mon Sep 17 00:00:00 2001 From: Stefaan Dutry Date: Mon, 21 May 2018 21:38:12 +0200 Subject: [PATCH 4/6] fix RestActionInvocation --- .../java/org/apache/struts2/rest/RestActionInvocation.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java index a6ad038a4..f53e55cd6 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java @@ -187,8 +187,7 @@ public class RestActionInvocation extends DefaultActionInvocation { } else { LOG.debug("Result not processed because the status code is not modified."); } - - } + } finally {} } /** From db0f5b50bc4ebe7a478c2d9efea180400e116fc1 Mon Sep 17 00:00:00 2001 From: Stefaan Dutry Date: Mon, 21 May 2018 21:53:29 +0200 Subject: [PATCH 5/6] remove ProfilingTimerBean Part of removing the profiling layer Issue: * WW-4779 --- .../util/profiling/ProfilingTimerBean.java | 129 ------------------ .../xwork2/util/profiling/package.html | 21 --- .../profiling/ProfilingTimerBeanTest.java | 127 ----------------- 3 files changed, 277 deletions(-) delete mode 100644 core/src/main/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBean.java delete mode 100644 core/src/main/java/com/opensymphony/xwork2/util/profiling/package.html delete mode 100644 core/src/test/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBeanTest.java diff --git a/core/src/main/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBean.java b/core/src/main/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBean.java deleted file mode 100644 index 3706d2843..000000000 --- a/core/src/main/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBean.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/* - * Copyright (c) 2002-2003, Atlassian Software Systems Pty Ltd All rights reserved. - * - * Redistribution and use in source and binary forms, with or without modification, - * are permitted provided that the following conditions are met: - * - * * Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation and/or - * other materials provided with the distribution. - * * Neither the name of Atlassian Software Systems Pty Ltd nor the names of - * its contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR - * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package com.opensymphony.xwork2.util.profiling; - -import java.util.ArrayList; -import java.util.List; - -/** - * Bean to contain information about the pages profiled - * - * @author Mike Cannon-Brookes - * @author Scott Farquhar - * @version $Date$ $Id$ - * - * @deprecated will be dropped with next major release (2.6) - */ -@Deprecated -public class ProfilingTimerBean implements java.io.Serializable { - - private static final long serialVersionUID = -6180672043920208784L; - - List children = new ArrayList<>(); - ProfilingTimerBean parent = null; - - String resource; - - long startTime; - long totalTime; - - public ProfilingTimerBean(String resource) { - this.resource = resource; - } - - protected void addParent(ProfilingTimerBean parent) { - this.parent = parent; - } - - public ProfilingTimerBean getParent() { - return parent; - } - - - public void addChild(ProfilingTimerBean child) { - children.add(child); - child.addParent(this); - } - - - public void setStartTime() { - this.startTime = System.currentTimeMillis(); - } - - public void setEndTime() { - this.totalTime = System.currentTimeMillis() - startTime; - } - - public String getResource() { - return resource; - } - - /** - * @param minTime minimum time - * @return a formatted string representing all the methods that took longer than a specified time. - */ - public String getPrintable(long minTime) { - return getPrintable("", minTime); - } - - protected String getPrintable(String indent, long minTime) { - //only print the value if we are larger or equal to the min time. - if (totalTime >= minTime) { - StringBuilder buffer = new StringBuilder(); - buffer.append(indent); - buffer.append("[" + totalTime + "ms] - " + resource); - buffer.append("\n"); - - for (ProfilingTimerBean aChildren : children) { - buffer.append((aChildren).getPrintable(indent + " ", minTime)); - } - - return buffer.toString(); - } else - return ""; - } -} - diff --git a/core/src/main/java/com/opensymphony/xwork2/util/profiling/package.html b/core/src/main/java/com/opensymphony/xwork2/util/profiling/package.html deleted file mode 100644 index 766d4b870..000000000 --- a/core/src/main/java/com/opensymphony/xwork2/util/profiling/package.html +++ /dev/null @@ -1,21 +0,0 @@ - -Classes to enable profiling of action execution. diff --git a/core/src/test/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBeanTest.java b/core/src/test/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBeanTest.java deleted file mode 100644 index 443f469fd..000000000 --- a/core/src/test/java/com/opensymphony/xwork2/util/profiling/ProfilingTimerBeanTest.java +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package com.opensymphony.xwork2.util.profiling; - -import junit.framework.TestCase; - -/** - * - * @author tm_jee - * @version $Date$ $Id$ - */ -public class ProfilingTimerBeanTest extends TestCase { - - public void testAddChild() throws Exception { - ProfilingTimerBean bean0 = new ProfilingTimerBean("bean0"); - ProfilingTimerBean bean1 = new ProfilingTimerBean("bean1"); - ProfilingTimerBean bean2 = new ProfilingTimerBean("bean2"); - ProfilingTimerBean bean3 = new ProfilingTimerBean("bean3"); - ProfilingTimerBean bean4 = new ProfilingTimerBean("bean4"); - ProfilingTimerBean bean5 = new ProfilingTimerBean("bean5"); - ProfilingTimerBean bean6 = new ProfilingTimerBean("bean6"); - ProfilingTimerBean bean7 = new ProfilingTimerBean("bean7"); - ProfilingTimerBean bean8 = new ProfilingTimerBean("bean8"); - - /* bean0 - * + bean1 - * + bean2 - * + bean3 - * + bean4 - * + bean5 - * + bean6 - * +bean7 - * + bean8 - */ - - bean0.addChild(bean1); - bean0.addChild(bean3); - bean0.addChild(bean8); - - bean1.addChild(bean2); - - bean3.addChild(bean4); - bean3.addChild(bean7); - - bean4.addChild(bean5); - - bean5.addChild(bean6); - - - // bean0 - assertNull(bean0.getParent()); - assertEquals(bean0.children.size(), 3); - assertTrue(bean0.children.contains(bean1)); - assertTrue(bean0.children.contains(bean3)); - assertTrue(bean0.children.contains(bean8)); - - // bean1 - assertEquals(bean1.getParent(), bean0); - assertEquals(bean1.children.size(), 1); - assertTrue(bean1.children.contains(bean2)); - - // bean2 - assertEquals(bean2.getParent(), bean1); - assertEquals(bean2.children.size(), 0); - - // bean3 - assertEquals(bean3.getParent(), bean0); - assertEquals(bean3.children.size(), 2); - assertTrue(bean3.children.contains(bean4)); - assertTrue(bean3.children.contains(bean7)); - - // bean4 - assertEquals(bean4.getParent(), bean3); - assertEquals(bean4.children.size(), 1); - assertTrue(bean4.children.contains(bean5)); - - // bean5 - assertEquals(bean5.getParent(), bean4); - assertEquals(bean5.children.size(), 1); - assertTrue(bean5.children.contains(bean6)); - - // bean6 - assertEquals(bean6.getParent(), bean5); - assertEquals(bean6.children.size(), 0); - - // bean7 - assertEquals(bean7.getParent(), bean3); - assertEquals(bean7.children.size(), 0); - - // bean8 - assertEquals(bean8.getParent(), bean0); - assertEquals(bean8.children.size(), 0); - } - - public void testTime() throws Exception { - ProfilingTimerBean bean0 = new ProfilingTimerBean("bean0"); - bean0.setStartTime(); - Thread.sleep(1050); - bean0.setEndTime(); - assertTrue(bean0.totalTime >= 1000); - } - - public void testPrint() throws Exception { - ProfilingTimerBean bean0 = new ProfilingTimerBean("bean0"); - bean0.setStartTime(); - Thread.sleep(1050); - bean0.setEndTime(); - assertEquals(bean0.getPrintable(2000), ""); - assertTrue(bean0.getPrintable(500).length() > 0); - } -} From 38c304696beb13b045639072c9455101de65a77c Mon Sep 17 00:00:00 2001 From: Stefaan Dutry Date: Mon, 21 May 2018 22:48:45 +0200 Subject: [PATCH 6/6] further clean up after rmoving profiling layer * removed empty catch blocks * removed no longer used Strings --- .../xwork2/DefaultActionInvocation.java | 122 ++++++++---------- .../xwork2/DefaultActionProxy.java | 30 ++--- .../apache/struts2/dispatcher/Dispatcher.java | 1 - .../struts2/rest/RestActionInvocation.java | 43 +++--- 4 files changed, 89 insertions(+), 107 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java index 1c40be511..6514ea9bc 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java @@ -236,65 +236,57 @@ public class DefaultActionInvocation implements ActionInvocation { * @throws ConfigurationException If no result can be found with the returned code */ public String invoke() throws Exception { - String profileKey = "invoke: "; - try { - if (executed) { - throw new IllegalStateException("Action has already executed"); - } + if (executed) { + throw new IllegalStateException("Action has already executed"); + } - if (asyncManager == null || !asyncManager.hasAsyncActionResult()) { - if (interceptors.hasNext()) { - final InterceptorMapping interceptorMapping = interceptors.next(); - String interceptorMsg = "interceptorMapping: " + interceptorMapping.getName(); - try { - Interceptor interceptor = interceptorMapping.getInterceptor(); - if (interceptor instanceof WithLazyParams) { - interceptor = lazyParamInjector.injectParams(interceptor, interceptorMapping.getParams(), invocationContext); - } - resultCode = interceptor.intercept(DefaultActionInvocation.this); - } finally {} - } else { - resultCode = invokeActionOnly(); + if (asyncManager == null || !asyncManager.hasAsyncActionResult()) { + if (interceptors.hasNext()) { + final InterceptorMapping interceptorMapping = interceptors.next(); + String interceptorMsg = "interceptorMapping: " + interceptorMapping.getName(); + Interceptor interceptor = interceptorMapping.getInterceptor(); + if (interceptor instanceof WithLazyParams) { + interceptor = lazyParamInjector.injectParams(interceptor, interceptorMapping.getParams(), invocationContext); } + resultCode = interceptor.intercept(DefaultActionInvocation.this); } else { - Object asyncActionResult = asyncManager.getAsyncActionResult(); - if (asyncActionResult instanceof Throwable) { - throw new Exception((Throwable) asyncActionResult); - } - asyncAction = null; - resultCode = saveResult(proxy.getConfig(), asyncActionResult); + resultCode = invokeActionOnly(); } - - if (asyncManager == null || asyncAction == null) { - // this is needed because the result will be executed, then control will return to the Interceptor, which will - // return above and flow through again - if (!executed) { - if (preResultListeners != null) { - LOG.trace("Executing PreResultListeners for result [{}]", result); - - for (Object preResultListener : preResultListeners) { - PreResultListener listener = (PreResultListener) preResultListener; - - String _profileKey = "preResultListener: "; - try { - listener.beforeResult(this, resultCode); - } finally {} - } - } - - // now execute the result, if we're supposed to - if (proxy.getExecuteResult()) { - executeResult(); - } - - executed = true; - } - } else { - asyncManager.invokeAsyncAction(asyncAction); + } else { + Object asyncActionResult = asyncManager.getAsyncActionResult(); + if (asyncActionResult instanceof Throwable) { + throw new Exception((Throwable) asyncActionResult); } + asyncAction = null; + resultCode = saveResult(proxy.getConfig(), asyncActionResult); + } - return resultCode; - } finally {} + if (asyncManager == null || asyncAction == null) { + // this is needed because the result will be executed, then control will return to the Interceptor, which will + // return above and flow through again + if (!executed) { + if (preResultListeners != null) { + LOG.trace("Executing PreResultListeners for result [{}]", result); + + for (Object preResultListener : preResultListeners) { + PreResultListener listener = (PreResultListener) preResultListener; + + listener.beforeResult(this, resultCode); + } + } + + // now execute the result, if we're supposed to + if (proxy.getExecuteResult()) { + executeResult(); + } + + executed = true; + } + } else { + asyncManager.invokeAsyncAction(asyncAction); + } + + return resultCode; } public String invokeActionOnly() throws Exception { @@ -325,7 +317,7 @@ public class DefaultActionInvocation implements ActionInvocation { gripe += (((" -- " + e.getMessage()) != null) ? e.getMessage() : " [no message in exception]"); throw new XWorkException(gripe, e, proxy.getConfig()); - } finally {} + } if (actionEventListener != null) { action = actionEventListener.prepare(action, stack); @@ -374,18 +366,16 @@ public class DefaultActionInvocation implements ActionInvocation { result = createResult(); String timerKey = "executeResult: " + getResultCode(); - try { - if (result != null) { - result.execute(this); - } else if (resultCode != null && !Action.NONE.equals(resultCode)) { - throw new ConfigurationException("No result defined for action " + getAction().getClass().getName() - + " and result " + getResultCode(), proxy.getConfig()); - } else { - if (LOG.isDebugEnabled()) { - LOG.debug("No result returned for action {} at {}", getAction().getClass().getName(), proxy.getConfig().getLocation()); - } + if (result != null) { + result.execute(this); + } else if (resultCode != null && !Action.NONE.equals(resultCode)) { + throw new ConfigurationException("No result defined for action " + getAction().getClass().getName() + + " and result " + getResultCode(), proxy.getConfig()); + } else { + if (LOG.isDebugEnabled()) { + LOG.debug("No result returned for action {} at {}", getAction().getClass().getName(), proxy.getConfig().getLocation()); } - } finally {} + } } public void init(ActionProxy proxy) { @@ -477,7 +467,7 @@ public class DefaultActionInvocation implements ActionInvocation { } else { throw e; } - } finally {} + } } /** diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java index d1bfe320c..6e0458e64 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java @@ -149,7 +149,6 @@ public class DefaultActionProxy implements ActionProxy, Serializable { String retCode = null; - String profileKey = "execute: "; try { retCode = invocation.invoke(); } finally { @@ -179,25 +178,22 @@ public class DefaultActionProxy implements ActionProxy, Serializable { } protected void prepare() { - String profileKey = "create DefaultActionProxy: "; - try { - config = configuration.getRuntimeConfiguration().getActionConfig(namespace, actionName); + config = configuration.getRuntimeConfiguration().getActionConfig(namespace, actionName); - if (config == null && unknownHandlerManager.hasUnknownHandlers()) { - config = unknownHandlerManager.handleUnknownAction(namespace, actionName); - } - if (config == null) { - throw new ConfigurationException(getErrorMessage()); - } + if (config == null && unknownHandlerManager.hasUnknownHandlers()) { + config = unknownHandlerManager.handleUnknownAction(namespace, actionName); + } + if (config == null) { + throw new ConfigurationException(getErrorMessage()); + } - resolveMethod(); + resolveMethod(); - if (config.isAllowedMethod(method)) { - invocation.init(this); - } else { - throw new ConfigurationException(prepareNotAllowedErrorMessage()); - } - } finally {} + if (config.isAllowedMethod(method)) { + invocation.init(this); + } else { + throw new ConfigurationException(prepareNotAllowedErrorMessage()); + } } protected String prepareNotAllowedErrorMessage() { diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index c372410de..67a97547c 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -580,7 +580,6 @@ public class Dispatcher { extraContext.put(ActionContext.VALUE_STACK, valueStackFactory.createValueStack(stack)); } - String timerKey = "Handling request from Dispatcher"; try { String namespace = mapping.getNamespace(); String name = mapping.getName(); diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java index f53e55cd6..286c80db1 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionInvocation.java @@ -161,33 +161,30 @@ public class RestActionInvocation extends DefaultActionInvocation { } protected void processResult() throws Exception { - String timerKey = "processResult: " + getResultCode(); - try { - HttpServletRequest request = ServletActionContext.getRequest(); - HttpServletResponse response = ServletActionContext.getResponse(); + HttpServletRequest request = ServletActionContext.getRequest(); + HttpServletResponse response = ServletActionContext.getResponse(); - // Select the target - selectTarget(); + // Select the target + selectTarget(); - // Get the httpHeaders - if (httpHeaders == null) { - httpHeaders = new DefaultHttpHeaders(resultCode); - } + // Get the httpHeaders + if (httpHeaders == null) { + httpHeaders = new DefaultHttpHeaders(resultCode); + } - // Apply headers - if (!hasErrors) { - httpHeaders.apply(request, response, target); - } else { - disableCatching(response); - } + // Apply headers + if (!hasErrors) { + httpHeaders.apply(request, response, target); + } else { + disableCatching(response); + } - // Don't return content on a not modified - if (httpHeaders.getStatus() != HttpServletResponse.SC_NOT_MODIFIED ) { - executeResult(); - } else { - LOG.debug("Result not processed because the status code is not modified."); - } - } finally {} + // Don't return content on a not modified + if (httpHeaders.getStatus() != HttpServletResponse.SC_NOT_MODIFIED ) { + executeResult(); + } else { + LOG.debug("Result not processed because the status code is not modified."); + } } /**