You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by bd...@apache.org on 2022/09/07 00:47:06 UTC

[directory-scimple] branch remove-repo-handle-exception created (now a803e4b)

This is an automated email from the ASF dual-hosted git repository.

bdemers pushed a change to branch remove-repo-handle-exception
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git


      at a803e4b  Remove web specific method from Repository interface

This branch includes the following new commits:

     new a803e4b  Remove web specific method from Repository interface

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[directory-scimple] 01/01: Remove web specific method from Repository interface

Posted by bd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

bdemers pushed a commit to branch remove-repo-handle-exception
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git

commit a803e4b1a8b9397bae62d6e6d6584b83b7432c7a
Author: Brian Demers <bd...@apache.org>
AuthorDate: Tue Sep 6 20:46:59 2022 -0400

    Remove web specific method from Repository interface
    
    Exception handling should ultimately move to ExceptionMapper implementations
---
 .../scim/server/repository/Repository.java         |  26 -----
 .../server/rest/BaseResourceTypeResourceImpl.java  |  26 +++--
 .../scim/server/repository/RepositoryTest.java     | 105 ---------------------
 .../rest/BaseResourceTypeResourceImplTest.java     |  37 ++++++++
 4 files changed, 54 insertions(+), 140 deletions(-)

diff --git a/scim-server/src/main/java/org/apache/directory/scim/server/repository/Repository.java b/scim-server/src/main/java/org/apache/directory/scim/server/repository/Repository.java
index e3cc7da..212e56b 100644
--- a/scim-server/src/main/java/org/apache/directory/scim/server/repository/Repository.java
+++ b/scim-server/src/main/java/org/apache/directory/scim/server/repository/Repository.java
@@ -21,16 +21,11 @@ package org.apache.directory.scim.server.repository;
 
 import java.util.List;
 
-import jakarta.ws.rs.WebApplicationException;
-import jakarta.ws.rs.core.Response;
-import jakarta.ws.rs.core.Response.Status;
-
 import org.apache.directory.scim.server.exception.UnableToCreateResourceException;
 import org.apache.directory.scim.server.exception.UnableToDeleteResourceException;
 import org.apache.directory.scim.server.exception.UnableToRetrieveExtensionsResourceException;
 import org.apache.directory.scim.server.exception.UnableToRetrieveResourceException;
 import org.apache.directory.scim.server.exception.UnableToUpdateResourceException;
-import org.apache.directory.scim.server.rest.BaseResourceTypeResourceImpl;
 import org.apache.directory.scim.spec.protocol.filter.FilterResponse;
 import org.apache.directory.scim.spec.protocol.search.Filter;
 import org.apache.directory.scim.spec.protocol.search.PageRequest;
@@ -127,25 +122,4 @@ public interface Repository<T extends ScimResource> {
    *         the appropriate list.
    */
   List<Class<? extends ScimExtension>> getExtensionList() throws UnableToRetrieveExtensionsResourceException;
-
-  /**
-   * <p>In the case where the repository throws an unhandled exception, this
-   * method will be passed that exception in order for the repository to convert
-   * it into the desired response.</p>
-   * <p>The returned response SHOULD fulfill the requirements for SCIM error
-   * responses as defined in <a
-   * href="https://tools.ietf.org/html/rfc7644#section-3.12">3.12. HTTP Status
-   * and Error Response Handling</a> of the SCIM specification.</p>
-   * <p>By default, exceptions are converted into a <code>500 Internal Server
-   * Error</code>.</p>
-   * @param unhandled
-   * @return
-   */
-  default Response handleException(Throwable unhandled) {
-    // Allow for ErrorMessageViolationExceptionMapper to handle JAX-RS exceptions by default
-    if (unhandled instanceof WebApplicationException) {
-      throw (WebApplicationException) unhandled;
-    }
-    return BaseResourceTypeResourceImpl.createGenericExceptionResponse(unhandled, Status.INTERNAL_SERVER_ERROR);
-  }
 }
diff --git a/scim-server/src/main/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImpl.java b/scim-server/src/main/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImpl.java
index 4c46acc..6d2e860 100644
--- a/scim-server/src/main/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImpl.java
+++ b/scim-server/src/main/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImpl.java
@@ -19,7 +19,6 @@
 
 package org.apache.directory.scim.server.rest;
 
