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

Exception mapping #24

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
19 changes: 15 additions & 4 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@
<artifactId>quarkus-hibernate-orm</artifactId>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-hibernate-validator</artifactId>
</dependency>

<!-- JDBC driver dependencies -->
<dependency>
<groupId>io.quarkus</groupId>
Expand Down Expand Up @@ -97,7 +102,7 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-smallrye-fault-tolerance</artifactId>
</dependency>

<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-jpamodelgen</artifactId>
Expand All @@ -106,7 +111,7 @@
<groupId>io.quarkus</groupId>
<artifactId>quarkus-spring-data-jpa</artifactId>
</dependency>

<dependency>
<groupId>com.querydsl</groupId>
<artifactId>querydsl-jpa</artifactId>
Expand Down Expand Up @@ -198,6 +203,12 @@
<artifactId>rest-assured</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.12.0</version>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down Expand Up @@ -238,8 +249,8 @@
</systemPropertyVariables>
</configuration>
</plugin>


<plugin>
<groupId>com.mysema.maven</groupId>
<artifactId>apt-maven-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ public abstract class ApplicationPersistenceEntity {
private static final long serialVersionUID = 1L;

@Id
@GeneratedValue(strategy = GenerationType.AUTO)
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Version
private Integer modificationCounter;

/**
Expand All @@ -42,7 +43,6 @@ public void setId(Long id) {
this.id = id;
}

@Version
public Integer getModificationCounter() {

return this.modificationCounter;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.devonfw.quarkus.general.service.exception;

public abstract class ApplicationBusinessException extends RuntimeException {

public ApplicationBusinessException() {

super();
}

public ApplicationBusinessException(String message) {

super(message);
}

public ApplicationBusinessException(String message, Exception e) {

super(message, e);
}

public boolean isTechnical() {

return false;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.devonfw.quarkus.general.service.exception;

public class InvalidParameterException extends ApplicationBusinessException {

public InvalidParameterException() {

super();
}

public InvalidParameterException(String message) {

super(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package com.devonfw.quarkus.general.service.exception;

import java.time.ZonedDateTime;
import java.util.HashMap;
import java.util.Map;

import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import javax.ws.rs.core.UriInfo;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;

import lombok.extern.slf4j.Slf4j;

@Provider
@Slf4j
public class RestServiceExceptionFacade implements ExceptionMapper<RuntimeException> {

private boolean exposeInternalErrorDetails = false;

@Context
UriInfo uriInfo;

@Override
public Response toResponse(RuntimeException exception) {

log.error("Exception:{},URL:{},ERROR:{}", exception.getClass().getCanonicalName(), this.uriInfo.getRequestUri(),
exception.getMessage());

if (exception instanceof ApplicationBusinessException) {
return createResponse((ApplicationBusinessException) exception);
}
if (exception instanceof WebApplicationException) {
Copy link
Contributor

Choose a reason for hiding this comment

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

else?

also we should add support for JAX-RS Exceptions (that we have been lacking in devon4j) with e.g. NotFoundException procuding 404.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Quarkus RESTEasy extension provides a mapper for NotFoundException out of the box, that produces a 404 response (https://github.com/quarkusio/quarkus/blob/main/extensions/resteasy-classic/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/NotFoundExceptionMapper.java).
Should we anyway implement and use an own mapper for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

If that already plays together with out exception mapper, it is fully sufficient. But then you could add a nice inline comment to the code giving this valuable information including that link so people can easily understand what is going on. I also was not aware of that so thanks for making this clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

But comming back to my original concern:
How about permission denied? How about validation failures (from BV)?
We do not want to report this as 500 (internal server error)!
This is IMHO only complete if we have all these cases covered.
Ideally we even add a JUnit that is testing all these scenarios to make them transparent and to document our features and behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the Quarkus ExceptionMapper implementations do not return any further information (only status code) I decided to create own implementations. So now we have exception mapper for NotFoundException, UnauthorizedException and ValidationException + our ApplicationBusinessException. Other exceptions are handled as WebApplicationException or as general Exception with status 500
@hohwille is this what you had in mind?

return createResponse((WebApplicationException) exception);
}
return createResponse(exception);
}

private Response createResponse(ApplicationBusinessException exception) {

return createResponse(Status.BAD_REQUEST, Error.APPLICATION_BUSINESS_EXCEPTION, exception);
}

private Response createResponse(WebApplicationException exception) {

Status status = Status.fromStatusCode(exception.getResponse().getStatus());
return createResponse(status, Error.WEB_APPLICATION_EXCEPTION, exception);
}

private Response createResponse(Exception exception) {

return createResponse(Status.INTERNAL_SERVER_ERROR, Error.UNDEFINED_ERROR_CODE, exception);
}

private Response createResponse(Status status, Error errorCode, Exception exception) {

Map<String, Object> jsonMap = new HashMap<>();
jsonMap.put("errorCode", errorCode.name());
if (this.exposeInternalErrorDetails) {
jsonMap.put("message", getExposedErrorDetails(exception));
} else {
jsonMap.put("message", exception.getMessage());
}
jsonMap.put("uri", this.uriInfo.getPath());
jsonMap.put("timestamp", ZonedDateTime.now().toString());
return Response.status(status).type(MediaType.APPLICATION_JSON).entity(jsonMap).build();
}

private String getExposedErrorDetails(Throwable error) {

StringBuilder buffer = new StringBuilder();
Throwable e = error;
while (e != null) {
if (buffer.length() > 0) {
buffer.append(System.lineSeparator());
}
buffer.append(e.getClass().getSimpleName());
buffer.append(": ");
buffer.append(e.getLocalizedMessage());
e = e.getCause();
}
return buffer.toString();
}

public enum Error {

APPLICATION_BUSINESS_EXCEPTION, WEB_APPLICATION_EXCEPTION, UNDEFINED_ERROR_CODE;
}
GuentherJulian marked this conversation as resolved.
Show resolved Hide resolved

}
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
package com.devonfw.quarkus.productmanagement.domain.repo;

import java.util.Optional;

import org.springframework.data.domain.Page;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.CrudRepository;
import org.springframework.data.repository.query.Param;

import com.devonfw.quarkus.productmanagement.domain.model.ProductEntity;

public interface ProductRepository extends CrudRepository<ProductEntity, Long>, ProductFragment {
public interface ProductRepository extends JpaRepository<ProductEntity, Long>, ProductFragment {

@Query("select a from ProductEntity a where title = :title")
ProductEntity findByTitle(@Param("title") String title);
Optional<ProductEntity> findByTitle(@Param("title") String title);

Page<ProductEntity> findAllByOrderByTitle();
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import javax.inject.Inject;
import javax.inject.Named;
import javax.transaction.Transactional;

import org.apache.commons.lang3.StringUtils;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.PageRequest;

import com.devonfw.quarkus.general.service.exception.InvalidParameterException;
import com.devonfw.quarkus.productmanagement.domain.model.ProductEntity;
import com.devonfw.quarkus.productmanagement.domain.repo.ProductRepository;
import com.devonfw.quarkus.productmanagement.service.v1.mapper.ProductMapper;
Expand All @@ -24,15 +27,15 @@
@Slf4j
public class UcFindProductImpl implements UcFindProduct {
@Inject
ProductRepository ProductRepository;
ProductRepository productRepository;

@Inject
ProductMapper mapper;

@Override
public Page<ProductDto> findProducts(ProductSearchCriteriaDto dto) {

Iterable<ProductEntity> productsIterator = this.ProductRepository.findAll();
Iterable<ProductEntity> productsIterator = this.productRepository.findAll();
List<ProductEntity> products = new ArrayList<ProductEntity>();
productsIterator.forEach(products::add);
List<ProductDto> productsDto = this.mapper.map(products);
Expand All @@ -42,49 +45,53 @@ public Page<ProductDto> findProducts(ProductSearchCriteriaDto dto) {
@Override
public Page<ProductDto> findProductsByCriteriaApi(ProductSearchCriteriaDto dto) {

List<ProductEntity> products = this.ProductRepository.findAllCriteriaApi(dto).getContent();
List<ProductEntity> products = this.productRepository.findAllCriteriaApi(dto).getContent();
List<ProductDto> productsDto = this.mapper.map(products);
return new PageImpl<>(productsDto, PageRequest.of(dto.getPageNumber(), dto.getPageSize()), productsDto.size());
}

@Override
public Page<ProductDto> findProductsByQueryDsl(ProductSearchCriteriaDto dto) {

List<ProductEntity> products = this.ProductRepository.findAllQueryDsl(dto).getContent();
List<ProductEntity> products = this.productRepository.findAllQueryDsl(dto).getContent();
List<ProductDto> productsDto = this.mapper.map(products);
return new PageImpl<>(productsDto, PageRequest.of(dto.getPageNumber(), dto.getPageSize()), productsDto.size());
}

@Override
public Page<ProductDto> findProductsByTitleQuery(ProductSearchCriteriaDto dto) {

List<ProductEntity> products = this.ProductRepository.findByTitleQuery(dto).getContent();
List<ProductEntity> products = this.productRepository.findByTitleQuery(dto).getContent();
List<ProductDto> productsDto = this.mapper.map(products);
return new PageImpl<>(productsDto, PageRequest.of(dto.getPageNumber(), dto.getPageSize()), productsDto.size());
}

@Override
public Page<ProductDto> findProductsByTitleNativeQuery(ProductSearchCriteriaDto dto) {

List<ProductEntity> products = this.ProductRepository.findByTitleNativeQuery(dto).getContent();
List<ProductEntity> products = this.productRepository.findByTitleNativeQuery(dto).getContent();
List<ProductDto> productsDto = this.mapper.map(products);
return new PageImpl<>(productsDto, PageRequest.of(dto.getPageNumber(), dto.getPageSize()), productsDto.size());
}

@Override
public Page<ProductDto> findProductsOrderedByTitle() {

List<ProductEntity> products = this.ProductRepository.findAllByOrderByTitle().getContent();
List<ProductEntity> products = this.productRepository.findAllByOrderByTitle().getContent();
List<ProductDto> productsDto = this.mapper.map(products);
return new PageImpl<>(productsDto);
}

@Override
public ProductDto findProduct(String id) {

ProductEntity product = this.ProductRepository.findById(Long.valueOf(id)).get();
if (product != null) {
return this.mapper.map(product);
if (!StringUtils.isNumeric(id)) {
throw new InvalidParameterException("Unable to parse ID: " + id);
}
GuentherJulian marked this conversation as resolved.
Show resolved Hide resolved

Optional<ProductEntity> product = this.productRepository.findById(Long.valueOf(id));
if (product.isPresent()) {
return this.mapper.map(product.get());
} else {
return null;
}
Expand All @@ -93,9 +100,9 @@ public ProductDto findProduct(String id) {
@Override
public ProductDto findProductByTitle(String title) {

ProductEntity product = this.ProductRepository.findByTitle(title);
if (product != null) {
return this.mapper.map(product);
Optional<ProductEntity> product = this.productRepository.findByTitle(title);
if (product.isPresent()) {
return this.mapper.map(product.get());
} else {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
public interface UcManageProduct {
ProductDto saveProduct(NewProductDto dto);

ProductDto deleteProduct(String id);
void deleteProduct(String id);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import javax.inject.Named;
import javax.transaction.Transactional;

import org.apache.commons.lang3.StringUtils;

import com.devonfw.quarkus.general.service.exception.InvalidParameterException;
import com.devonfw.quarkus.productmanagement.domain.model.ProductEntity;
import com.devonfw.quarkus.productmanagement.domain.repo.ProductRepository;
import com.devonfw.quarkus.productmanagement.service.v1.mapper.ProductMapper;
Expand All @@ -27,14 +30,12 @@ public ProductDto saveProduct(NewProductDto dto) {
}

@Override
public ProductDto deleteProduct(String id) {

ProductEntity product = this.productRepository.findById(Long.valueOf(id)).get();
if (product != null) {
this.productRepository.delete(product);
return this.mapper.map(product);
} else {
return null;
public void deleteProduct(String id) {

if (!StringUtils.isNumeric(id)) {
throw new InvalidParameterException("Unable to parse ID: " + id);
}
GuentherJulian marked this conversation as resolved.
Show resolved Hide resolved

this.productRepository.deleteById(Long.valueOf(id));
}
}
Loading