Merge branch 'develop' into 360-modify-notvalid-deaccession-reason

Author: stevenwinship

src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java

@@ -892,6 +892,8 @@ protected <T> T execCommand( Command<T> cmd ) throws WrappedResponse {
             throw new WrappedResponse(ex, badRequest(ex.getMessage(), ex.getFieldErrors()));
         } catch (InvalidCommandArgumentsException ex) {
             throw new WrappedResponse(ex, error(Status.BAD_REQUEST, ex.getMessage()));
+        } catch (ConflictException ex) {
+            throw new WrappedResponse(ex, conflict(ex.getMessage()));
         } catch (CommandException ex) {
             Logger.getLogger(AbstractApiBean.class.getName()).log(Level.SEVERE, "Error while executing command " + cmd, ex);
             throw new WrappedResponse(ex, error(Status.INTERNAL_SERVER_ERROR, ex.getMessage()));

src/main/java/edu/harvard/iq/dataverse/engine/command/exception/ConflictException.java

@@ -0,0 +1,14 @@
+package edu.harvard.iq.dataverse.engine.command.exception;
+
+import edu.harvard.iq.dataverse.engine.command.Command;
+
+/**
+ * An exception raised when a command cannot be executed because it clashes with the current state, e.g. when trying to
+ * create a resource that already exists.
+ */
+public class ConflictException extends CommandException {
+
+    public ConflictException(String message, Command aCommand) {
+        super(message, aCommand);
+    }
+}

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java

@@ -16,6 +16,7 @@
 import edu.harvard.iq.dataverse.engine.command.CommandContext;
 import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
 import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
+import edu.harvard.iq.dataverse.engine.command.exception.ConflictException;
 import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException;
 import edu.harvard.iq.dataverse.util.BundleUtil;
 
@@ -73,7 +74,7 @@ public RoleAssignment execute(CommandContext ctxt) throws CommandException {
         }
     }
     if(isExistingRole(ctxt)){
-        throw new IllegalCommandException(BundleUtil.getStringFromBundle("datasets.api.grant.role.assignee.has.role.error"), this);
+        throw new ConflictException(BundleUtil.getStringFromBundle("datasets.api.grant.role.assignee.has.role.error"), this);
     }
     // TODO make sure the role is defined on the dataverse.
     RoleAssignment roleAssignment = new RoleAssignment(role, grantee, defPoint, privateUrlToken, anonymizedAccess);

src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java

@@ -2867,4 +2867,46 @@ public void testDataverseMetadataLanguage() {
         singleLang.then().assertThat().body("data", equalTo(List.of(Map.of("locale", "en", "title", "English"))));
     }
 
+    @Test
+    public void testAssignRoleOnDataverse() {
+        // Create user
+        Response createUser = UtilIT.createRandomUser();
+        createUser.prettyPrint();
+        String username = UtilIT.getUsernameFromResponse(createUser);
+        String apiToken = UtilIT.getApiTokenFromResponse(createUser);
+
+        // Create collection as that user
+        Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken);
+        createDataverseResponse.prettyPrint();
+        String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse);
+
+        // Create second user
+        Response createSecondUserResponse = UtilIT.createRandomUser();
+        String secondUsername = UtilIT.getUsernameFromResponse(createSecondUserResponse);
+        String secondApiToken = UtilIT.getApiTokenFromResponse(createSecondUserResponse);
+
+        // Let the first user assign a role on their collection to the second user
+        Response grantRoleResponse = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR, "@" + secondUsername, apiToken);
+        grantRoleResponse.prettyPrint();
+        grantRoleResponse.then().assertThat().statusCode(OK.getStatusCode());
+
+        // Let the first user assign the same role to the same user again -> 409 Conflict
+        Response grantRoleTwiceResponse = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR, "@" + secondUsername, apiToken);
+        grantRoleTwiceResponse.prettyPrint();
+        grantRoleTwiceResponse.then().assertThat().statusCode(CONFLICT.getStatusCode());
+
+        // Clean up
+        Response deleteDataverse = UtilIT.deleteDataverse(dataverseAlias, apiToken);
+        deleteDataverse.prettyPrint();
+        deleteDataverse.then().assertThat().statusCode(OK.getStatusCode());
+
+        Response deleteUserResponse = UtilIT.deleteUser(username);
+        deleteUserResponse.prettyPrint();
+        assertEquals(200, deleteUserResponse.getStatusCode());
+
+        Response deleteSecondUserResponse = UtilIT.deleteUser(secondUsername);
+        deleteSecondUserResponse.prettyPrint();
+        assertEquals(200, deleteSecondUserResponse.getStatusCode());
+    }
+
 }

src/test/java/edu/harvard/iq/dataverse/util/cache/CacheFactoryBeanTest.java

@@ -109,7 +109,9 @@ public void init() throws IOException {
             RateLimitUtil.rateLimitMap.clear();
             RateLimitUtil.rateLimits.clear();
         }
-
+        // Reset cache for each test
+        cache.rateLimitCache.clear();
+        
         // Reset to default auth user
         authUser.setRateLimitTier(1);
         authUser.setSuperuser(false);
@@ -163,15 +165,23 @@ public void testAuthenticatedUserGettingRateLimited() throws InterruptedExceptio
         authUser.setRateLimitTier(2); // 120 cals per hour - 1 added token every 30 seconds
         boolean rateLimited = false;
         int cnt;
+        long startTime = System.currentTimeMillis();
         for (cnt = 0; cnt <200; cnt++) {
             rateLimited = !cache.checkRate(authUser, action);
             if (rateLimited) {
                 break;
             }
         }
-        assertTrue(rateLimited);
-        assertEquals(120, cnt);
+        long endTime = System.currentTimeMillis();
+        System.out.println("Test loop took " + (endTime - startTime) + " ms");
+        //Add a few seconds to account for time outside loop
+        long durationMinutes = (6000 + endTime - startTime) / 60000L;
+        // 120 calls/hr = 2 calls/min. Add any tokens that may have been added during the test run
+        long expectedMax = 120 + (durationMinutes * 2);
 
+        assertTrue(rateLimited);
+        assertTrue(cnt >= 120 && cnt <= expectedMax, "cnt was " + cnt + ", expected between 120 and " + expectedMax);
+        
         for (cnt = 0; cnt <60; cnt++) {
             Thread.sleep(1000);// Wait for bucket to be replenished (check each second for 1 minute max)
             rateLimited = !cache.checkRate(authUser, action);