Allow updates of single data sets #934

grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy

@@ -0,0 +1,30 @@
+package au.org.ala.ecodata
+
+import org.apache.http.HttpStatus
+
+class DataSetSummaryController {
+
+    static responseFormats = ['json', 'xml']
+    static allowedMethods = [update:['POST', 'PUT']]
+
+    ProjectService projectService
+
+    /** Updates a single dataset for a project */
+    def update(String projectId) {
+        Map dataSet = request.JSON
+
+        if (!projectId && !dataSet.projectId) {
+            respond status: 400, message: "projectId is required"
+            return
+        }
+
+        projectId = projectId || dataSet.projectId
+
+        if (dataSet.projectId && dataSet.projectId != projectId) {
+            respond status: HttpStatus.SC_BAD_REQUEST, message: "projectId must match the data set projectId"
+            return
+        }
+
+        respond projectService.updateDataSet(projectId, dataSet)
+    }
+}

grails-app/services/au/org/ala/ecodata/ParatooService.groovy

@@ -18,7 +18,7 @@ import static grails.async.Promises.task
  */
 @Slf4j
 class ParatooService {
-    static final Object LOCK = new Object()
+
     static final String DATASET_DATABASE_TABLE = 'Database Table'
     static final int PARATOO_MAX_RETRIES = 3
     static final String PARATOO_PROTOCOL_PATH = '/protocols'
@@ -186,18 +186,7 @@ class ParatooService {
         dataSet.orgMintedIdentifier = paratooCollectionId.encodeAsOrgMintedIdentifier()
 
         log.info "Minting identifier for Monitor collection: ${paratooCollectionId}: ${dataSet.orgMintedIdentifier}"
-        Map result
-        synchronized (LOCK) {
-            Map latestProject = projectService.get(projectId)
-            if (!latestProject.custom) {
-                latestProject.custom = [:]
-            }
-            if (!latestProject.custom.dataSets) {
-                latestProject.custom.dataSets = []
-            }
-            latestProject.custom.dataSets << dataSet
-            result = projectService.update([custom: latestProject.custom], projectId, false)
-        }
+        Map result = projectService.updateDataSet(projectId, dataSet)
 
         if (!result.error) {
             result.orgMintedIdentifier = dataSet.orgMintedIdentifier
@@ -239,18 +228,12 @@ class ParatooService {
             log.error("An error occurred feching ${collection.orgMintedUUID}: ${e.message}", e)
             userService.clearCurrentUser()
         }
-      
+
         promise.onComplete { Map result ->
             userService.clearCurrentUser()
         }
 
-        def result
-        synchronized (LOCK) {
-            Map latestProject = projectService.get(project.id)
-            Map latestDataSet = latestProject.custom?.dataSets?.find { it.dataSetId == collection.orgMintedUUID }
-            latestDataSet.putAll(dataSet)
-            result = projectService.update([custom: latestProject.custom], project.id, false)
-        }
+        def result = projectService.updateDataSet(project.id, dataSet)
 
         [updateResult: result, promise: promise]
     }
@@ -320,12 +303,7 @@ class ParatooService {
                 dataSet.areSpeciesRecorded = records?.size() > 0
                 dataSet.activityId = activityId
 
-                synchronized (LOCK) {
-                    Map latestProject = projectService.get(project.project.projectId)
-                    Map latestDataSet = latestProject.custom?.dataSets?.find { it.dataSetId == collection.orgMintedUUID }
-                    latestDataSet.putAll(dataSet)
-                    projectService.update([custom: latestProject.custom], project.id, false)
-                }
+                projectService.updateDataSet(project.id, dataSet)
             }
         }
     }

grails-app/services/au/org/ala/ecodata/ProjectService.groovy