-import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.security.NoSuchAlgorithmException;
@@ -30,6 +29,7 @@ import java.util.Optional;
 import java.util.Set;
 
 import jakarta.enterprise.inject.spi.CDI;
+import jakarta.ws.rs.WebApplicationException;
 import jakarta.ws.rs.core.EntityTag;
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.Response.ResponseBuilder;
@@ -128,7 +128,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e) {
         log.error("Uncaught repository exception", e);
 
-        return repository.handleException(e);
+        return handleException(e);
       }
 
       if (resource != null) {
@@ -266,7 +266,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e) {
         log.error("Uncaught repository exception", e);
 
-        return repository.handleException(e);
+        return handleException(e);
       }
 
       EntityTag etag = null;
@@ -352,7 +352,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e) {
         log.error("Uncaught repository exception", e);
 
-        return repository.handleException(e);
+        return handleException(e);
       }
 
       // If no resources are found, we should still return a ListResponse with
@@ -441,7 +441,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e) {
         log.error("Uncaught repository exception", e);
 
-        return repository.handleException(e);
+        return handleException(e);
       }
 
       if (stored == null) {
@@ -470,7 +470,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e1) {
         log.error("Uncaught repository exception", e1);
 
-        return repository.handleException(e1);
+        return handleException(e1);
       }
 
       // Process Attributes
@@ -541,7 +541,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e) {
         log.error("Uncaught repository exception", e);
 
-        return repository.handleException(e);
+        return handleException(e);
       }
 
       if (stored == null) {
@@ -572,7 +572,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e1) {
         log.error("Uncaught repository exception", e1);
 
-        return repository.handleException(e1);
+        return handleException(e1);
       }
 
       // Process Attributes
@@ -642,7 +642,7 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
       } catch (Exception e) {
         log.error("Uncaught repository exception", e);
 
-        return repository.handleException(e);
+        return handleException(e);
       }
     } catch (ScimServerException sse) {
       LOG.error("Error Processing SCIM Request", sse);
@@ -724,4 +724,12 @@ public abstract class BaseResourceTypeResourceImpl<T extends ScimResource> imple
     return evaluatePreconditionsResponse.entity(er)
                                         .build();
   }
+
+  Response handleException(Throwable unhandled) {
+    // Allow for ErrorMessageViolationExceptionMapper to handle JAX-RS exceptions by default
+    if (unhandled instanceof WebApplicationException) {
+      throw (WebApplicationException) unhandled;
+    }
+    return BaseResourceTypeResourceImpl.createGenericExceptionResponse(unhandled, Status.INTERNAL_SERVER_ERROR);
+  }
 }
