From 6c7de855b40cb72cdda677a98c5311a930079eda Mon Sep 17 00:00:00 2001 From: Sergey Grigoriev Date: Wed, 29 May 2024 14:15:51 +0200 Subject: [PATCH] fix: exceptions are refactored not to use JAX-RS exceptions in services (#18) --- .../exception/ObjectNotFoundException.java | 11 +++++++++ .../NamedSettingsInternalController.java | 4 ++-- .../ObjectNotFoundExceptionMapper.java | 20 ++++++++++++++++ .../generic/service/PolarionService.java | 15 ++++++------ .../settings/NamedSettingsRegistry.java | 4 ++-- .../extension/generic/util/ScopeUtils.java | 6 ++--- .../NamedSettingsInternalControllerTest.java | 12 +++++----- .../generic/service/PolarionServiceTest.java | 24 +++++++++---------- 8 files changed, 62 insertions(+), 34 deletions(-) create mode 100644 app/src/main/java/ch/sbb/polarion/extension/generic/exception/ObjectNotFoundException.java create mode 100644 app/src/main/java/ch/sbb/polarion/extension/generic/rest/exception/ObjectNotFoundExceptionMapper.java diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/exception/ObjectNotFoundException.java b/app/src/main/java/ch/sbb/polarion/extension/generic/exception/ObjectNotFoundException.java new file mode 100644 index 0000000..f7f0693 --- /dev/null +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/exception/ObjectNotFoundException.java @@ -0,0 +1,11 @@ +package ch.sbb.polarion.extension.generic.exception; + +public class ObjectNotFoundException extends RuntimeException { + public ObjectNotFoundException(String message) { + super(message); + } + + public ObjectNotFoundException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalController.java b/app/src/main/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalController.java index 5ccaf99..6d65b51 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalController.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalController.java @@ -1,5 +1,6 @@ package ch.sbb.polarion.extension.generic.rest.controller; +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; import ch.sbb.polarion.extension.generic.service.PolarionService; import ch.sbb.polarion.extension.generic.settings.GenericNamedSettings; import ch.sbb.polarion.extension.generic.settings.NamedSettingsRegistry; @@ -17,7 +18,6 @@ import javax.ws.rs.DELETE; import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; -import javax.ws.rs.NotFoundException; import javax.ws.rs.POST; import javax.ws.rs.PUT; import javax.ws.rs.Path; @@ -113,7 +113,7 @@ public SettingsModel readSetting(@PathParam("feature") String feature, @PathPara if (model != null) { return model; } else { - throw new NotFoundException("Cannot find data using specified parameters"); + throw new ObjectNotFoundException("Cannot find data using specified parameters"); } } diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/rest/exception/ObjectNotFoundExceptionMapper.java b/app/src/main/java/ch/sbb/polarion/extension/generic/rest/exception/ObjectNotFoundExceptionMapper.java new file mode 100644 index 0000000..acb7ab6 --- /dev/null +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/rest/exception/ObjectNotFoundExceptionMapper.java @@ -0,0 +1,20 @@ +package ch.sbb.polarion.extension.generic.rest.exception; + +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; +import com.polarion.core.util.logging.Logger; + +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +@Provider +public class ObjectNotFoundExceptionMapper implements ExceptionMapper { + private static final Logger logger = Logger.getLogger(ObjectNotFoundExceptionMapper.class); + + public Response toResponse(ObjectNotFoundException e) { + logger.error("Not found error in controller: " + e.getMessage(), e); + return Response.status(Response.Status.NOT_FOUND.getStatusCode()) + .entity(e.getMessage()) + .build(); + } +} diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/service/PolarionService.java b/app/src/main/java/ch/sbb/polarion/extension/generic/service/PolarionService.java index 59af19a..75d58a5 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/service/PolarionService.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/service/PolarionService.java @@ -1,5 +1,6 @@ package ch.sbb.polarion.extension.generic.service; +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; import ch.sbb.polarion.extension.generic.fields.ConverterContext; import ch.sbb.polarion.extension.generic.fields.IConverter; import ch.sbb.polarion.extension.generic.fields.model.FieldMetadata; @@ -44,8 +45,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import javax.ws.rs.BadRequestException; -import javax.ws.rs.NotFoundException; import java.security.PrivilegedAction; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; @@ -95,7 +94,7 @@ public IProject getProject(@NotNull String projectId) { @NotNull public IProject getProject(@NotNull String projectId, @Nullable String revision) { if (StringUtils.isEmptyTrimmed(projectId)) { - throw new BadRequestException("Parameter 'projectId' should be provided"); + throw new IllegalArgumentException("Parameter 'projectId' should be provided"); } //Note: it seems that there is no way to get project for already deleted project (unlike workitems/modules & collections) @@ -118,7 +117,7 @@ public IWorkItem getWorkItem(@NotNull String projectId, @NotNull String workItem public IWorkItem getWorkItem(@NotNull String projectId, @NotNull String workItemId, @Nullable String revision) { ITrackerProject trackerProject = getTrackerProject(projectId); if (StringUtils.isEmptyTrimmed(workItemId)) { - throw new BadRequestException("Parameter 'workItemId' should be provided"); + throw new IllegalArgumentException("Parameter 'workItemId' should be provided"); } return getResolvableObjectOrThrow(trackerProject.getWorkItem(workItemId), revision, String.format("WorkItem '%s' not found in project '%s'", workItemId, projectId)); @@ -133,10 +132,10 @@ public IModule getModule(@NotNull String projectId, @NotNull String spaceId, @No public IModule getModule(@NotNull String projectId, @NotNull String spaceId, @NotNull String documentName, @Nullable String revision) { IProject project = getProject(projectId); if (StringUtils.isEmptyTrimmed(spaceId)) { - throw new BadRequestException("Parameter 'spaceId' should be provided"); + throw new IllegalArgumentException("Parameter 'spaceId' should be provided"); } if (StringUtils.isEmptyTrimmed(documentName)) { - throw new BadRequestException("Parameter 'documentName' should be provided"); + throw new IllegalArgumentException("Parameter 'documentName' should be provided"); } ILocation location = Location.getLocation(spaceId + "/" + documentName); @@ -157,7 +156,7 @@ public IBaselineCollection getCollection(@NotNull String projectId, @NotNull Str public IBaselineCollection getCollection(@NotNull String projectId, @NotNull String collectionId, @Nullable String revision) { IProject project = getProject(projectId); if (StringUtils.isEmptyTrimmed(collectionId)) { - throw new BadRequestException("Parameter 'collectionId' should be provided"); + throw new IllegalArgumentException("Parameter 'collectionId' should be provided"); } IBaselineCollection collection = ObjectUtils.requireNotNull( @@ -173,7 +172,7 @@ public IBaselineCollection getCollection(@NotNull String projectId, @NotNull Str public T getResolvableObjectOrThrow(@NotNull IPObject object, @Nullable String revision, @NotNull String notFoundMessage) { return Optional.of(getObjectRevision(object, revision)) .filter(w -> !w.isUnresolvable()) - .orElseThrow(() -> new NotFoundException(notFoundMessage)); + .orElseThrow(() -> new ObjectNotFoundException(notFoundMessage)); } @SuppressWarnings("unchecked") diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/settings/NamedSettingsRegistry.java b/app/src/main/java/ch/sbb/polarion/extension/generic/settings/NamedSettingsRegistry.java index aec2f36..1171294 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/settings/NamedSettingsRegistry.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/settings/NamedSettingsRegistry.java @@ -1,8 +1,8 @@ package ch.sbb.polarion.extension.generic.settings; +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; import lombok.Getter; -import javax.ws.rs.NotFoundException; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -33,7 +33,7 @@ public GenericNamedSettings getByFeatureName(String featureName) { return settingsSet.stream() .filter(s -> s.getFeatureName().equals(featureName)) .findFirst() - .orElseThrow(() -> new NotFoundException("No settings found by featureName: " + featureName)); + .orElseThrow(() -> new ObjectNotFoundException("No settings found by featureName: " + featureName)); } public NamedSettingsRegistry setScopeAgnostic(boolean scopeAgnostic) { diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/util/ScopeUtils.java b/app/src/main/java/ch/sbb/polarion/extension/generic/util/ScopeUtils.java index ade41b5..7b9bc91 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/util/ScopeUtils.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/util/ScopeUtils.java @@ -1,5 +1,6 @@ package ch.sbb.polarion.extension.generic.util; +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; import com.polarion.alm.projects.IProjectService; import com.polarion.alm.projects.internal.model.Project; import com.polarion.alm.projects.model.IProject; @@ -13,7 +14,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import javax.ws.rs.NotFoundException; import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.regex.Matcher; @@ -34,7 +34,7 @@ public static ILocation getContextLocation(@NotNull String scope) { try { return getContextLocationByProject(project); } catch (Exception e) { - throw new NotFoundException("No scope found: " + scope, e); + throw new ObjectNotFoundException("No scope found: " + scope, e); } } } @@ -46,7 +46,7 @@ public static ILocation getContextLocationByProject(@NotNull String projectId) { if (((Project) project).exists()) { return project.getLocation(); } else { - throw new NotFoundException("Project '" + projectId + "' does not exist"); + throw new ObjectNotFoundException("Project '" + projectId + "' does not exist"); } } diff --git a/app/src/test/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalControllerTest.java b/app/src/test/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalControllerTest.java index 1acf3bd..8e340e5 100644 --- a/app/src/test/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalControllerTest.java +++ b/app/src/test/java/ch/sbb/polarion/extension/generic/rest/controller/NamedSettingsInternalControllerTest.java @@ -1,5 +1,6 @@ package ch.sbb.polarion.extension.generic.rest.controller; +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; import ch.sbb.polarion.extension.generic.service.PolarionService; import ch.sbb.polarion.extension.generic.settings.GenericNamedSettings; import ch.sbb.polarion.extension.generic.settings.NamedSettingsRegistry; @@ -13,7 +14,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.junit.jupiter.MockitoExtension; -import javax.ws.rs.NotFoundException; import java.util.List; import static org.junit.jupiter.api.Assertions.*; @@ -50,7 +50,7 @@ void testReadSettingNames() { assertTrue(controller.readSettingNames("feature1", "testScope").containsAll( List.of(generateSettingName(8), generateSettingName(1), generateSettingName(3)))); - assertThrows(NotFoundException.class, () -> controller.readSettingNames("feature2", "testScope")); + assertThrows(ObjectNotFoundException.class, () -> controller.readSettingNames("feature2", "testScope")); } @Test @@ -93,7 +93,7 @@ void testReadRevisionsList() { when(settings.listRevisions(any(), any())).thenReturn(testList); assertEquals(testList, controller.readRevisionsList("feature1", "someName", "testScope")); - NotFoundException exception = assertThrows(NotFoundException.class, + ObjectNotFoundException exception = assertThrows(ObjectNotFoundException.class, () -> controller.readRevisionsList("feature2", "someName", "testScope")); assertEquals("No settings found by featureName: feature2", exception.getMessage()); } @@ -108,7 +108,7 @@ void testDeleteSetting() { verify(settings, times(1)).delete(eq("testScope"), settingIdCaptor.capture()); assertEquals("name1", settingIdCaptor.getValue().getIdentifier()); - NotFoundException exception = assertThrows(NotFoundException.class, + ObjectNotFoundException exception = assertThrows(ObjectNotFoundException.class, () -> controller.deleteSetting("feature2", "someName", "testScope")); assertEquals("No settings found by featureName: feature2", exception.getMessage()); } @@ -121,7 +121,7 @@ void testReadSetting() { assertEquals("feature1Model", settingsModel.getName()); when(settings.read(any(), any(), any())).thenReturn(null); - NotFoundException exception = assertThrows(NotFoundException.class, + ObjectNotFoundException exception = assertThrows(ObjectNotFoundException.class, () -> controller.readSetting("feature1", "name1", "testScope", "1")); assertEquals("Cannot find data using specified parameters", exception.getMessage()); } @@ -186,7 +186,7 @@ void testGetDefaultValues() { assertEquals(model, controller.getDefaultValues("feature1")); - NotFoundException exception = assertThrows(NotFoundException.class, + ObjectNotFoundException exception = assertThrows(ObjectNotFoundException.class, () -> controller.getDefaultValues("feature2")); assertEquals("No settings found by featureName: feature2", exception.getMessage()); } diff --git a/app/src/test/java/ch/sbb/polarion/extension/generic/service/PolarionServiceTest.java b/app/src/test/java/ch/sbb/polarion/extension/generic/service/PolarionServiceTest.java index 94a522b..85b73b2 100644 --- a/app/src/test/java/ch/sbb/polarion/extension/generic/service/PolarionServiceTest.java +++ b/app/src/test/java/ch/sbb/polarion/extension/generic/service/PolarionServiceTest.java @@ -1,5 +1,6 @@ package ch.sbb.polarion.extension.generic.service; +import ch.sbb.polarion.extension.generic.exception.ObjectNotFoundException; import ch.sbb.polarion.extension.generic.fields.FieldType; import ch.sbb.polarion.extension.generic.fields.model.FieldMetadata; import ch.sbb.polarion.extension.generic.fields.model.Option; @@ -36,9 +37,6 @@ import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; -import javax.ws.rs.BadRequestException; -import javax.ws.rs.NotFoundException; - import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -89,7 +87,7 @@ void init() { void testGetNotExistentProject() { mockProject(Boolean.FALSE); - assertThrows(NotFoundException.class, () -> polarionService.getProject(PROJECT_ID, null)); + assertThrows(ObjectNotFoundException.class, () -> polarionService.getProject(PROJECT_ID, null)); } @Test @@ -97,7 +95,7 @@ void testGetNotExistentWorkItem() { IProject project = mockProject(Boolean.TRUE); mockWorkItem(project, Boolean.FALSE); - assertThrows(NotFoundException.class, () -> polarionService.getWorkItem(PROJECT_ID, WI_ID)); + assertThrows(ObjectNotFoundException.class, () -> polarionService.getWorkItem(PROJECT_ID, WI_ID)); } @Test @@ -105,7 +103,7 @@ void testGetNotExistentModule() { IProject project = mockProject(Boolean.TRUE); mockModule(project, Boolean.FALSE); - assertThrows(NotFoundException.class, () -> polarionService.getModule(PROJECT_ID, SPACE_ID, DOCUMENT_NAME)); + assertThrows(ObjectNotFoundException.class, () -> polarionService.getModule(PROJECT_ID, SPACE_ID, DOCUMENT_NAME)); } @Test @@ -115,7 +113,7 @@ void testGetNotExistentCollection() { IBaselineCollection collection = mockCollection(Boolean.FALSE); transactionalExecutorMockedStatic.when(() -> TransactionalExecutor.executeSafelyInReadOnlyTransaction(any())).thenReturn(collection); - assertThrows(NotFoundException.class, () -> polarionService.getCollection(PROJECT_ID, COLLECTION_ID)); + assertThrows(ObjectNotFoundException.class, () -> polarionService.getCollection(PROJECT_ID, COLLECTION_ID)); } } @@ -132,7 +130,7 @@ void testGetProjectWithRevision() { assertNotNull(polarionService.getProject(PROJECT_ID, REVISION)); - BadRequestException exception = assertThrows(BadRequestException.class, + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> polarionService.getProject("", REVISION)); assertEquals("Parameter 'projectId' should be provided", exception.getMessage()); } @@ -168,11 +166,11 @@ void testGetWorkItemWithRevision() { assertNotNull(polarionService.getWorkItem(PROJECT_ID, WI_ID, REVISION)); - BadRequestException exception = assertThrows(BadRequestException.class, + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> polarionService.getWorkItem("", WI_ID, REVISION)); assertEquals("Parameter 'projectId' should be provided", exception.getMessage()); - exception = assertThrows(BadRequestException.class, + exception = assertThrows(IllegalArgumentException.class, () -> polarionService.getWorkItem(PROJECT_ID, "", REVISION)); assertEquals("Parameter 'workItemId' should be provided", exception.getMessage()); } @@ -192,11 +190,11 @@ void testGetModuleWithRevision() { assertNotNull(polarionService.getModule(PROJECT_ID, SPACE_ID, DOCUMENT_NAME, REVISION)); - BadRequestException exception = assertThrows(BadRequestException.class, + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> polarionService.getModule(PROJECT_ID, "", DOCUMENT_NAME, REVISION)); assertEquals("Parameter 'spaceId' should be provided", exception.getMessage()); - exception = assertThrows(BadRequestException.class, + exception = assertThrows(IllegalArgumentException.class, () -> polarionService.getModule(PROJECT_ID, SPACE_ID, "", REVISION)); assertEquals("Parameter 'documentName' should be provided", exception.getMessage()); } @@ -221,7 +219,7 @@ void testGetCollectionWithRevision() { assertNotNull(polarionService.getCollection(PROJECT_ID, COLLECTION_ID, REVISION)); - BadRequestException exception = assertThrows(BadRequestException.class, + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> polarionService.getCollection(PROJECT_ID, "", REVISION)); assertEquals("Parameter 'collectionId' should be provided", exception.getMessage()); }