7159 fix restassured etc

src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java

@@ -780,21 +780,37 @@ public WorkflowComment addWorkflowComment(WorkflowComment workflowComment) {
         return workflowComment;
     }
 
+    /**
+     * This method used to throw CommandException, which was pretty pointless 
+     * seeing how it's called asynchronously. As of v5.0 any CommanExceptiom 
+     * thrown by the FinalizeDatasetPublicationCommand below will be caught 
+     * and we'll log it as a warning - which is the best we can do at this point.
+     * Any failure notifications to users should be sent from inside the command.
+     */
     @Asynchronous
-    public void callFinalizePublishCommandAsynchronously(Long datasetId, CommandContext ctxt, DataverseRequest request, boolean isPidPrePublished) throws CommandException {
+    @TransactionAttribute(TransactionAttributeType.SUPPORTS)
+    public void callFinalizePublishCommandAsynchronously(Long datasetId, CommandContext ctxt, DataverseRequest request, boolean isPidPrePublished) {
 
         // Since we are calling the next command asynchronously anyway - sleep here 
         // for a few seconds, just in case, to make sure the database update of 
         // the dataset initiated by the PublishDatasetCommand has finished, 
         // to avoid any concurrency/optimistic lock issues. 
+        // Aug. 2020/v5.0: It MAY be working consistently without any 
+        // sleep here, after the call the method has been moved to the onSuccess()
+        // portion of the PublishDatasetCommand. I'm going to leave the 1 second
+        // sleep below, for just in case reasons: -- L.A.
         try {
-            Thread.sleep(15000);
+            Thread.sleep(1000);
         } catch (Exception ex) {
-            logger.warning("Failed to sleep for 15 seconds.");
+            logger.warning("Failed to sleep for a second.");
         }
         logger.fine("Running FinalizeDatasetPublicationCommand, asynchronously");
         Dataset theDataset = find(datasetId);
-        commandEngine.submit(new FinalizeDatasetPublicationCommand(theDataset, request, isPidPrePublished));
+        try {
+            commandEngine.submit(new FinalizeDatasetPublicationCommand(theDataset, request, isPidPrePublished));
+        } catch (CommandException cex) {
+            logger.warning("CommandException caught when executing the asynchronous portion of the Dataset Publication Command.");
+        }
     }
 
     /*

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

@@ -1020,7 +1020,7 @@ public Response publishDataset(@PathParam("id") String id, @QueryParam("type") S
             PublishDatasetResult res = execCommand(new PublishDatasetCommand(ds,
                         createDataverseRequest(user),
                     isMinor));
-            return res.isCompleted() ? ok(json(res.getDataset())) : accepted(json(res.getDataset()));
+            return res.isWorkflow() ? accepted(json(res.getDataset())) : ok(json(res.getDataset()));
             }
         } catch (WrappedResponse ex) {
             return ex.getResponse();

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

@@ -173,15 +173,6 @@ public Dataset execute(CommandContext ctxt) throws CommandException {
             theDataset.getLatestVersion().setVersionState(RELEASED);
         }
 
-
-        // Remove locks
-        ctxt.engine().submit(new RemoveLockCommand(getRequest(), theDataset, DatasetLock.Reason.Workflow));
-        ctxt.engine().submit(new RemoveLockCommand(getRequest(), theDataset, DatasetLock.Reason.finalizePublication));
-        if ( theDataset.isLockedFor(DatasetLock.Reason.InReview) ) {
-            ctxt.engine().submit( 
-                    new RemoveLockCommand(getRequest(), theDataset, DatasetLock.Reason.InReview) );
-        }
-
         final Dataset ds = ctxt.em().merge(theDataset);
 
         ctxt.workflows().getDefaultWorkflow(TriggerType.PostPublishDataset).ifPresent(wf -> {
@@ -199,6 +190,13 @@ public Dataset execute(CommandContext ctxt) throws CommandException {
             notifyUsersDatasetPublishStatus(ctxt, theDataset, UserNotification.Type.PUBLISHEDDS);
         }
 
+        // Finally, unlock the dataset:
+        ctxt.datasets().removeDatasetLocks(theDataset, DatasetLock.Reason.Workflow);
+        ctxt.datasets().removeDatasetLocks(theDataset, DatasetLock.Reason.finalizePublication);
+        if ( theDataset.isLockedFor(DatasetLock.Reason.InReview) ) {
+            ctxt.datasets().removeDatasetLocks(theDataset, DatasetLock.Reason.InReview);
+        }
+
         return readyDataset;
     }
 
@@ -211,18 +209,20 @@ public boolean onSuccess(CommandContext ctxt, Object r) {
         } catch (ClassCastException e){
             dataset  = ((PublishDatasetResult) r).getDataset();
         }
-
+        
         try {
             Future<String> indexString = ctxt.index().indexDataset(dataset, true);                   
         } catch (IOException | SolrServerException e) {    
-            String failureLogText = "Post-publication indexing failed. You can kickoff a re-index of this dataset with: \r\n curl http://localhost:8080/api/admin/index/datasets/" + dataset.getId().toString();
+            String failureLogText = "Post-publication indexing failed. You can kick off a re-index of this dataset with: \r\n curl http://localhost:8080/api/admin/index/datasets/" + dataset.getId().toString();
             failureLogText += "\r\n" + e.getLocalizedMessage();
             LoggingUtil.writeOnSuccessFailureLog(this, failureLogText,  dataset);
             retVal = false;
         }
 
         exportMetadata(dataset, ctxt.settings());
+
         ctxt.datasets().updateLastExportTimeStamp(dataset.getId());
+
         return retVal;
     }
 

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

@@ -21,6 +21,7 @@
 import java.util.Optional;
 import java.util.logging.Logger;
 import static java.util.stream.Collectors.joining;
+import static edu.harvard.iq.dataverse.engine.command.impl.PublishDatasetResult.Status;
 
 /**
  * Kick-off a dataset publication process. The process may complete immediately, 
@@ -91,7 +92,7 @@ public PublishDatasetResult execute(CommandContext ctxt) throws CommandException
             theDataset = ctxt.em().merge(theDataset);
             ctxt.em().flush();
             ctxt.workflows().start(prePubWf.get(), buildContext(theDataset, TriggerType.PrePublishDataset, datasetExternallyReleased));
-            return new PublishDatasetResult(theDataset, false);
+            return new PublishDatasetResult(theDataset, Status.Workflow);
 
         } else{
             // We will skip trying to register the global identifiers for datafiles 
@@ -135,16 +136,18 @@ public PublishDatasetResult execute(CommandContext ctxt) throws CommandException
             lock.setInfo(info);
             ctxt.datasets().addDatasetLock(theDataset, lock);
             theDataset = ctxt.em().merge(theDataset);
-            ctxt.datasets().callFinalizePublishCommandAsynchronously(theDataset.getId(), ctxt, request, datasetExternallyReleased);
-            return new PublishDatasetResult(theDataset, false);
+            // The call to FinalizePublicationCommand has been moved to the new @onSuccess()
+            // method:
+            //ctxt.datasets().callFinalizePublishCommandAsynchronously(theDataset.getId(), ctxt, request, datasetExternallyReleased);
+            return new PublishDatasetResult(theDataset, Status.Inprogress);
 
             /**
               * Code for for "synchronous" (while-you-wait) publishing 
               * is preserved below, commented out:
             } else {
                 // Synchronous publishing (no workflow involved)
                 theDataset = ctxt.engine().submit(new FinalizeDatasetPublicationCommand(theDataset, getRequest(),datasetExternallyReleased));
-                return new PublishDatasetResult(theDataset, true);
+                return new PublishDatasetResult(theDataset, Status.Completed);
             } */
         }
     }
