From 09ab40939146f40aeb79162b738fc2f309fa1563 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sat, 20 Feb 2016 20:32:53 +0100 Subject: [PATCH] DATACMNS-820 - Polishing. Fixed indentation to use tabs instead of spaces. Tweaked structure of if-clauses in PropertyAccessingMethodInterceptor to reduce nesting. Made helper method static. Restructured test cases slightly and used ExpectedException rule to verify exceptions. Moved newly introduced test methods more to the end of the file (new tests last). Added author and copyright year extensions where necessary. Original pull request: #155. --- .../PropertyAccessingMethodInterceptor.java | 19 ++-- .../SpelAwareProxyProjectionFactory.java | 3 +- ...tyAccessingMethodInterceptorUnitTests.java | 88 ++++++++++--------- ...lAwareProxyProjectionFactoryUnitTests.java | 39 ++++---- 4 files changed, 75 insertions(+), 74 deletions(-) diff --git a/src/main/java/org/springframework/data/projection/PropertyAccessingMethodInterceptor.java b/src/main/java/org/springframework/data/projection/PropertyAccessingMethodInterceptor.java index de2c0d0c5..f1e9cd8ee 100644 --- a/src/main/java/org/springframework/data/projection/PropertyAccessingMethodInterceptor.java +++ b/src/main/java/org/springframework/data/projection/PropertyAccessingMethodInterceptor.java @@ -67,20 +67,19 @@ class PropertyAccessingMethodInterceptor implements MethodInterceptor { throw new IllegalStateException("Invoked method is not a property accessor!"); } - if (isSetterMethod(method, descriptor)) { - if (invocation.getArguments().length != 1) { - throw new IllegalStateException("Invoked setter method requires exactly one argument!"); - } + if (!isSetterMethod(method, descriptor)) { + return target.getPropertyValue(descriptor.getName()); + } - target.setPropertyValue(descriptor.getName(), invocation.getArguments()[0]); - return null; - } + if (invocation.getArguments().length != 1) { + throw new IllegalStateException("Invoked setter method requires exactly one argument!"); + } - return target.getPropertyValue(descriptor.getName()); + target.setPropertyValue(descriptor.getName(), invocation.getArguments()[0]); + return null; } - private boolean isSetterMethod(Method method, PropertyDescriptor descriptor) { + private static boolean isSetterMethod(Method method, PropertyDescriptor descriptor) { return method.equals(descriptor.getWriteMethod()); } - } diff --git a/src/main/java/org/springframework/data/projection/SpelAwareProxyProjectionFactory.java b/src/main/java/org/springframework/data/projection/SpelAwareProxyProjectionFactory.java index b3f698ff9..99892c4cc 100644 --- a/src/main/java/org/springframework/data/projection/SpelAwareProxyProjectionFactory.java +++ b/src/main/java/org/springframework/data/projection/SpelAwareProxyProjectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-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. @@ -36,6 +36,7 @@ import org.springframework.util.ReflectionUtils; * * @author Oliver Gierke * @author Thomas Darimont + * @author Mark Paluch * @since 1.10 */ public class SpelAwareProxyProjectionFactory extends ProxyProjectionFactory implements BeanFactoryAware { diff --git a/src/test/java/org/springframework/data/projection/PropertyAccessingMethodInterceptorUnitTests.java b/src/test/java/org/springframework/data/projection/PropertyAccessingMethodInterceptorUnitTests.java index 163589ef1..e240a2a79 100644 --- a/src/test/java/org/springframework/data/projection/PropertyAccessingMethodInterceptorUnitTests.java +++ b/src/test/java/org/springframework/data/projection/PropertyAccessingMethodInterceptorUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-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. @@ -32,6 +32,7 @@ import org.springframework.beans.NotWritablePropertyException; * Unit tests for {@link PropertyAccessingMethodInterceptor}. * * @author Oliver Gierke + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class PropertyAccessingMethodInterceptorUnitTests { @@ -63,67 +64,68 @@ public class PropertyAccessingMethodInterceptorUnitTests { new PropertyAccessingMethodInterceptor(new Source()).invoke(invocation); } - /** - * @see DATACMNS-820 - */ - @Test - public void triggersWritePropertyAccessOnTarget() throws Throwable { + /** + * @see DATAREST-221 + */ + @Test + public void forwardsObjectMethodInvocation() throws Throwable { - Source source = new Source(); - source.firstname = "Dave"; + when(invocation.getMethod()).thenReturn(Object.class.getMethod("toString")); - when(invocation.getMethod()).thenReturn(Projection.class.getMethod("setFirstname", String.class)); - when(invocation.getArguments()).thenReturn(new Object[] { "Carl" }); - MethodInterceptor interceptor = new PropertyAccessingMethodInterceptor(source); + new PropertyAccessingMethodInterceptor(new Source()).invoke(invocation); + } - interceptor.invoke(invocation); + /** + * @see DATACMNS-630 + */ + @Test(expected = IllegalStateException.class) + public void rejectsNonAccessorMethod() throws Throwable { - assertThat(source.firstname, is((Object) "Carl")); - } + when(invocation.getMethod()).thenReturn(Projection.class.getMethod("someGarbage")); - /** - * @see DATACMNS-820 - */ - @Test(expected = IllegalStateException.class) - public void throwsAppropriateExceptionIfTheInvocationHasNoArguments() throws Throwable { + new PropertyAccessingMethodInterceptor(new Source()).invoke(invocation); + } - Source source = new Source(); + /** + * @see DATACMNS-820 + */ + @Test + public void triggersWritePropertyAccessOnTarget() throws Throwable { - when(invocation.getMethod()).thenReturn(Projection.class.getMethod("setFirstname", String.class)); - when(invocation.getArguments()).thenReturn(new Object[0]); - MethodInterceptor interceptor = new PropertyAccessingMethodInterceptor(source); + Source source = new Source(); + source.firstname = "Dave"; - interceptor.invoke(invocation); - } + when(invocation.getMethod()).thenReturn(Projection.class.getMethod("setFirstname", String.class)); + when(invocation.getArguments()).thenReturn(new Object[] { "Carl" }); - /** - * @see DATACMNS-820 - */ - @Test(expected = NotWritablePropertyException.class) - public void throwsAppropriateExceptionIfThePropertyCannotWritten() throws Throwable { + new PropertyAccessingMethodInterceptor(source).invoke(invocation); - when(invocation.getMethod()).thenReturn(Projection.class.getMethod("setGarbage", String.class)); - when(invocation.getArguments()).thenReturn(new Object[] { "Carl" }); - new PropertyAccessingMethodInterceptor(new Source()).invoke(invocation); - } + assertThat(source.firstname, is((Object) "Carl")); + } /** - * @see DATAREST-221 + * @see DATACMNS-820 */ - @Test - public void forwardsObjectMethodInvocation() throws Throwable { + @Test(expected = IllegalStateException.class) + public void throwsAppropriateExceptionIfTheInvocationHasNoArguments() throws Throwable { - when(invocation.getMethod()).thenReturn(Object.class.getMethod("toString")); - new PropertyAccessingMethodInterceptor(new Source()).invoke(invocation); + Source source = new Source(); + + when(invocation.getMethod()).thenReturn(Projection.class.getMethod("setFirstname", String.class)); + when(invocation.getArguments()).thenReturn(new Object[0]); + + new PropertyAccessingMethodInterceptor(source).invoke(invocation); } /** - * @see DATACMNS-630 + * @see DATACMNS-820 */ - @Test(expected = IllegalStateException.class) - public void rejectsNonAccessorMethod() throws Throwable { + @Test(expected = NotWritablePropertyException.class) + public void throwsAppropriateExceptionIfThePropertyCannotWritten() throws Throwable { + + when(invocation.getMethod()).thenReturn(Projection.class.getMethod("setGarbage", String.class)); + when(invocation.getArguments()).thenReturn(new Object[] { "Carl" }); - when(invocation.getMethod()).thenReturn(Projection.class.getMethod("someGarbage")); new PropertyAccessingMethodInterceptor(new Source()).invoke(invocation); } diff --git a/src/test/java/org/springframework/data/projection/SpelAwareProxyProjectionFactoryUnitTests.java b/src/test/java/org/springframework/data/projection/SpelAwareProxyProjectionFactoryUnitTests.java index 39bd0bdf8..f12897958 100644 --- a/src/test/java/org/springframework/data/projection/SpelAwareProxyProjectionFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/projection/SpelAwareProxyProjectionFactoryUnitTests.java @@ -21,7 +21,9 @@ import static org.junit.Assert.*; import java.util.List; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.beans.NotWritablePropertyException; import org.springframework.beans.factory.annotation.Value; @@ -34,7 +36,9 @@ import org.springframework.beans.factory.annotation.Value; */ public class SpelAwareProxyProjectionFactoryUnitTests { - private SpelAwareProxyProjectionFactory factory; + public @Rule ExpectedException exception = ExpectedException.none(); + + SpelAwareProxyProjectionFactory factory; @Before public void setup() { @@ -55,6 +59,18 @@ public class SpelAwareProxyProjectionFactoryUnitTests { assertThat(excerpt.getFullName(), is("Dave Matthews")); } + /** + * @see DATACMNS-630 + */ + @Test + public void excludesAtValueAnnotatedMethodsForInputProperties() { + + List properties = factory.getInputProperties(CustomerExcerpt.class); + + assertThat(properties, hasSize(1)); + assertThat(properties, hasItem("firstname")); + } + /** * @see DATACMNS-820 */ @@ -82,25 +98,8 @@ public class SpelAwareProxyProjectionFactoryUnitTests { ProjectionWithNotWriteableProperty projection = factory.createProjection(ProjectionWithNotWriteableProperty.class, customer); - try { - projection.setFirstName("Carl"); - fail("Missing NotWritablePropertyException"); - } catch (NotWritablePropertyException e) { - assertThat(e, instanceOf(NotWritablePropertyException.class)); - } - - } - - /** - * @see DATACMNS-630 - */ - @Test - public void excludesAtValueAnnotatedMethodsForInputProperties() { - - List properties = factory.getInputProperties(CustomerExcerpt.class); - - assertThat(properties, hasSize(1)); - assertThat(properties, hasItem("firstname")); + exception.expect(NotWritablePropertyException.class); + projection.setFirstName("Carl"); } static class Customer {