Merge pull request #312 from xenit-eu/master

Release 2.0.5

CHANGELOG.md

@@ -1,6 +1,6 @@
 ---
 title: Changelog - Dynamic Extensions for Alfresco
-date: 8 October 2019
+date: 10 September 2020
 report: true
 colorlinks: true
 ---
@@ -21,6 +21,11 @@ Version template:
  -->
 # Dynamic Extensions For Alfresco Changelog
 
+## [2.0.5] - 2020-09-10
+### Fixed
+* [#306](https://github.com/xenit-eu/dynamic-extensions-for-alfresco/issues/306) Alfresco 6.2.1 issues due to cglib imports in control-panel bundle
+* [#308](https://github.com/xenit-eu/dynamic-extensions-for-alfresco/issues/308) Calling webscript without required `@RequestParam` causes 500 error response
+
 ## [2.0.4] - 2020-05-27
 ### Fixed
 * [#297](https://github.com/xenit-eu/dynamic-extensions-for-alfresco/issues/297) Multiple bean candidates for a Quartz' `SchedulerFactoryBean`
@@ -109,4 +114,3 @@ Registering a component implementing multiple Activiti listener Interfaces throw
 * [#143](https://github.com/xenit-eu/dynamic-extensions-for-alfresco/issues/143) WebScriptUtil.extractHttpServletResponse() not working.
 * [#151](https://github.com/xenit-eu/dynamic-extensions-for-alfresco/issues/151) @ResponseBody with void return type causes NullpointerException
 
-

annotations-runtime/src/main/java/com/github/dynamicextensionsalfresco/webscripts/arguments/RequestParamArgumentResolver.java

@@ -5,6 +5,7 @@
 
 import com.github.dynamicextensionsalfresco.webscripts.annotations.RequestParam;
 
+import org.springframework.extensions.webscripts.WebScriptException;
 import org.springframework.extensions.webscripts.WebScriptRequest;
 import org.springframework.extensions.webscripts.WebScriptResponse;
 import org.springframework.util.Assert;
@@ -72,8 +73,8 @@ private Object handleSingleParameter(final Class<?> parameterType, final Request
 			if (StringUtils.hasText(requestParam.defaultValue())) {
 				value = getStringValueConverter().convertStringValue(parameterType, requestParam.defaultValue());
 			}
-			if (requestParam.required() && value == null) {
-				throw new IllegalStateException(String.format("Request parameter not available: %s", parameterName));
+			if (value == null) {
+				throwMissingParameterIfMandatory(requestParam, parameterName);
 			}
 		}
 		return value;
@@ -85,8 +86,8 @@ private Object handleDelimitedParameter(final Class<?> parameterType, final Requ
 		if (parameterValue == null) {
 			if (StringUtils.hasText(requestParam.defaultValue())) {
 				parameterValue = requestParam.defaultValue();
-			} else if (requestParam.required()) {
-				throw new IllegalStateException(String.format("Request parameter not available: %s", parameterName));
+			} else {
+				throwMissingParameterIfMandatory(requestParam, parameterName);
 			}
 		}
         if (parameterValue != null) {
@@ -101,15 +102,21 @@ private Object handleMultipleParameters(final Class<?> parameterType, final Requ
 			final WebScriptRequest request, final String parameterName) {
 		String[] parameterValues = request.getParameterValues(parameterName);
 		if (parameterValues == null) {
-			if (requestParam.required()) {
-				throw new IllegalStateException(String.format("Request parameter not available: %s", parameterName));
-			} else {
-				parameterValues = new String[] { requestParam.defaultValue() };
-			}
+			throwMissingParameterIfMandatory(requestParam, parameterName);
+			parameterValues = new String[] { requestParam.defaultValue() };
 		}
 		return convertToArray(parameterType.getComponentType(), parameterValues);
 	}
 
+	private void throwMissingParameterIfMandatory(RequestParam requestParam, String parameterName) {
+		if(requestParam.required()) {
+			throw new WebScriptException(
+					requestParam.missingParameterHttpStatusCode(),
+					String.format("Request parameter not available: %s", parameterName)
+			);
+		}
+	}
+
 	private Object[] convertToArray(final Class<?> arrayComponentType, final String[] parameterValues) {
 		final Object[] values = (Object[]) Array.newInstance(arrayComponentType, parameterValues.length);
 		for (int i = 0; i < parameterValues.length; i++) {

annotations-runtime/src/test/java/com/github/dynamicextensionsalfresco/webscripts/RequestParamHandler.java

@@ -53,4 +53,20 @@ public void handleQName(@RequestParam final QName qname) {
 	@Uri("/handleNodeRef")
 	public void handleNodeRef(@RequestParam final NodeRef nodeRef) {
 	}
+
+	@Uri("/handleMissingRequiredParameterDefault")
+	public void handleMissingRequiredParameterDefault(@RequestParam final String expectedCode) {
+	}
+
+	@Uri("/handleMissingRequiredParameter422")
+	public void handleMissingRequiredParameter422(@RequestParam(missingParameterHttpStatusCode = 422) final String expectedCode) {
+	}
+
+	@Uri("/handleOptionalMissingParameter")
+	public void handleOptionalMissingParameter(@RequestParam(required = false) final String expectedCode) {
+	}
+
+	@Uri("/handleOptionalMissingParameter422")
+	public void handleOptionalMissingParameter422(@RequestParam(missingParameterHttpStatusCode = 422, required = false) final String expectedCode) {
+	}
 }

annotations-runtime/src/test/java/com/github/dynamicextensionsalfresco/webscripts/RequestParamTest.java

@@ -1,14 +1,21 @@
 package com.github.dynamicextensionsalfresco.webscripts;
 
-import static org.mockito.Mockito.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 import com.github.dynamicextensionsalfresco.webscripts.annotations.RequestParam;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.namespace.NamespacePrefixResolver;
 import org.alfresco.service.namespace.NamespaceService;
+import org.junit.Assert;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.extensions.webscripts.WebScriptException;
 
 /**
  * Integration test for {@link RequestParam} handling.
@@ -91,4 +98,44 @@ public void handleNodeRef() {
 				"workspace://SpacesStore/c269c803-4fd6-4aad-9114-3a42ff263fdc"));
 		verify(handler).handleNodeRef(new NodeRef("workspace://SpacesStore/c269c803-4fd6-4aad-9114-3a42ff263fdc"));
 	}
+
+	@Test
+	public void testHandleMissingRequiredParameterDefault() {
+		try {
+			handleGet("/handleMissingRequiredParameterDefault", new MockWebScriptRequest());
+			verify(handler).handleMissingRequiredParameterDefault(eq(null));
+			Assert.fail("Must throw an WebScriptException!");
+		} catch (WebScriptException ex) {
+			assertEquals(400, ex.getStatus());
+		} catch (Exception ex) {
+			Assert.fail("Must throw an WebScriptException!");
+		}
+	}
+
+	@Test
+	public void testHandleMissingRequiredParameterWithCustomHttpStatus() {
+		try {
+			handleGet("/handleMissingRequiredParameter422", new MockWebScriptRequest());
+			verify(handler).handleMissingRequiredParameter422(eq(null));
+			Assert.fail("Must throw an WebScriptException!");
+		} catch (WebScriptException ex) {
+			assertEquals(ex.getStatus(), 422);
+		} catch (Exception ex) {
+			Assert.fail("Must throw an WebScriptException!");
+		}
+	}
+
+	@Test
+	public void testHandleOptionalMissingParameter() {
+		handleGet("/handleOptionalMissingParameter", new MockWebScriptRequest());
+		verify(handler).handleOptionalMissingParameter(eq(null));
+		assertTrue(true); // no exceptions thrown = GOOD!
+	}
+
+	@Test
+	public void testHandleOptionalMissingParameterWithCustomHttpStatus() {
+		handleGet("/handleOptionalMissingParameter422", new MockWebScriptRequest());
+		verify(handler).handleOptionalMissingParameter422(eq(null));
+		assertTrue(true); // no exceptions thrown = GOOD!
+	}
 }

annotations/src/main/java/com/github/dynamicextensionsalfresco/webscripts/annotations/RequestParam.java

@@ -31,4 +31,8 @@
 	 */
 	String delimiter() default "";
 
+	/**
+	 * Specifies the status code to be returned when a required parameter is missing
+	 */
+	int missingParameterHttpStatusCode() default 400;
 }

build.gradle

@@ -31,7 +31,7 @@ nexusStaging {
 
 allprojects {
     group = 'eu.xenit.de'
-    version = '2.0.4'
+    version = '2.0.5'
 
     boolean isRelease = System.env.BRANCH_NAME?.startsWith("release")
     if (!isRelease)

control-panel/build.gradle

@@ -48,9 +48,6 @@ jar {
             'Alfresco-Spring-Configuration': 'com.github.dynamicextensionsalfresco.controlpanel',
             'Spring-Context': ';publish-context:=false',
             'Import-Package': 'javax.annotation;version=!, ' +
-                    'net.sf.cglib.core, ' +
-                    'net.sf.cglib.proxy, ' +
-                    'net.sf.cglib.reflect, ' +
                     'org.aopalliance.aop, ' +
                     'org.aopalliance.intercept, ' +
                     'org.springframework.aop, ' +

documentation/Extension_Point_WebScripts.md

@@ -198,6 +198,27 @@ protected void handleIllegalArgument(IllegalArgumentException exception, WebScri
     response.setStatus(400);
 }
 ```
+This handler method must be defined in the same `@WebScript` annotated class that will throw the Exceptions to be handled.
+
+It is also possible to separate out exception handler methods into their own class, using an interface with default methods.
+The interface can then be implemented by any `@WebScript` class that requires it:
+```java
+public interface ExceptionHandlers {
+    @ExceptionHandler(IllegalArgumentException.class)
+    default void handleIllegalArgument(IllegalArgumentException exception, WebScriptResponse response) {
+        response.setStatus(400);
+    }
+}
+
+@Component @Webscript
+public class SampleWebscript implements ExceptionHandlers {
+    @Uri(value = "/document")
+    public void retrieveDocument(@RequestParam String name) {
+        if (!isValid(name))
+            throw new IllegalArgumentException();
+    }
+}
+```
 
 ## Before
 

integration-tests/alfresco-enterprise-62/overload.gradle

@@ -1,6 +1,6 @@
 ext {
-    alfrescoBaseWar = 'org.alfresco:content-services:6.2.0@war'
-    alfrescoBaseImage = 'hub.xenit.eu/alfresco-enterprise/alfresco-repository-enterprise:6.2.0'
+    alfrescoBaseWar = 'org.alfresco:content-services:6.2.1@war'
+    alfrescoBaseImage = 'hub.xenit.eu/alfresco-enterprise/alfresco-repository-enterprise:6.2.1'
 
     postgresImage = 'postgres:10.1'
 }

integration-tests/src/test/java/eu/xenit/dynamicextensionsalfresco/SpringProxyMagicTest.java

@@ -0,0 +1,48 @@
+package eu.xenit.dynamicextensionsalfresco;
+
+import static io.restassured.RestAssured.given;
+import static org.hamcrest.Matchers.equalTo;
+
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Integration tests that will cause Spring proxy magic to happen in Alfresco. Makes use of the '/greeting/...'
+ * endpoints which are defined in the 'test-bundle'
+ * <p>
+ * Interesting read on Spring and CGLIB proxies: http://www.javabyexamples.com/cglib-proxying-in-spring-configuration
+ */
+public class SpringProxyMagicTest extends RestAssuredTest {
+
+    private static final Logger logger = LoggerFactory.getLogger(BehaviourTest.class);
+
+    @Test
+    public void triggerAndTestSpringProxyMagic() {
+        logger.info("Test scenario: make use of a Bean which was initialized by an @Configuration annotated class."
+                + " By doing so, we try to proactively trigger a ClassNotFoundException for any of the Spring CGLIB"
+                + "related classes.");
+
+        given()
+                .log().ifValidationFails()
+                .when()
+                .get("s/dynamic-extensions/testing/greeting")
+                .then()
+                .log().ifValidationFails()
+                .statusCode(200)
+                .body(equalTo("\"Hello there, adventurer!\""));
+
+        for (int i = 0; i < 3; i++) {
+            // Beans initialized via the @Bean annotation in an @Configuration class should only be initialized once
+            given()
+                    .log().ifValidationFails()
+                    .when()
+                    .get("s/dynamic-extensions/testing/greeting/number-of-instances")
+                    .then()
+                    .log().ifValidationFails()
+                    .statusCode(200)
+                    .body(equalTo("1"));
+        }
+    }
+
+}

integration-tests/test-bundle/src/main/java/eu/xenit/de/testing/greeting/GreetingConfiguration.java

@@ -0,0 +1,25 @@
+package eu.xenit.de.testing.greeting;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration
+@SuppressWarnings("unused")
+public class GreetingConfiguration {
+
+    @Bean
+    public GreetingService greetingService() {
+        return new GreetingService();
+    }
+
+    @Bean
+    public GreetingServiceWrapper greetingServiceWrapper() {
+        return new GreetingServiceWrapper(greetingService());
+    }
+
+    @Bean
+    public GreetingServiceWrapper anotherGreetingServiceWrapper() {
+        return new GreetingServiceWrapper(greetingService());
+    }
+
+}

integration-tests/test-bundle/src/main/java/eu/xenit/de/testing/greeting/GreetingService.java

@@ -0,0 +1,19 @@
+package eu.xenit.de.testing.greeting;
+
+public class GreetingService {
+
+    private static int numberOfInstances;
+
+    public GreetingService() {
+        numberOfInstances++;
+    }
+
+    public String getGreeting() {
+        return "Hello there, adventurer!";
+    }
+
+    public static int getNumberOfInstances() {
+        return numberOfInstances;
+    }
+
+}

integration-tests/test-bundle/src/main/java/eu/xenit/de/testing/greeting/GreetingServiceWrapper.java

@@ -0,0 +1,14 @@
+package eu.xenit.de.testing.greeting;
+
+public class GreetingServiceWrapper {
+
+    private final GreetingService greetingService;
+
+    public GreetingServiceWrapper(GreetingService greetingService) {
+        this.greetingService = greetingService;
+    }
+
+    public GreetingService getGreetingService() {
+        return greetingService;
+    }
+}

integration-tests/test-bundle/src/main/java/eu/xenit/de/testing/greeting/GreetingWebScript.java

@@ -0,0 +1,45 @@
+package eu.xenit.de.testing.greeting;
+
+import static eu.xenit.de.testing.Constants.TEST_WEBSCRIPTS_BASE_URI;
+import static eu.xenit.de.testing.Constants.TEST_WEBSCRIPTS_FAMILY;
+
+import com.github.dynamicextensionsalfresco.webscripts.annotations.Uri;
+import com.github.dynamicextensionsalfresco.webscripts.annotations.WebScript;
+import java.util.Objects;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.stereotype.Component;
+
+@Component
+@WebScript(families = TEST_WEBSCRIPTS_FAMILY, baseUri = TEST_WEBSCRIPTS_BASE_URI + "/greeting")
+@SuppressWarnings("unused")
+public class GreetingWebScript {
+
+    private final GreetingService greetingService;
+
+    @Autowired
+    public GreetingWebScript(
+            @Qualifier("greetingServiceWrapper") GreetingServiceWrapper first,
+            @Qualifier("anotherGreetingServiceWrapper") GreetingServiceWrapper second) {
+
+        if (!Objects.equals(first.getGreetingService(), second.getGreetingService())) {
+            throw new IllegalStateException("Requiring only one GreetingService to rule them all");
+        }
+
+        this.greetingService = first.getGreetingService();
+    }
+
+    @Uri
+    public ResponseEntity<String> getGreeting() {
+        return new ResponseEntity<>(greetingService.getGreeting(), HttpStatus.OK);
+    }
+
+    @Uri("/number-of-instances")
+    public ResponseEntity<Integer> getNumberOfInstances() {
+        return new ResponseEntity<>(GreetingService.getNumberOfInstances(), HttpStatus.OK);
+    }
+
+
+}