Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to spring boot 3 #47

Merged
merged 10 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/spring-hello-world/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ to render and consume [HAL+JSON](https://stateless.group/hal_specification.html)

# Build and Run

Using **JDK 8, 11 or 17** and **Apache Maven 3.6.3** (or higher) you should be able to build and run the example like this:
Using **JDK 17** and **Apache Maven 3.6.3** (or higher) you should be able to build and run the example like this:

```
git clone https://github.com/wcm-io-caravan/caravan-rhyme.git
cd caravan-rhyme
git checkout master
mvn -f examples/spring-helloworld/ clean verify spring-boot:run
mvn -f examples/spring-hello-world/ clean verify spring-boot:run
```

If there are any failures during the build or integration tests, then please open an issue on github!
Expand Down
12 changes: 8 additions & 4 deletions examples/spring-hello-world/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
</scm>

<properties>
<java.version>11</java.version>
<spring-boot.version>2.5.8</spring-boot.version>
<java.version>17</java.version>
<spring-boot.version>3.1.3</spring-boot.version>
</properties>

<dependencyManagement>
Expand All @@ -45,6 +45,10 @@
<dependencies>

<!-- Spring dependencies -->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-webflux</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-hateoas</artifactId>
Expand All @@ -58,7 +62,7 @@
<dependency>
<groupId>io.wcm.caravan</groupId>
<artifactId>io.wcm.caravan.rhyme.spring</artifactId>
<version>1.0.0</version>
<version>2.0.0-SNAPSHOT</version>
</dependency>

<!-- Test dependencies -->
Expand Down Expand Up @@ -156,4 +160,4 @@
</plugins>
</pluginManagement>
</build>
</project>
</project>
2 changes: 1 addition & 1 deletion examples/spring-hypermedia/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Make sure not to miss loading a `company:detailedEmployee` link when you run the
# Build and Run


Using **JDK 8, 11 or 17** and **Apache Maven 3.6.3** (or higher) you should be able to build and run the example like this:
Using **JDK 17** and **Apache Maven 3.6.3** (or higher) you should be able to build and run the example like this:

```
git clone https://github.com/wcm-io-caravan/caravan-rhyme.git
Expand Down
8 changes: 4 additions & 4 deletions examples/spring-hypermedia/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
</scm>

<properties>
<java.version>11</java.version>
<spring-boot.version>2.5.8</spring-boot.version>
<java.version>17</java.version>
<spring-boot.version>3.1.3</spring-boot.version>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -80,7 +80,7 @@
<dependency>
<groupId>io.wcm.caravan</groupId>
<artifactId>io.wcm.caravan.rhyme.spring</artifactId>
<version>1.0.1-SNAPSHOT</version>
<version>2.0.0-SNAPSHOT</version>
</dependency>

<!-- Test dependencies -->
Expand Down Expand Up @@ -190,4 +190,4 @@
</plugins>
</pluginManagement>
</build>
</project>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.methodOn;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Lazy;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

Expand All @@ -48,10 +49,13 @@ class CompanyApiController implements CompanyApi {

// inject the controllers for all resources that are linked from the entry point
@Autowired
@Lazy
private EmployeeController employees;
@Autowired
@Lazy
private ManagerController managers;
@Autowired
@Lazy
private DetailedEmployeeController detailedEmployees;

@Autowired
Expand Down Expand Up @@ -108,7 +112,6 @@ public ManagerResource getManagerById(Long id) {
}

@Override

public DetailedEmployeeResource getDetailedEmployeeById(Long id) {
return detailedEmployees.findById(id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@

import java.time.Duration;

import javax.servlet.http.HttpServletRequest;

import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -41,6 +39,7 @@
import io.wcm.caravan.rhyme.spring.api.RhymeLinkBuilder;
import io.wcm.caravan.rhyme.spring.api.SpringRhyme;
import io.wcm.caravan.rhyme.spring.api.UrlFingerprinting;
import jakarta.servlet.http.HttpServletRequest;

/**
* Defines common logic (e.g. URL fingerprinting) for building any link to your resources in this project .
Expand Down Expand Up @@ -126,9 +125,9 @@ boolean hasClientPreferences() {
* by the {@link WebMvcLinkBuilder} class, but additional query parameters that are not directly
* present in the API and controller signatures can be appended by {@link UrlFingerprinting}.
* @param webMvcLinkBuilder created with {@link WebMvcLinkBuilder#linkTo(Class)} and
* {@link WebMvcLinkBuilder#methodOn(Class, Object...)}
* {@link WebMvcLinkBuilder#methodOn(Class, Object...)}
* @return a {@link RhymeLinkBuilder} that you can use to decorate the link with name and title attributes, and
* then finally build it
* then finally build it
* @see WebMvcLinkBuilder
*/
RhymeLinkBuilder create(WebMvcLinkBuilder webMvcLinkBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
*/
package io.wcm.caravan.rhyme.examples.spring.hypermedia;

import javax.persistence.Entity;
import javax.persistence.EntityListeners;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.OneToOne;

import com.fasterxml.jackson.annotation.JsonIgnore;

import jakarta.persistence.Entity;
import jakarta.persistence.EntityListeners;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;

/**
* @author Greg Turnquist
*/
Expand All @@ -46,7 +46,7 @@ public class Employee {
* To break the recursive, bidirectional relationship, don't serialize {@literal manager}.
*/
@JsonIgnore
@OneToOne
@ManyToOne
private Manager manager;

Employee(String name, String role, Manager manager) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ class EmployeeController {

// inject the controllers for all related resources
@Autowired
@org.springframework.context.annotation.Lazy
private CompanyApiController api;
@Autowired
@org.springframework.context.annotation.Lazy
private ManagerController managers;
@Autowired
@org.springframework.context.annotation.Lazy
private DetailedEmployeeController detailedEmployees;

@Autowired
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
import java.util.ArrayList;
import java.util.List;

import javax.persistence.Entity;
import javax.persistence.EntityListeners;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.OneToMany;

import com.fasterxml.jackson.annotation.JsonIgnore;

import jakarta.persistence.Entity;
import jakarta.persistence.EntityListeners;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.OneToMany;

/**
* @author Greg Turnquist
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ class ManagerController {

// inject the controllers for all related resources
@Autowired
@org.springframework.context.annotation.Lazy
private CompanyApiController api;
@Autowired
@org.springframework.context.annotation.Lazy
private EmployeeController employees;

@Autowired
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@

import java.time.Instant;

import javax.persistence.PostPersist;
import javax.persistence.PostRemove;
import javax.persistence.PostUpdate;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Component;

import io.wcm.caravan.rhyme.spring.api.UrlFingerprinting;
import jakarta.persistence.PostPersist;
import jakarta.persistence.PostRemove;
import jakarta.persistence.PostUpdate;

/**
* Keeps track of any modifications to the repositories for {@link Employee} and {@link Manager} entities, so that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.internal.util.collections.Iterables;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.MediaType;

import com.google.common.collect.Iterables;

import io.wcm.caravan.hal.resource.Link;
import io.wcm.caravan.rhyme.api.exceptions.HalApiClientException;
import io.wcm.caravan.rhyme.api.exceptions.HalApiServerException;
Expand Down Expand Up @@ -75,12 +76,12 @@ void setUp() {

protected Long getIdOfFirstEmployee() {

return Iterables.firstOf(employeeRepository.findAll()).getId();
return Iterables.get(employeeRepository.findAll(), 0).getId();
}

protected Long getIdOfFirstManager() {

return Iterables.firstOf(managerRepository.findAll()).getId();
return Iterables.get(managerRepository.findAll(), 0).getId();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected CompanyApi getApiImplementationOrClientProxy() {
// since we don't have access to the repository in this test, the IDs need to be hard-coded
@Override
protected Long getIdOfFirstEmployee() {
return 2L;
return 1L;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,12 @@ void should_extract_status_code_from_NoHandlerFoundException() {

String nonExistingUrl = baseUri + "/foo/bar";

HalResponse errorResponse = getResponseFromCaughtClientException(
(er) -> client.getRemoteResource(nonExistingUrl, ErrorThrowingResource.class));
ErrorThrowingResource errorResource = client.getRemoteResource(nonExistingUrl, ErrorThrowingResource.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't this test use getResponseFromCaughtClientException any more? I didn't spot any difference in the assertions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the 404 is returned with another content-type "application/json" and not "application/vnd.error".
I decided that this is acceptable for simple unmapped URLs. Or do you think It's a regression?

Perhaps I'll refactor the code a little bit to spot this difference more clearly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay I missed that.

Then this is indeed a minor regression. The goal for the spring integration is that for as many errors as possible a full vnd.error response is being returned.

This is what the VndErrorHandlingControllerAdvice should take care of.

The previous solution was already a bit fragile, as it required some additional configuration in the application properties:

This did already change with some minor spring 2.x. update a while ago.

I guess if you start one of the spring examples now, and call a non-existant URL like /foo/bar, you'll probably see a default 404 JSON response from the spring framework now, instead of the custom vnd.error responses you get for any other exceptions.

This is something we would want to avoid, and make sure these HandlerNotFoundExceptions also are treated as other framework exceptions (i.e. caught and rendered by the VndErrorHandlingControllerAdvice )

If it's too much work to get this working again however, it's not essential

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I even managed to restore the expected error handling with new Spring version.
That obsoletes this question.

Thanks @feffef to spot this issue again.


assertThat(errorResponse.getStatus())
.isEqualTo(HttpStatus.NOT_FOUND.value());
HalApiClientException ex = assertThrows(HalApiClientException.class,
() -> errorResource.getStateWithError());

assertThat(ex.getErrorResponse()).isNotNull();
assertThat(ex.getErrorResponse().getStatus()).isEqualTo(HttpStatus.NOT_FOUND.value());
}
}
6 changes: 6 additions & 0 deletions integration/spring/changes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
xsi:schemaLocation="http://maven.apache.org/changes/1.0.0 http://maven.apache.org/plugins/maven-changes-plugin/xsd/changes-1.0.0.xsd">
<body>

<release version="2.0.0">
<action type="add" dev="ckumpe">
Migrate to Java 17 and Spring Boot 3
</action>
</release>

<release version="1.0.2" date="not released">
<action type="add" dev="ssauder">
Reduced expectations in AbstractHalResourceLoaderTest on how exactly malformed responses are handled by client implementations.
Expand Down
19 changes: 13 additions & 6 deletions integration/spring/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</parent>

<artifactId>io.wcm.caravan.rhyme.spring</artifactId>
<version>1.0.1-SNAPSHOT</version>
<version>2.0.0-SNAPSHOT</version>
<packaging>bundle</packaging>

<name>Rhyme - Spring Integration</name>
Expand All @@ -45,8 +45,8 @@

<properties>
<site.url.module.prefix>rhyme/spring</site.url.module.prefix>
<java.version>11</java.version>
<spring-boot.version>2.5.8</spring-boot.version>
<java.version>17</java.version>
<spring-boot.version>3.1.3</spring-boot.version>
</properties>

<dependencyManagement>
Expand All @@ -64,10 +64,9 @@

<dependencies>
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
<version>4.0.1</version>
</dependency>

<!-- Spring Dependencies -->
Expand Down Expand Up @@ -147,6 +146,14 @@
<plugin>
<groupId>org.apache.felix</groupId>
<artifactId>maven-bundle-plugin</artifactId>
<dependencies>
<!-- this is needed for Java 17 support until there's new release of the plugin with a newer version of ASM -->
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
<version>9.1</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>baseline</id>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ class SpringExceptionStatusAndLoggingStrategy implements ExceptionStatusAndLoggi
@Override
public Integer extractStatusCode(Throwable error) {

if (error instanceof ResponseStatusException) {
return ((ResponseStatusException)error).getStatus().value();
if (error instanceof ResponseStatusException responsestatusexception) {
return responsestatusexception.getStatusCode().value();
}

if (error.getClass().isAnnotationPresent(ResponseStatus.class)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

import javax.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequest;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -98,7 +98,7 @@ private static RhymeBuilder createRhymeBuilder(HttpServletRequest httpRequest, H

private static String getRequestUrl(HttpServletRequest httpRequest) {

StringBuffer requestUrl = httpRequest.getRequestURL();
StringBuilder requestUrl = new StringBuilder(httpRequest.getRequestURL());

if (httpRequest.getQueryString() != null) {
requestUrl.append("?");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.util.Map;
import java.util.function.Supplier;

import javax.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequest;

import org.apache.commons.lang3.StringUtils;
import org.springframework.hateoas.server.mvc.WebMvcLinkBuilder;
Expand Down
Loading