@@ -196,6 +199,24 @@ private void verifyCommandArguments() throws IllegalCommandException {
                 throw new IllegalCommandException("Cannot release as minor version. Re-try as major release.", this);
             }
         }
-    }   
+    }
+
+
+    @Override
+    public boolean onSuccess(CommandContext ctxt, Object r) {
+        Dataset dataset = null;
+        try{
+            dataset = (Dataset) r;
+        } catch (ClassCastException e){
+            dataset  = ((PublishDatasetResult) r).getDataset();
+        }
+
+        if (dataset != null) {
+            ctxt.datasets().callFinalizePublishCommandAsynchronously(dataset.getId(), ctxt, request, datasetExternallyReleased);
+            return true;
+        }
+
+        return false;
+    }
 
 }

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

@@ -11,11 +11,21 @@
 public class PublishDatasetResult {
 
     private final Dataset dataset;
-    private final boolean isCompleted;
+    private final Status status;
+
+    public enum Status {
+        /** Dataset has been published */
+        Completed,
+        /** Publish workflow is in progress */
+        Workflow,
+        /** Publishing is being finalized asynchronously */
+        Inprogress
+    }
+
 
-    public PublishDatasetResult(Dataset dataset, boolean isCompleted) {
+    public PublishDatasetResult(Dataset dataset, Status status) {
         this.dataset = dataset;
-        this.isCompleted = isCompleted;
+        this.status = status;
     }
 
     /**
@@ -34,7 +44,14 @@ public Dataset getDataset() {
      * @return {@code true} iff the publication process was completed. {@code false} otherwise.
      */
     public boolean isCompleted() {
-        return isCompleted;
+        return status == Status.Completed;
     }
 
+    public boolean isWorkflow() {
+        return status == Status.Workflow;
+    }
+
+    public boolean isInProgress() {
+        return status == Status.Inprogress;
+    }
 }

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

@@ -51,7 +51,8 @@ public class UtilIT {
     private static final String BUILTIN_USER_KEY = "burrito";
     private static final String EMPTY_STRING = "";
     public static final int MAXIMUM_INGEST_LOCK_DURATION = 3;
-
+    public static final int MAXIMUM_PUBLISH_LOCK_DURATION = 15;
+
     private static SwordConfigurationImpl swordConfiguration = new SwordConfigurationImpl();
 
     static Matcher<String> equalToCI( String value ) {
@@ -1017,10 +1018,14 @@ static Response deleteFile(Integer fileId, String apiToken) {
     }
 
     static Response publishDatasetViaSword(String persistentId, String apiToken) {
-        return given()
+        Response publishResponse =  given()
                 .auth().basic(apiToken, EMPTY_STRING)
                 .header("In-Progress", "false")
                 .post(swordConfiguration.getBaseUrlPathCurrent() + "/edit/study/" + persistentId);
+
+        // Wait for the dataset to get unlocked, if/as needed:
+        sleepForLock(persistentId, "finalizePublication", apiToken, UtilIT.MAXIMUM_INGEST_LOCK_DURATION);            
+        return publishResponse;
     }
 
     static Response publishDatasetViaNativeApi(String idOrPersistentId, String majorOrMinor, String apiToken) {
@@ -1036,18 +1041,36 @@ static Response publishDatasetViaNativeApi(String idOrPersistentId, String major
                     .urlEncodingEnabled(false)
                     .header(UtilIT.API_TOKEN_HTTP_HEADER, apiToken);
         }
-        return requestSpecification.post("/api/datasets/" + idInPath + "/actions/:publish?type=" + majorOrMinor + optionalQueryParam);
+        Response publishResponse = requestSpecification.post("/api/datasets/" + idInPath + "/actions/:publish?type=" + majorOrMinor + optionalQueryParam);
+
+        // Wait for the dataset to get unlocked, if/as needed:
+        sleepForLock(idOrPersistentId, "finalizePublication", apiToken, UtilIT.MAXIMUM_INGEST_LOCK_DURATION);            
+
+        return publishResponse;
     }
 
     static Response publishDatasetViaNativeApiDeprecated(String persistentId, String majorOrMinor, String apiToken) {
         /**
          * @todo This should be a POST rather than a GET:
          * https://github.com/IQSS/dataverse/issues/2431
          */
-        return given()
+        Response publishResponse = given()
                 .header(API_TOKEN_HTTP_HEADER, apiToken)
                 .urlEncodingEnabled(false)
                 .get("/api/datasets/:persistentId/actions/:publish?type=" + majorOrMinor + "&persistentId=" + persistentId);
+
+        // Wait for the dataset to get unlocked, if/as needed:
+        sleepForLock(persistentId, "finalizePublication", apiToken, UtilIT.MAXIMUM_INGEST_LOCK_DURATION);            
+
+        return publishResponse;
+    }
+
+    // Compatibility method wrapper (takes Integer for the dataset id) 
+    // It used to use the GET version of the publish API; I'm switching it to 
+    // use the new POST variant. The explicitly marked @deprecated method above
+    // should be sufficient for testing the deprecated API call. 
+    static Response publishDatasetViaNativeApi(Integer datasetId, String majorOrMinor, String apiToken) {
+        return publishDatasetViaNativeApi(datasetId.toString(), majorOrMinor, apiToken);
     }
 
     static Response modifyDatasetPIDMetadataViaApi(String persistentId,  String apiToken) {
@@ -1061,17 +1084,6 @@ static Response modifyDatasetPIDMetadataViaApi(String persistentId,  String apiT
                 .get("/api/datasets/:persistentId/&persistentId=" + persistentId);
     }
 
-    static Response publishDatasetViaNativeApi(Integer datasetId, String majorOrMinor, String apiToken) {
-        /**
-         * @todo This should be a POST rather than a GET:
-         * https://github.com/IQSS/dataverse/issues/2431
-         */
-        return given()
-                .header(API_TOKEN_HTTP_HEADER, apiToken)
-                .urlEncodingEnabled(false)
-                .get("/api/datasets/" + datasetId + "/actions/:publish?type=" + majorOrMinor);
-    }
-
     static Response publishDataverseViaSword(String alias, String apiToken) {
         return given()
                 .auth().basic(apiToken, EMPTY_STRING)
@@ -2187,13 +2199,20 @@ public void testSwordStatementWithFiles() {
 
     //Helper function that returns true if a given dataset locked for a  given reason is unlocked within
     // a given duration returns false if still locked after given duration
+    // (the version of the method that takes a long for the dataset id is 
+    // for backward compatibility with how the method is called for the 
+    // Ingest lock throughout the test code)
     static Boolean sleepForLock(long datasetId, String lockType, String apiToken, int duration) {
+        String datasetIdAsString = String.valueOf(datasetId);
+        return sleepForLock(datasetIdAsString, lockType, apiToken, duration);
+    }
 
-        Response lockedForIngest = UtilIT.checkDatasetLocks(datasetId, lockType, apiToken);
+    static Boolean sleepForLock(String idOrPersistentId, String lockType, String apiToken, int duration) {
+        Response lockedForIngest = UtilIT.checkDatasetLocks(idOrPersistentId, lockType, apiToken);
         int i = 0;
         do {
             try {
-                lockedForIngest = UtilIT.checkDatasetLocks(datasetId, lockType, apiToken);
+                lockedForIngest = UtilIT.checkDatasetLocks(idOrPersistentId, lockType, apiToken);
                 Thread.sleep(1000);
                 i++;
                 if (i > duration) {
@@ -2232,12 +2251,27 @@ static Boolean sleepForSearch(String searchPart, String apiToken,  String subTre
 
     }
 
-
-
+    // backward compatibility version of the method that takes long for the id:
     static Response checkDatasetLocks(long datasetId, String lockType, String apiToken) {
+        String datasetIdAsString = String.valueOf(datasetId);
+        return checkDatasetLocks(datasetIdAsString, lockType, apiToken);
+    }
+
+    static Response checkDatasetLocks(String idOrPersistentId, String lockType, String apiToken) {
+        String idInPath = idOrPersistentId; // Assume it's a number.
+        String queryParams = ""; // If idOrPersistentId is a number we'll just put it in the path.
+        if (!NumberUtils.isNumber(idOrPersistentId)) {
+            idInPath = ":persistentId";
+            queryParams = "?persistentId=" + idOrPersistentId;
+        }
+
+        if (lockType != null) {
+            queryParams = "".equals(queryParams) ? "?type="+lockType : queryParams+"&type="+lockType;
+        }
+
         Response response = given()
             .header(API_TOKEN_HTTP_HEADER, apiToken)
-            .get("api/datasets/" + datasetId + "/locks" + (lockType == null ? "" : "?type="+lockType));
+            .get("api/datasets/" + idInPath + "/locks" + queryParams);
         return response;       
     }