From e9a4bcd776e82afb135091f7ad48ada90bf5d2e6 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 31 Jan 2018 07:21:35 +0100 Subject: [PATCH 1/4] WW-4741 Does not create session if it doesn't exist --- .../struts2/interceptor/I18nInterceptor.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java index 502c21bd0..ba4993660 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java @@ -281,15 +281,16 @@ public class I18nInterceptor extends AbstractInterceptor { @Override public Locale store(ActionInvocation invocation, Locale locale) { - //save it in session - Map session = invocation.getInvocationContext().getSession(); + LOG.debug("Do not create session if it doesn't exist"); + HttpSession session = ServletActionContext.getRequest().getSession(false); if (session != null) { - String sessionId = ServletActionContext.getRequest().getSession().getId(); + String sessionId = session.getId(); synchronized (sessionId.intern()) { - session.put(attributeName, locale); + invocation.getInvocationContext().getSession().put(attributeName, locale); } } + return locale; } @@ -298,19 +299,15 @@ public class I18nInterceptor extends AbstractInterceptor { Locale locale = null; LOG.debug("Checks session for saved locale"); - Map session = invocation.getInvocationContext().getSession(); + HttpSession session = ServletActionContext.getRequest().getSession(false); if (session != null) { - //[WW-4741] Do not force session creation while this is a read operation - HttpSession httpSession = ServletActionContext.getRequest().getSession(false); - if(null != httpSession) { - String sessionId = httpSession.getId(); - synchronized (sessionId.intern()) { - Object sessionLocale = session.get(attributeName); - if (sessionLocale != null && sessionLocale instanceof Locale) { - locale = (Locale) sessionLocale; - LOG.debug("Applied session locale: {}", locale); - } + String sessionId = session.getId(); + synchronized (sessionId.intern()) { + Object sessionLocale = invocation.getInvocationContext().getSession().get(attributeName); + if (sessionLocale != null && sessionLocale instanceof Locale) { + locale = (Locale) sessionLocale; + LOG.debug("Applied session locale: {}", locale); } } } From 92e931d169ccd35cf3a8456451a74901979aed76 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 31 Jan 2018 07:21:53 +0100 Subject: [PATCH 2/4] WW-4741 Improves tests to cover the new logic --- .../struts2/interceptor/I18nInterceptorTest.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java index 0a1942fb7..9159633fe 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java @@ -31,6 +31,7 @@ import org.apache.struts2.dispatcher.HttpParameters; import org.easymock.EasyMock; import org.easymock.IArgumentMatcher; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpSession; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletResponse; @@ -45,14 +46,20 @@ public class I18nInterceptorTest extends TestCase { private ActionInvocation mai; private ActionContext ac; private Map session; + private MockHttpServletRequest request; public void testEmptyParamAndSession() throws Exception { interceptor.intercept(mai); } public void testNoSession() throws Exception { - ac.setSession(null); - interceptor.intercept(mai); + request.setSession(null); + try { + interceptor.intercept(mai); + assertTrue(true); + } catch (Exception ignore) { + fail("Shouldn't throw any exception!"); + } } public void testDefaultLocale() throws Exception { @@ -235,7 +242,9 @@ public class I18nInterceptorTest extends TestCase { ac = new ActionContext(ctx); ServletActionContext.setContext(ac); - ServletActionContext.setRequest(new MockHttpServletRequest()); + request = new MockHttpServletRequest(); + request.setSession(new MockHttpSession()); + ServletActionContext.setRequest(request); Action action = new Action() { public String execute() throws Exception { From b5cfccace3e7044dccfa3b73b80adc657a29ddad Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 31 Jan 2018 13:33:44 +0330 Subject: [PATCH 3/4] test no session but with locale parameter See also WW-4741 --- .../struts2/interceptor/I18nInterceptor.java | 3 ++- .../interceptor/I18nInterceptorTest.java | 21 ++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java index ba4993660..682bbe6d7 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java @@ -281,7 +281,6 @@ public class I18nInterceptor extends AbstractInterceptor { @Override public Locale store(ActionInvocation invocation, Locale locale) { - LOG.debug("Do not create session if it doesn't exist"); HttpSession session = ServletActionContext.getRequest().getSession(false); if (session != null) { @@ -289,6 +288,8 @@ public class I18nInterceptor extends AbstractInterceptor { synchronized (sessionId.intern()) { invocation.getInvocationContext().getSession().put(attributeName, locale); } + } else { + LOG.debug("session creation avoided as it doesn't exist already"); } return locale; diff --git a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java index 9159633fe..f31f96c3f 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java @@ -52,7 +52,7 @@ public class I18nInterceptorTest extends TestCase { interceptor.intercept(mai); } - public void testNoSession() throws Exception { + public void testNoSessionNoLocale() throws Exception { request.setSession(null); try { interceptor.intercept(mai); @@ -60,6 +60,25 @@ public class I18nInterceptorTest extends TestCase { } catch (Exception ignore) { fail("Shouldn't throw any exception!"); } + + assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed + + assertNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should not be stored here + } + + public void testNoSessionButLocale() throws Exception { + prepare(I18nInterceptor.DEFAULT_PARAMETER, "da_DK"); //prevents shouldStore to being false + request.setSession(null); + try { + interceptor.intercept(mai); + assertTrue(true); + } catch (Exception ignore) { + fail("Shouldn't throw any exception!"); + } + + assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed + + assertNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should not be stored here } public void testDefaultLocale() throws Exception { From caeea2145f3e9e834e9854b2b160c2c3cf0da609 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 31 Jan 2018 13:57:21 +0330 Subject: [PATCH 4/4] test not create when no session See also WW-4741 --- .../struts2/interceptor/I18nInterceptorTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java index f31f96c3f..fcc0dd11b 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java @@ -61,9 +61,10 @@ public class I18nInterceptorTest extends TestCase { fail("Shouldn't throw any exception!"); } - assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed - - assertNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should not be stored here + assertFalse("should have been removed", + mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); + assertNull("should not be created", request.getSession(false)); + assertNull("should not be stored here", session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); } public void testNoSessionButLocale() throws Exception { @@ -76,9 +77,10 @@ public class I18nInterceptorTest extends TestCase { fail("Shouldn't throw any exception!"); } - assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed - - assertNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should not be stored here + assertFalse("should have been removed", + mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); + assertNull("should not be created", request.getSession(false)); + assertNull("should not be stored here", session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); } public void testDefaultLocale() throws Exception {