@@ -26,6 +26,11 @@ class ProjectService {
     static final ENHANCED = 'enhanced'
     static final PRIVATE_SITES_REMOVED = 'privatesitesremoved'
 
+    /** A Map containing a per-project lock for synchronizing locks for updates.  The purpose of this
+     * is to support concurrent edits on different project data set summaries which are currently modelled as
+     * an embedded array but can be added and updated by both the UI and the Monitor (Parataoo) application API */
+    static final Map PROJECT_UPDATE_LOCKS = Collections.synchronizedMap([:].withDefault{ new Object() })
+
     GrailsApplication grailsApplication
     MessageSource messageSource
     SessionLocaleResolver localeResolver
@@ -51,6 +56,8 @@ class ProjectService {
         grailsApplication.mainContext.commonService
     }*/
 
+
+
     def getBrief(listOfIds, version = null) {
         if (listOfIds) {
             if (version) {
@@ -469,30 +476,32 @@ class ProjectService {
     }
 
     def update(Map props, String id, Boolean shouldUpdateCollectory = true) {
-        Project project = Project.findByProjectId(id)
-        if (project) {
-            // retrieve any project activities associated with the project
-            List projectActivities = projectActivityService.getAllByProject(id)
-            props = includeProjectFundings(props)
-            props = includeProjectActivities(props, projectActivities)
+        synchronized (PROJECT_UPDATE_LOCKS.get(id)) {
+            Project project = Project.findByProjectId(id)
+            if (project) {
+                // retrieve any project activities associated with the project
+                List projectActivities = projectActivityService.getAllByProject(id)
+                props = includeProjectFundings(props)
+                props = includeProjectActivities(props, projectActivities)
 
-            try {
-                bindEmbeddedProperties(project, props)
-                commonService.updateProperties(project, props)
-                if (shouldUpdateCollectory) {
-                    updateCollectoryLinkForProject(project, props)
+                try {
+                    bindEmbeddedProperties(project, props)
+                    commonService.updateProperties(project, props)
+                    if (shouldUpdateCollectory) {
+                        updateCollectoryLinkForProject(project, props)
+                    }
+                    return [status: 'ok']
+                } catch (Exception e) {
+                    Project.withSession { session -> session.clear() }
+                    def error = "Error updating project ${id} - ${e.message}"
+                    log.error error, e
+                    return [status: 'error', error: error]
                 }
-                return [status: 'ok']
-            } catch (Exception e) {
-                Project.withSession { session -> session.clear() }
-                def error = "Error updating project ${id} - ${e.message}"
-                log.error error, e
+            } else {
+                def error = "Error updating project - no such id ${id}"
+                log.error error
                 return [status: 'error', error: error]
             }
-        } else {
-            def error = "Error updating project - no such id ${id}"
-            log.error error
-            return [status: 'error', error: error]
         }
     }
 
@@ -1054,4 +1063,38 @@ class ProjectService {
         records
     }
 
+    /**
+     * Updates a single data set associated with a project.  Because the datasets are stored as an embedded
+     * array in the Project collection, this method is synchronized on the project to avoid concurrent updates to
+     * different data sets overwriting each other.
+     * Due to the way it's been modelled as an embedded array, the client is allowed to supply a dataSetId
+     * when creating a new data set (e.g. a data set created by a submission from the Monitor app uses the
+     * submissionId as the dataSetId).
+     * @param projectId The project to update
+     * @param dataSet the data set to update.
+     * @return
+     */
+    Map updateDataSet(String projectId, Map dataSet) {
+        synchronized (PROJECT_UPDATE_LOCKS.get(projectId)) {
+            Project project = Project.findByProjectId(projectId)
+
+            if (!dataSet.dataSetId) {
+                dataSet.dataSetId = Identifiers.getNew(true, '')
+            }
+            Map matchingDataSet = project.custom?.dataSets?.find { it.dataSetId == dataSet.dataSetId }
+            if (matchingDataSet) {
+                matchingDataSet.putAll(dataSet)
+            } else {
+                if (!project.custom) {
+                    project.custom = [:]
+                }
+                if (!project.custom?.dataSets) {
+                    project.custom.dataSets = []
+                }
+                project.custom.dataSets.add(dataSet)
+            }
+            update([custom: project.custom], project.projectId, false)
+        }
+    }
+
 }

src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy

@@ -0,0 +1,44 @@
+package au.org.ala.ecodata
+
+import grails.testing.web.controllers.ControllerUnitTest
+import org.apache.http.HttpStatus
+import spock.lang.Specification
+
+class DataSetSummaryControllerSpec extends Specification implements ControllerUnitTest<DataSetSummaryController> {
+
+    ProjectService projectService = Mock(ProjectService)
+    def setup() {
+        controller.projectService = projectService
+    }
+
+    def cleanup() {
+    }
+
+    void "The update method delegates to the projectService"() {
+        setup:
+        String projectId = 'p1'
+        Map dataSetSummary = [dataSetId:'d1', name:'Data set 1']
+
+        when:
+        request.json = dataSetSummary
+        controller.update(projectId)
+
+        then:
+        1 * projectService.updateDataSet(projectId, dataSetSummary) >> [status:'ok']
+        response.json == ['status':'ok']
+
+    }
+
+    void "A project id must be specified either in the path or as part of the data set summary"() {
+        setup:
+        Map dataSetSummary = [dataSetId: 'd1', name: 'Data set 1']
+
+        when:
+        request.json = dataSetSummary
+        controller.update()
+
+        then:
+        0 * projectService.updateDataSet(_, _)
+        response.status == HttpStatus.SC_BAD_REQUEST
+    }
+}

src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy

@@ -138,19 +138,17 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest<ParatooSer
 
     }
 
-    void "The service can create a data set from a submitted collection"() {
+    void "The service should create a data set in the planned state when the mintCollectionId method is called"() {
         setup:
         ParatooCollectionId collectionId = buildCollectionId()
         String projectId = 'p1'
-        Map project = GormMongoUtil.extractDboProperties(getProject())
 
         when:
         Map result = service.mintCollectionId('u1', collectionId)
 
         then:
-        1 * projectService.get(projectId) >> project
-        1 * projectService.update(_, projectId, false) >> { data, pId, updateCollectory ->
-            Map dataSet = data.custom.dataSets[1]  // The stubbed project already has a dataSet, so the new one will be index=1
+        1 * projectService.updateDataSet(projectId, _) >> { pId, dataSet ->
+            pId == projectId
             assert dataSet.surveyId != null
             assert dataSet.surveyId.eventTime != null
             assert dataSet.surveyId.userId == 'org1'
@@ -194,9 +192,8 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest<ParatooSer
         then:
         1 * webService.doPost(*_) >> [resp: [collections: ["coarse-woody-debris-survey": [uuid: "1", createdAt: "2023-09-01T00:00:00.123Z", start_date_time: "2023-09-01T00:00:00.123Z", end_date_time: "2023-09-01T00:00:00.123Z"]]]]
         1 * tokenService.getAuthToken(true) >> Mock(AccessToken)
-        2 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]]
-        1 * projectService.update([custom: [dataSets: [expectedDataSetAsync]]], 'p1', false) >> [status: 'ok']
-        1 * projectService.update([custom: [dataSets: [expectedDataSetSync]]], 'p1', false) >> [status: 'ok']
+        1 * projectService.updateDataSet(projectId, expectedDataSetAsync) >> [status: 'ok']
+        1 * projectService.updateDataSet(projectId, expectedDataSetSync) >> [status: 'ok']
         1 * activityService.create({
             it.startDate == "2023-09-01T00:00:00Z" && it.endDate == "2023-09-01T00:00:00Z" &&
             it.plannedStartDate == "2023-09-01T00:00:00Z" && it.plannedEndDate == "2023-09-01T00:00:00Z" &&
@@ -310,8 +307,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest<ParatooSer
         then:
         1 * webService.doPost(*_) >> [resp: surveyData]
         1 * tokenService.getAuthToken(true) >> Mock(AccessToken)
-        2 * projectService.update(_, projectId, false) >> [status: 'ok']
-        2 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]]
+        2 * projectService.updateDataSet(projectId, _) >> [status: 'ok']
         1 * siteService.create(_) >> { site = it[0]; [siteId: 's1'] }
         1 * activityService.create({
             it.startDate == "2023-09-22T00:59:47Z" && it.endDate == "2023-09-23T00:59:47Z" &&

src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy

@@ -761,4 +761,49 @@ class ProjectServiceSpec extends MongoSpec implements ServiceUnitTest<ProjectSer
 
     }
 
+    void "The updateDataSet method will update (or insert) a data set into a Project"() {
+        setup:
+        Project project = new Project(projectId: '345', name: "Project 345", isMERIT: true, hubId:"12345")
+        project.save(flush: true, failOnError: true)
+        Map dataSet = [name: 'Test Data Set', description: 'Test Description', dataSetId:'d1']
+
+        when:
+        Map resp = service.updateDataSet(project.projectId, dataSet)
+
+        then:
+        resp.status == 'ok'
+        Project actual = Project.findByProjectId(project.projectId)
+        actual.projectId == project.projectId
+        actual.name == project.name
+        actual.isMERIT == project.isMERIT
+        actual.hubId == project.hubId
+        actual.custom.dataSets == [dataSet]
+
+        when:
+        Map dataSet2 = [name: 'Test Data Set 2', description: 'Test Description 2', dataSetId:'d2']
+        resp = service.updateDataSet(project.projectId, dataSet2)
+
+        then:
+        resp.status == 'ok'
+        Project actual2 = Project.findByProjectId(project.projectId)
+        actual2.projectId == project.projectId
+        actual2.name == project.name
+        actual2.isMERIT == project.isMERIT
+        actual2.hubId == project.hubId
+        actual2.custom.dataSets == [dataSet, dataSet2]
+
+        when:
+        dataSet.name = dataSet.name + " - Updated"
+        resp = service.updateDataSet(project.projectId, dataSet2)
+
+        then:
+        resp.status == 'ok'
+        Project actual3 = Project.findByProjectId(project.projectId)
+        actual3.projectId == project.projectId
+        actual3.name == project.name
+        actual3.isMERIT == project.isMERIT
+        actual3.hubId == project.hubId
+        actual3.custom.dataSets == [dataSet, dataSet2]
+    }
+
 }