Skip to content

Commit

Permalink
fix(plugin): #29276 - Fix portlet serialization using immutables and …
Browse files Browse the repository at this point in the history
…jackson (#29345)

### Proposed Changes
* JAXB Serialisation was causing issues due to the masking of fields
from the base Portlet by DotPortlet. When you reference the cast to
Portlet you get the field on that class, and get different results when
cast as DotPotlet. Instead of extending Portlet in this way, or tying
the serialization to the underlying object we can make use of the
immutables framework to create a serializable specific version of
Portlet that can easily be mapped to and from the underlying Portlet.
This model can be used in many cases providing an abstraction layer that
is more flexible to changes and easier to test.

### Testing

* Portlets need to be able to load and should test the Plugins page
loads and check the logs for error
* Add plugins including the cdn plugin and ensure it starts up
correctly.
* Add the CDN Plugin to the menus and browse to the plugin page. It will
show loading spinners and show an error in the log as it is not
configured but the page headings should show and there should not be
other errors.
* On stop of the plugin there may be an error. This is expected when the
jersey servlet container is restarted. The plugin should be able to be
started again after.
```[18/07/24 17:48:21:869 GMT]  WARN mapper.RuntimeExceptionMapper: ServiceLocatorImpl(__HK2_Generated_46,47,1198896597) has been shut down
java.lang.IllegalStateException: ServiceLocatorImpl(__HK2_Generated_46,47,1198896597) has been shut down```


related to #29276
  • Loading branch information
spbolton authored Jul 25, 2024
1 parent e10a52b commit 2c23184
Show file tree
Hide file tree
Showing 24 changed files with 1,436 additions and 909 deletions.
47 changes: 22 additions & 25 deletions bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
<aws-java-sdk.version>1.12.488</aws-java-sdk.version>
<twelvemonkeys.version>3.10.1</twelvemonkeys.version>
<swagger.version>2.2.0</swagger.version>
<bytebuddy.version>1.14.12</bytebuddy.version>
<bytebuddy.version>1.14.15</bytebuddy.version>
<batik.version>1.17</batik.version>
<bouncy-castle.version>1.70</bouncy-castle.version>
<awaitility.version>3.0.0</awaitility.version>
<awaitility.version>4.0.0</awaitility.version>
<shedlock.version>4.33.0</shedlock.version>
<glowroot.version>0.14.1</glowroot.version>
<jackson.version>2.16.1</jackson.version>
<jersey.version>2.22.1</jersey.version>
<jackson.version>2.17.2</jackson.version>
<jersey.version>2.27</jersey.version>
<graalvm.version>22.3.3</graalvm.version>
</properties>
<dependencyManagement>
Expand Down Expand Up @@ -1132,14 +1132,6 @@
</dependency>


<!-- JAX-RS Rest API -->

<dependency>
<groupId>javax.ws.rs</groupId>
<artifactId>javax.ws.rs-api</artifactId>
<version>2.0.1</version>
</dependency>

<!-- Rest API Documentation -->
<dependency>
<groupId>io.swagger.core.v3</groupId>
Expand Down Expand Up @@ -1382,7 +1374,11 @@
<type>pom</type>
<scope>import</scope>
</dependency>

<dependency>
<groupId>org.glassfish.jersey.inject</groupId>
<artifactId>jersey-hk2</artifactId>
<version>${jersey.version}</version>
</dependency>
<dependency>
<groupId>jakarta.inject</groupId>
<artifactId>jakarta.inject-api</artifactId>
Expand Down Expand Up @@ -1466,6 +1462,13 @@
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<exclusions>
<exclusion>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<!-- Replaced with hamcrest org.hamcrest:hamcrest:2.1 -->
</exclusion>
</exclusions>
</dependency>

<dependency>
Expand All @@ -1488,29 +1491,23 @@
<artifactId>awaitility</artifactId>
<version>${awaitility.version}</version>
</dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility-proxy</artifactId>
<version>${awaitility.version}</version>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>5.11.0</version>
<version>5.12.0</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>5.11.0</version>
<version>5.12.0</version>
<scope>test</scope>
</dependency>
<dependency>
<!-- Hamcrest is a framework for writing matcher objects allowing 'match' rules to be defined declaratively. -->
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
<version>1.3</version>
<!-- often just used for testing we have in production code
Note, scope does not pass down to child, must declare scope explicitly-->
<artifactId>hamcrest</artifactId>
<version>2.1</version>
<scope>test</scope>
</dependency>

<dependency>
Expand Down
24 changes: 11 additions & 13 deletions dotCMS/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,11 @@
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.dataformat</groupId>
<artifactId>jackson-dataformat-xml</artifactId>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-guava</artifactId>
Expand Down Expand Up @@ -975,13 +980,6 @@
</dependency>


<!-- JAX-RS Rest API -->

<dependency>
<groupId>javax.ws.rs</groupId>
<artifactId>javax.ws.rs-api</artifactId>
<scope>compile</scope>
</dependency>

<!-- Rest API Documentation -->
<dependency>
Expand Down Expand Up @@ -1223,6 +1221,11 @@
<groupId>org.glassfish.jersey.media</groupId>
<artifactId>jersey-media-sse</artifactId>
</dependency>
<dependency>
<groupId>org.glassfish.jersey.inject</groupId>
<artifactId>jersey-hk2</artifactId>
</dependency>


<dependency>
<!-- Immutables is a Java annotation processor to eliminate the burden of writing immutable types.
Expand Down Expand Up @@ -1309,11 +1312,6 @@
<artifactId>awaitility</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility-proxy</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand All @@ -1323,7 +1321,7 @@
<dependency>
<!-- Hamcrest is a framework for writing matcher objects allowing 'match' rules to be defined declaratively. -->
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
<artifactId>hamcrest</artifactId>
<!-- often just used for testing we have in production code -->
<scope>compile</scope>
</dependency>
Expand Down
16 changes: 16 additions & 0 deletions dotCMS/src/main/java/com/dotcms/annotations/AllowNulls.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.dotcms.annotations;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* This annotation is used in Immutables to allow null values in the generated classes collections
* See <a href="https://immutables.github.io/immutable.html#nulls-in-collection">https://immutables.github.io/immutable.html#nulls-in-collection</a>
*/
@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
public @interface AllowNulls {

}
16 changes: 16 additions & 0 deletions dotCMS/src/main/java/com/dotcms/annotations/Nullable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.dotcms.annotations;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* This annotation is used in Immutables to allow null values in the generated classes collections
* See <a href="https://immutables.github.io/immutable.html#nulls-in-collection">https://immutables.github.io/immutable.html#nulls-in-collection</a>
*/
@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
public @interface Nullable {

}
15 changes: 15 additions & 0 deletions dotCMS/src/main/java/com/dotcms/annotations/SkipNulls.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.dotcms.annotations;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
/**
* This annotation is used in Immutables to skip null values in the generated classes collections
* See <a href="https://immutables.github.io/immutable.html#nulls-in-collection">https://immutables.github.io/immutable.html#nulls-in-collection</a>
*/
@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
public @interface SkipNulls {

}
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,21 @@ public final Response saveNew(@Context final HttpServletRequest request,

final Portlet contentPortlet = portletApi.findPortlet("content");

final Map<String, String> initValues = new HashMap<>(contentPortlet.getInitParams());
initValues.put("name", formData.portletName);
initValues.put("baseTypes", formData.baseTypes);
initValues.put("contentTypes", formData.contentTypes);
initValues.put(DATA_VIEW_MODE_KEY, formData.dataViewMode);
final DotPortlet newPortlet = DotPortlet.builder()
.portletId(portletId)
.portletClass(contentPortlet.getPortletClass())

final Portlet newPortlet = APILocator.getPortletAPI()
.savePortlet(new DotPortlet(portletId, contentPortlet.getPortletClass(), initValues), initData.getUser());
.putInitParam("name", formData.portletName)
.putInitParam("baseTypes", formData.baseTypes)
.putInitParam("contentTypes", formData.contentTypes)
.putInitParam(DATA_VIEW_MODE_KEY, formData.dataViewMode)
.build();

return Response.ok(new ResponseEntityView<>(Map.of(JSON_RESPONSE_PORTLET_ATTR, newPortlet.getPortletId()))).build();

final Portlet savedPortlet = APILocator.getPortletAPI()
.savePortlet(newPortlet.toPortlet(), initData.getUser());

return Response.ok(new ResponseEntityView<>(Map.of(JSON_RESPONSE_PORTLET_ATTR, savedPortlet.getPortletId()))).build();
} catch (final Exception e) {
Logger.error(this, String.format("An error occurred when saving new Portlet with ID " +
"'%s': %s", portletId, ExceptionUtil.getErrorMessage(e)), e);
Expand Down Expand Up @@ -158,18 +163,24 @@ public final Response updatePortlet(@Context final HttpServletRequest request, f
}
final Portlet contentPortlet = portletApi.findPortlet("content");

final Map<String, String> initValues = new HashMap<>(contentPortlet.getInitParams());
initValues.put("name", formData.portletName);
initValues.put("baseTypes", formData.baseTypes);
initValues.put("contentTypes", formData.contentTypes);
initValues.put(DATA_VIEW_MODE_KEY, formData.dataViewMode);
final DotPortlet updatedPortlet = DotPortlet.builder()
.portletId(portletId)
.portletClass(contentPortlet.getPortletClass())
.putInitParam("name", formData.portletName)
.putInitParam("baseTypes", formData.baseTypes)
.putInitParam("contentTypes", formData.contentTypes)
.putInitParam(DATA_VIEW_MODE_KEY, formData.dataViewMode)
.build();


final Portlet newPortlet = APILocator.getPortletAPI()
.savePortlet(new DotPortlet(portletId, contentPortlet.getPortletClass(), initValues), initData.getUser());
.savePortlet(updatedPortlet.toPortlet(), initData.getUser());

return Response.ok(new ResponseEntityView<>(Map.of(JSON_RESPONSE_PORTLET_ATTR, newPortlet.getPortletId()))).build();

} catch (Exception e) {
Logger.error(this, String.format("An error occurred when updating Portlet with ID " +
"'%s': %s", formData.portletId, ExceptionUtil.getErrorMessage(e)), e);
response = ResponseUtil.mapExceptionResponse(e);
}

Expand Down
Loading

0 comments on commit 2c23184

Please sign in to comment.