diff --git a/scim-server/src/test/java/org/apache/directory/scim/server/repository/RepositoryTest.java b/scim-server/src/test/java/org/apache/directory/scim/server/repository/RepositoryTest.java
deleted file mode 100644
index c60fcc4..0000000
--- a/scim-server/src/test/java/org/apache/directory/scim/server/repository/RepositoryTest.java
+++ /dev/null
@@ -1,105 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
-
- * http://www.apache.org/licenses/LICENSE-2.0
-
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.directory.scim.server.repository;
-
-import org.apache.directory.scim.spec.protocol.data.ErrorResponse;
-import org.apache.directory.scim.spec.protocol.filter.FilterResponse;
-import org.apache.directory.scim.spec.protocol.search.Filter;
-import org.apache.directory.scim.spec.protocol.search.PageRequest;
-import org.apache.directory.scim.spec.protocol.search.SortRequest;
-import org.apache.directory.scim.spec.resources.ScimExtension;
-import org.apache.directory.scim.spec.resources.ScimResource;
-import org.junit.jupiter.api.Test;
-
-import jakarta.ws.rs.WebApplicationException;
-import jakarta.ws.rs.core.Response;
-import java.util.List;
-
-import static org.hamcrest.MatcherAssert.assertThat;
-
-import static com.googlecode.catchexception.CatchException.catchException;
-import static com.googlecode.catchexception.CatchException.caughtException;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.sameInstance;
-
-public class RepositoryTest {
-
-  @Test
-  public void handleException_jaxrsExceptionTest() {
-
-    Exception e = new WebApplicationException();
-    catchException(() -> new RepositoryAdapter().handleException(e));
-    assertThat(caughtException(), sameInstance(e));
-  }
-
-  @Test
-  public void handleException_runtimeExceptionTest() {
-
-    Exception e = new RuntimeException("fake test exception");
-    Response response = new RepositoryAdapter().handleException(e);
-    assertThat(response.getStatus(), is(500));
-    assertThat(((ErrorResponse)response.getEntity()).getDetail(), is("fake test exception"));
-  }
-
-  @Test
-  public void handleException_nullExceptionTest() {
-
-    Response response = new RepositoryAdapter().handleException(null);
-    assertThat(response.getStatus(), is(500));
-    assertThat(((ErrorResponse)response.getEntity()).getDetail(), is("Unknown Server Error"));
-  }
-
-  private class RepositoryAdapter implements Repository<ScimResource> {
-
-    @Override
-    public Class<ScimResource> getResourceClass() {
-      return ScimResource.class;
-    }
-
-    @Override
-    public ScimResource create(ScimResource resource) {
-      return null;
-    }
-
-    @Override
-    public ScimResource update(UpdateRequest<ScimResource> updateRequest) {
-      return null;
-    }
-
-    @Override
-    public ScimResource get(String id) {
-      return null;
-    }
-
-    @Override
-    public FilterResponse<ScimResource> find(Filter filter, PageRequest pageRequest, SortRequest sortRequest) {
-      return null;
-    }
-
-    @Override
-    public void delete(String id) {
-
-    }
-
-    @Override
-    public List<Class<? extends ScimExtension>> getExtensionList() {
-      return null;
-    }
-  }
-}
diff --git a/scim-server/src/test/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImplTest.java b/scim-server/src/test/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImplTest.java
index 35ea18a..f562ca4 100644
--- a/scim-server/src/test/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImplTest.java
+++ b/scim-server/src/test/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImplTest.java
@@ -19,6 +19,11 @@
 
 package org.apache.directory.scim.server.rest;
 
+import static com.googlecode.catchexception.CatchException.catchException;
+import static com.googlecode.catchexception.CatchException.caughtException;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
@@ -28,6 +33,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
+import jakarta.ws.rs.WebApplicationException;
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.Response.Status;
@@ -214,6 +220,37 @@ public class BaseResourceTypeResourceImplTest {
     assertTrue(response.getEntity() instanceof ErrorResponse);
     assertTrue(((ErrorResponse)response.getEntity()).getDetail().equals("Cannot include both attributes and excluded attributes in a single request"));
   }
+
+  @Test
+  public void handleException_jaxrsExceptionTest() {
+    BaseResourceTypeResourceImpl<ScimUser> baseResourceImpl = mock(BaseResourceTypeResourceImpl.class);
+    when(baseResourceImpl.handleException(any())).thenCallRealMethod();
+
+    Exception e = new WebApplicationException();
+    catchException(() -> baseResourceImpl.handleException(e));
+    assertThat(caughtException(), sameInstance(e));
+  }
+
+  @Test
+  public void handleException_runtimeExceptionTest() {
+    BaseResourceTypeResourceImpl<ScimUser> baseResourceImpl = mock(BaseResourceTypeResourceImpl.class);
+    when(baseResourceImpl.handleException(any())).thenCallRealMethod();
+
+    Exception e = new RuntimeException("fake test exception");
+    Response response = baseResourceImpl.handleException(e);
+    assertThat(response.getStatus(), is(500));
+    assertThat(((ErrorResponse)response.getEntity()).getDetail(), is("fake test exception"));
+  }
+
+  @Test
+  public void handleException_nullExceptionTest() {
+    BaseResourceTypeResourceImpl<ScimUser> baseResourceImpl = mock(BaseResourceTypeResourceImpl.class);
+    when(baseResourceImpl.handleException(any())).thenCallRealMethod();
+
+    Response response = baseResourceImpl.handleException(null);
+    assertThat(response.getStatus(), is(500));
+    assertThat(((ErrorResponse)response.getEntity()).getDetail(), is("Unknown Server Error"));
+  }
   
   private ScimUser getScimUser() throws PhoneNumberParseException {
     ScimUser user = new ScimUser();