diff --git a/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java b/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java index 4182342a26..6eb5de182d 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/ServletRequestAttributes.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -108,15 +108,24 @@ public class ServletRequestAttributes extends AbstractRequestAttributes { */ protected final HttpSession getSession(boolean allowCreate) { if (isRequestActive()) { - return this.request.getSession(allowCreate); + HttpSession session = this.request.getSession(allowCreate); + this.session = session; + return session; } else { // Access through stored session reference, if any... - if (this.session == null && allowCreate) { - throw new IllegalStateException( - "No session found and request already completed - cannot create new session!"); + HttpSession session = this.session; + if (session == null) { + if (allowCreate) { + throw new IllegalStateException( + "No session found and request already completed - cannot create new session!"); + } + else { + session = this.request.getSession(false); + this.session = session; + } } - return this.session; + return session; } } @@ -251,25 +260,26 @@ public class ServletRequestAttributes extends AbstractRequestAttributes { */ @Override protected void updateAccessedSessionAttributes() { - // Store session reference for access after request completion. - this.session = this.request.getSession(false); - // Update all affected session attributes. - if (this.session != null) { - try { - for (Map.Entry entry : this.sessionAttributesToUpdate.entrySet()) { - String name = entry.getKey(); - Object newValue = entry.getValue(); - Object oldValue = this.session.getAttribute(name); - if (oldValue == newValue && !isImmutableSessionAttribute(name, newValue)) { - this.session.setAttribute(name, newValue); + if (!this.sessionAttributesToUpdate.isEmpty()) { + // Update all affected session attributes. + HttpSession session = getSession(false); + if (session != null) { + try { + for (Map.Entry entry : this.sessionAttributesToUpdate.entrySet()) { + String name = entry.getKey(); + Object newValue = entry.getValue(); + Object oldValue = session.getAttribute(name); + if (oldValue == newValue && !isImmutableSessionAttribute(name, newValue)) { + session.setAttribute(name, newValue); + } } } + catch (IllegalStateException ex) { + // Session invalidated - shouldn't usually happen. + } } - catch (IllegalStateException ex) { - // Session invalidated - shouldn't usually happen. - } + this.sessionAttributesToUpdate.clear(); } - this.sessionAttributesToUpdate.clear(); } /** diff --git a/spring-web/src/test/java/org/springframework/web/context/request/ServletRequestAttributesTests.java b/spring-web/src/test/java/org/springframework/web/context/request/ServletRequestAttributesTests.java index ef1afaad9f..603e264279 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/ServletRequestAttributesTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/ServletRequestAttributesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -78,8 +78,7 @@ public class ServletRequestAttributesTests { request.setSession(session); ServletRequestAttributes attrs = new ServletRequestAttributes(request); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY)); } @Test @@ -89,11 +88,11 @@ public class ServletRequestAttributesTests { MockHttpServletRequest request = new MockHttpServletRequest(); request.setSession(session); ServletRequestAttributes attrs = new ServletRequestAttributes(request); + assertSame(VALUE, attrs.getAttribute(KEY, RequestAttributes.SCOPE_SESSION)); attrs.requestCompleted(); request.close(); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY)); } @Test @@ -104,8 +103,7 @@ public class ServletRequestAttributesTests { request.setSession(session); ServletRequestAttributes attrs = new ServletRequestAttributes(request); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_GLOBAL_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY)); } @Test @@ -115,11 +113,11 @@ public class ServletRequestAttributesTests { MockHttpServletRequest request = new MockHttpServletRequest(); request.setSession(session); ServletRequestAttributes attrs = new ServletRequestAttributes(request); + assertSame(VALUE, attrs.getAttribute(KEY, RequestAttributes.SCOPE_GLOBAL_SESSION)); attrs.requestCompleted(); request.close(); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_GLOBAL_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY)); } @Test diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/PortletRequestAttributes.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/PortletRequestAttributes.java index c1f5b1eb83..936dfdec33 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/PortletRequestAttributes.java +++ b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/context/PortletRequestAttributes.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -107,15 +107,24 @@ public class PortletRequestAttributes extends AbstractRequestAttributes { */ protected final PortletSession getSession(boolean allowCreate) { if (isRequestActive()) { - return this.request.getPortletSession(allowCreate); + PortletSession session = this.request.getPortletSession(allowCreate); + this.session = session; + return session; } else { // Access through stored session reference, if any... - if (this.session == null && allowCreate) { - throw new IllegalStateException( - "No session found and request already completed - cannot create new session!"); + PortletSession session = this.session; + if (session == null) { + if (allowCreate) { + throw new IllegalStateException( + "No session found and request already completed - cannot create new session!"); + } + else { + session = this.request.getPortletSession(false); + this.session = session; + } } - return this.session; + return session; } } @@ -147,9 +156,7 @@ public class PortletRequestAttributes extends AbstractRequestAttributes { return value; } } - else { - return null; - } + return null; } } @@ -217,9 +224,7 @@ public class PortletRequestAttributes extends AbstractRequestAttributes { return StringUtils.toStringArray(session.getAttributeNames()); } } - else { - return new String[0]; - } + return new String[0]; } } @@ -263,32 +268,34 @@ public class PortletRequestAttributes extends AbstractRequestAttributes { */ @Override protected void updateAccessedSessionAttributes() { - this.session = this.request.getPortletSession(false); - if (this.session != null) { - try { - for (Map.Entry entry : this.sessionAttributesToUpdate.entrySet()) { - String name = entry.getKey(); - Object newValue = entry.getValue(); - Object oldValue = this.session.getAttribute(name); - if (oldValue == newValue) { - this.session.setAttribute(name, newValue); + if (!this.sessionAttributesToUpdate.isEmpty() || !this.globalSessionAttributesToUpdate.isEmpty()) { + PortletSession session = getSession(false); + if (session != null) { + try { + for (Map.Entry entry : this.sessionAttributesToUpdate.entrySet()) { + String name = entry.getKey(); + Object newValue = entry.getValue(); + Object oldValue = session.getAttribute(name); + if (oldValue == newValue) { + session.setAttribute(name, newValue); + } + } + for (Map.Entry entry : this.globalSessionAttributesToUpdate.entrySet()) { + String name = entry.getKey(); + Object newValue = entry.getValue(); + Object oldValue = session.getAttribute(name, PortletSession.APPLICATION_SCOPE); + if (oldValue == newValue) { + session.setAttribute(name, newValue, PortletSession.APPLICATION_SCOPE); + } } } - for (Map.Entry entry : this.globalSessionAttributesToUpdate.entrySet()) { - String name = entry.getKey(); - Object newValue = entry.getValue(); - Object oldValue = this.session.getAttribute(name, PortletSession.APPLICATION_SCOPE); - if (oldValue == newValue) { - this.session.setAttribute(name, newValue, PortletSession.APPLICATION_SCOPE); - } + catch (IllegalStateException ex) { + // Session invalidated - shouldn't usually happen. } } - catch (IllegalStateException ex) { - // Session invalidated - shouldn't usually happen. - } + this.sessionAttributesToUpdate.clear(); + this.globalSessionAttributesToUpdate.clear(); } - this.sessionAttributesToUpdate.clear(); - this.globalSessionAttributesToUpdate.clear(); } /** diff --git a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/context/PortletRequestAttributesTests.java b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/context/PortletRequestAttributesTests.java index 4875a2d695..40237e8182 100644 --- a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/context/PortletRequestAttributesTests.java +++ b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/context/PortletRequestAttributesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ package org.springframework.web.portlet.context; import java.io.Serializable; import javax.portlet.PortletRequest; +import javax.portlet.PortletSession; import org.junit.Test; @@ -37,12 +38,12 @@ public class PortletRequestAttributesTests { private static final String KEY = "ThatThingThatThing"; - @SuppressWarnings("serial") - private static final Serializable VALUE = new Serializable() { }; + private static final Serializable VALUE = new Serializable() { + }; - @Test(expected=IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) public void testCtorRejectsNullArg() throws Exception { new PortletRequestAttributes(null); } @@ -85,53 +86,51 @@ public class PortletRequestAttributesTests { @Test public void testSetSessionScopedAttribute() throws Exception { MockPortletSession session = new MockPortletSession(); - session.setAttribute(KEY, VALUE); + session.setAttribute(KEY, VALUE, PortletSession.PORTLET_SCOPE); MockPortletRequest request = new MockPortletRequest(); request.setSession(session); PortletRequestAttributes attrs = new PortletRequestAttributes(request); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY, PortletSession.PORTLET_SCOPE)); } @Test public void testSetSessionScopedAttributeAfterCompletion() throws Exception { MockPortletSession session = new MockPortletSession(); - session.setAttribute(KEY, VALUE); + session.setAttribute(KEY, VALUE, PortletSession.PORTLET_SCOPE); MockPortletRequest request = new MockPortletRequest(); request.setSession(session); PortletRequestAttributes attrs = new PortletRequestAttributes(request); + assertSame(VALUE, attrs.getAttribute(KEY, RequestAttributes.SCOPE_SESSION)); attrs.requestCompleted(); request.close(); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY, PortletSession.PORTLET_SCOPE)); } @Test public void testSetGlobalSessionScopedAttribute() throws Exception { MockPortletSession session = new MockPortletSession(); - session.setAttribute(KEY, VALUE); + session.setAttribute(KEY, VALUE, PortletSession.APPLICATION_SCOPE); MockPortletRequest request = new MockPortletRequest(); request.setSession(session); PortletRequestAttributes attrs = new PortletRequestAttributes(request); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_GLOBAL_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY, PortletSession.APPLICATION_SCOPE)); } @Test public void testSetGlobalSessionScopedAttributeAfterCompletion() throws Exception { MockPortletSession session = new MockPortletSession(); - session.setAttribute(KEY, VALUE); + session.setAttribute(KEY, VALUE, PortletSession.APPLICATION_SCOPE); MockPortletRequest request = new MockPortletRequest(); request.setSession(session); PortletRequestAttributes attrs = new PortletRequestAttributes(request); + assertSame(VALUE, attrs.getAttribute(KEY, RequestAttributes.SCOPE_GLOBAL_SESSION)); attrs.requestCompleted(); request.close(); attrs.setAttribute(KEY, VALUE, RequestAttributes.SCOPE_GLOBAL_SESSION); - Object value = session.getAttribute(KEY); - assertSame(VALUE, value); + assertSame(VALUE, session.getAttribute(KEY, PortletSession.APPLICATION_SCOPE)); } @Test