From 6170591e6883c6ca30ce63311780491ef16286ea Mon Sep 17 00:00:00 2001 From: chrisala Date: Mon, 29 Apr 2024 11:11:58 +1000 Subject: [PATCH 01/11] 4.6-SNAPSHOT --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 93e530559..56e413bff 100644 --- a/build.gradle +++ b/build.gradle @@ -20,7 +20,7 @@ plugins { id "com.gorylenko.gradle-git-properties" version "2.4.1" } -version "4.5" +version "4.6-SNAPSHOT" group "au.org.ala" description "Ecodata" From 9a0ebdc22938865a82ad8c37f0f592406444a260 Mon Sep 17 00:00:00 2001 From: chrisala Date: Wed, 1 May 2024 10:01:40 +1000 Subject: [PATCH 02/11] Handle grouping null dates in DateGroup #928 --- .../groovy/au/org/ala/ecodata/reporting/ReportGroups.groovy | 5 +++++ .../au/org/ala/ecodata/reporting/ReportGroupsSpec.groovy | 2 ++ 2 files changed, 7 insertions(+) diff --git a/src/main/groovy/au/org/ala/ecodata/reporting/ReportGroups.groovy b/src/main/groovy/au/org/ala/ecodata/reporting/ReportGroups.groovy index bbeeb7be5..d8f3ba5fd 100644 --- a/src/main/groovy/au/org/ala/ecodata/reporting/ReportGroups.groovy +++ b/src/main/groovy/au/org/ala/ecodata/reporting/ReportGroups.groovy @@ -119,6 +119,7 @@ class ReportGroups { static class DateGroup extends SinglePropertyGroupingStrategy { + static final String MISSING_DATE_GROUP_NAME = "Date missing" static DateTimeFormatter parser = ISODateTimeFormat.dateTimeNoMillis().withZone(DateTimeZone.default) DateTimeFormatter dateFormatter List buckets @@ -157,6 +158,10 @@ class ReportGroups { def group(data) { def value = propertyAccessor.getPropertyValue(data) + if (!value) { + return MISSING_DATE_GROUP_NAME // Use a special group for null / empty dates. + } + int result = bucketIndex(value) // we put results with an exact date match into the group where the end date of the bucket matches diff --git a/src/test/groovy/au/org/ala/ecodata/reporting/ReportGroupsSpec.groovy b/src/test/groovy/au/org/ala/ecodata/reporting/ReportGroupsSpec.groovy index 81679f6fd..11408dbda 100644 --- a/src/test/groovy/au/org/ala/ecodata/reporting/ReportGroupsSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/reporting/ReportGroupsSpec.groovy @@ -105,6 +105,8 @@ public class ReportGroupsSpec extends Specification { ['2014-09-01T00:00:00Z', '2014-10-01T00:00:00Z', '2014-11-01T00:00:00Z'] | '2014-08-13T00:00:00Z' | 'MM-yyyy' | 'Before 09-2014' ['2014-09-01T00:00:00Z', '2014-10-01T00:00:00Z', '2014-11-01T00:00:00Z'] | '2014-11-01T00:00:01Z' | 'MM-yyyy' | 'After 10-2014' ['2014-10-01T00:00:00Z', '2015-01-01T00:00:00Z', '2015-04-01T00:00:00Z'] | '2014-10-13T00:00:00Z' | 'MM-yyyy' | '10-2014 - 12-2014' + ['2014-10-01T00:00:00Z', '2015-01-01T00:00:00Z', '2015-04-01T00:00:00Z'] | null | 'MM-yyyy' | 'Date missing' + ['2014-10-01T00:00:00Z', '2015-01-01T00:00:00Z', '2015-04-01T00:00:00Z'] | '' | 'MM-yyyy' | 'Date missing' } From 5899f31da01f9dbca95bb0eeb1a799a762399d10 Mon Sep 17 00:00:00 2001 From: temi Date: Wed, 1 May 2024 14:25:43 +1000 Subject: [PATCH 03/11] AtlasOfLivingAustralia/fieldcapture#3171 - track user in audit messages --- .../services/au/org/ala/ecodata/ParatooService.groovy | 6 ++++++ .../groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy | 3 +++ 2 files changed, 9 insertions(+) diff --git a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy index c0d745bf9..75942f019 100644 --- a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy @@ -228,11 +228,17 @@ class ParatooService { Map authHeader = getAuthHeader() Promise promise = task { + userService.setCurrentUser(userId) asyncFetchCollection(collection, authHeader, userId, project) } promise.onError { Throwable e -> log.error("An error occurred feching ${collection.orgMintedUUID}: ${e.message}", e) + userService.clearCurrentUser() } + promise.onComplete { Map result -> + userService.clearCurrentUser() + } + def result = projectService.update([custom: project.project.custom], project.id, false) [updateResult: result, promise: promise] } diff --git a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy index a57dc2dfe..3bb8f0642 100644 --- a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy @@ -210,6 +210,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [userId: userId] + 1 * userService.setCurrentUser(userId) and: result.updateResult == [status: 'ok'] @@ -297,6 +298,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [resp: surveyData] @@ -318,6 +320,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [userId: userId] + 1 * userService.setCurrentUser(userId) and: site.name == "SATFLB0001 - Control (100 x 100)" From a718362eec2bc833f64ddade98527c47934331f7 Mon Sep 17 00:00:00 2001 From: temi Date: Thu, 2 May 2024 11:39:42 +1000 Subject: [PATCH 04/11] #927 - changed how start and end dates are extracted for off-plot and on-plot protocols - added dates to activity - adds external id to activity --- build.gradle | 2 +- .../au/org/ala/ecodata/ParatooService.groovy | 21 +++--- .../paratoo/ParatooProtocolConfig.groovy | 65 +++---------------- .../org/ala/ecodata/ParatooServiceSpec.groovy | 12 +++- .../paratoo/ParatooProtocolConfigSpec.groovy | 20 ++---- .../paratoo/basalAreaDbhReverseLookup.json | 3 +- 6 files changed, 43 insertions(+), 80 deletions(-) diff --git a/build.gradle b/build.gradle index 56e413bff..21e18e5cc 100644 --- a/build.gradle +++ b/build.gradle @@ -20,7 +20,7 @@ plugins { id "com.gorylenko.gradle-git-properties" version "2.4.1" } -version "4.6-SNAPSHOT" +version "4.6-DATES-SNAPSHOT" group "au.org.ala" description "Ecodata" diff --git a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy index 7e2ee8013..86044d2dd 100644 --- a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy @@ -297,6 +297,11 @@ class ParatooService { surveyDataAndObservations[PARATOO_DATAMODEL_PLOT_LAYOUT] = dataSet.siteId } + dataSet.startDate = config.getStartDate(surveyDataAndObservations) + dataSet.endDate = config.getEndDate(surveyDataAndObservations) + dataSet.format = DATASET_DATABASE_TABLE + dataSet.sizeUnknown = true + // Delete previously created activity so that duplicate species records are not created. // Updating existing activity will also create duplicates since it relies on outputSpeciesId to determine // if a record is new and new ones are created by code. @@ -304,16 +309,11 @@ class ParatooService { activityService.delete(dataSet.activityId, true) } - String activityId = createActivityFromSurveyData(form, surveyDataAndObservations, surveyId, dataSet.siteId, userId) + String activityId = createActivityFromSurveyData(form, surveyDataAndObservations, surveyId, dataSet, userId) List records = recordService.getAllByActivity(activityId) dataSet.areSpeciesRecorded = records?.size() > 0 dataSet.activityId = activityId - dataSet.startDate = config.getStartDate(surveyDataAndObservations) - dataSet.endDate = config.getEndDate(surveyDataAndObservations) - dataSet.format = DATASET_DATABASE_TABLE - dataSet.sizeUnknown = true - synchronized (LOCK) { Map latestProject = projectService.get(project.project.projectId) Map latestDataSet = latestProject.custom?.dataSets?.find { it.dataSetId == collection.orgMintedUUID } @@ -456,14 +456,19 @@ class ParatooService { * @param siteId * @return */ - private String createActivityFromSurveyData(ActivityForm activityForm, Map surveyObservations, ParatooCollectionId collection, String siteId, String userId) { + private String createActivityFromSurveyData(ActivityForm activityForm, Map surveyObservations, ParatooCollectionId collection, Map dataSet, String userId) { Map activityProps = [ type : activityForm.name, formVersion : activityForm.formVersion, description : "Activity submitted by monitor", projectId : collection.projectId, publicationStatus: "published", - siteId : siteId, + siteId : dataSet.siteId, + startDate : dataSet.startDate, + endDate : dataSet.endDate, + plannedStartDate : dataSet.startDate, + plannedEndDate : dataSet.endDate, + externalIds : [new ExternalId(idType: ExternalId.IdType.MONITOR_MINTED_COLLECTION_ID, externalId: dataSet.dataSetId)], userId : userId, outputs : [[ data: surveyObservations, diff --git a/src/main/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfig.groovy b/src/main/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfig.groovy index e6b0be669..9ccd795c1 100644 --- a/src/main/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfig.groovy +++ b/src/main/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfig.groovy @@ -1,7 +1,6 @@ package au.org.ala.ecodata.paratoo import au.org.ala.ecodata.* -import au.org.ala.ecodata.converter.ISODateBindingConverter import au.org.ala.ecodata.metadata.OutputMetadata import au.org.ala.ecodata.metadata.PropertyAccessor import com.fasterxml.jackson.annotation.JsonIgnoreProperties @@ -25,9 +24,6 @@ class ParatooProtocolConfig { String endDatePath = 'end_date_time' String surveyIdPath = 'survey_metadata' String plotVisitPath = 'plot_visit' - String plotProtocolObservationDatePath = "date_time" - String plotVisitStartDatePath = "${plotVisitPath}.start_date" - String plotVisitEndDatePath = "${plotVisitPath}.end_date" String plotLayoutPath = "${plotVisitPath}.plot_layout" String plotLayoutIdPath = "${plotLayoutPath}.id" String plotLayoutPointsPath = "${plotLayoutPath}.plot_points" @@ -55,46 +51,13 @@ class ParatooProtocolConfig { return null } - def date - if (usesPlotLayout) { - List dates = getDatesFromObservation(surveyData) - date = dates ? DateUtil.format(dates.first()) : null - return date + def date = getProperty(surveyData, startDatePath) + if (!date) { + date = getPropertyFromSurvey(surveyData, startDatePath) } - else { - date = getProperty(surveyData, startDatePath) - if (!date) { - date = getPropertyFromSurvey(surveyData, startDatePath) - } - date = getFirst(date) - return removeMilliseconds(date) - } - } - - /** - * Get date from plotProtocolObservationDatePath and sort them. - * @param surveyData - reverse lookup output which includes survey and observation data - * @return - */ - List getDatesFromObservation(Map surveyData) { - Map surveysData = surveyData.findAll { key, value -> - ![ getSurveyAttributeName(), ParatooService.PARATOO_DATAMODEL_PLOT_SELECTION, - ParatooService.PARATOO_DATAMODEL_PLOT_VISIT, ParatooService.PARATOO_DATAMODEL_PLOT_LAYOUT].contains(key) - } - List result = [] - ISODateBindingConverter converter = new ISODateBindingConverter() - surveysData.each { key, value -> - def dates = getProperty(value, plotProtocolObservationDatePath) - dates = dates instanceof List ? dates : [dates] - - result.addAll(dates.collect { String date -> - date ? converter.convert(date, ISODateBindingConverter.FORMAT) : null - }) - } - - result = result.findAll { it != null } - result.sort() + date = getFirst(date) + return removeMilliseconds(date) } def getPropertyFromSurvey(Map surveyData, String path) { @@ -107,21 +70,13 @@ class ParatooProtocolConfig { return null } - def date - if (usesPlotLayout) { - def dates = getDatesFromObservation(surveyData) - date = dates ? DateUtil.format(dates.last()) : null - return date + def date = getProperty(surveyData, endDatePath) + if (!date) { + date = getPropertyFromSurvey(surveyData, endDatePath) } - else { - date = getProperty(surveyData, endDatePath) - if (!date) { - date = getPropertyFromSurvey(surveyData, endDatePath) - } - date = getFirst(date) - return removeMilliseconds(date) - } + date = getFirst(date) + return removeMilliseconds(date) } Map getSurveyId(Map surveyData) { diff --git a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy index f79449bdf..5cf4b473b 100644 --- a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy @@ -197,7 +197,11 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [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 * activityService.create(_) >> [activityId: '123'] + 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" && + it.externalIds[0].externalId == "d1" && it.externalIds[0].idType == ExternalId.IdType.MONITOR_MINTED_COLLECTION_ID + }) >> [activityId: '123'] 1 * activityService.delete("123", true) >> [status: 'ok'] 1 * recordService.getAllByActivity('123') >> [] 1 * settingService.getSetting('paratoo.surveyData.mapping') >> { @@ -307,7 +311,11 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [status: 'ok'] 2 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] 1 * siteService.create(_) >> { site = it[0]; [siteId: 's1'] } - 1 * activityService.create(_) >> [activityId: '123'] + 1 * activityService.create({ + it.startDate == "2023-09-22T00:59:47Z" && it.endDate == "2023-09-23T00:59:47Z" && + it.plannedStartDate == "2023-09-22T00:59:47Z" && it.plannedEndDate == "2023-09-23T00:59:47Z" && + it.externalIds[0].externalId == "d1" && it.externalIds[0].idType == ExternalId.IdType.MONITOR_MINTED_COLLECTION_ID + }) >> [activityId: '123'] 1 * recordService.getAllByActivity('123') >> [] 1 * settingService.getSetting('paratoo.surveyData.mapping') >> { (["guid-3": [ diff --git a/src/test/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfigSpec.groovy b/src/test/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfigSpec.groovy index dbfbf87fa..9b2560aac 100644 --- a/src/test/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfigSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/paratoo/ParatooProtocolConfigSpec.groovy @@ -100,9 +100,7 @@ class ParatooProtocolConfigSpec extends Specification { Map observation = apiOutput.collections Map floristicsSurveyConfig = [ apiEndpoint:'floristics-veg-survey-lites', - usesPlotLayout:true, - startDatePath: 'plot_visit.start_date', - endDatePath: 'plot_visit.end_date' + usesPlotLayout:true ] ParatooProtocolConfig config = new ParatooProtocolConfig(floristicsSurveyConfig) config.setSurveyId(ParatooCollectionId.fromMap([survey_metadata: apiOutput.survey_metadata])) @@ -134,8 +132,8 @@ class ParatooProtocolConfigSpec extends Specification { transformData(observation, activityForm, config) then: - config.getStartDate(observation) == "2024-04-08T01:23:28Z" - config.getEndDate(observation) == "2024-04-10T01:23:28Z" + config.getStartDate(observation) == "2022-09-21T01:55:44Z" + config.getEndDate(observation) == "2022-09-21T01:55:44Z" config.getGeoJson(observation) == [type: "Feature", geometry: [type: "Polygon", coordinates: [[[152.880694, -27.388252], [152.880651, -27.388336], [152.880518, -27.388483], [152.880389, -27.388611], [152.88028, -27.388749], [152.880154, -27.388903], [152.880835, -27.389463], [152.880644, -27.389366], [152.880525, -27.389248], [152.88035, -27.389158], [152.880195, -27.389021], [152.880195, -27.389373], [152.880797, -27.388316], [152.881448, -27.388909], [152.881503, -27.388821], [152.881422, -27.388766], [152.881263, -27.388644], [152.881107, -27.388549], [152.880939, -27.388445], [152.881314, -27.389035], [152.88122, -27.389208], [152.881089, -27.389343], [152.880973, -27.389472], [152.880916, -27.389553], [152.880694, -27.388252]]]], properties: [name: "QDASEQ0001 - Control (100 x 100)", externalId: 1, description: "QDASEQ0001 - Control (100 x 100)", notes: "some comment"]] } @@ -144,9 +142,7 @@ class ParatooProtocolConfigSpec extends Specification { Map surveyData = readSurveyData('basalAreaDbhReverseLookup') Map basalAreaDbhMeasureSurveyConfig = [ apiEndpoint:'basal-area-dbh-measure-surveys', - usesPlotLayout:true, - startDatePath: 'start_date', - endDatePath: 'start_date', + usesPlotLayout:true ] ParatooProtocolConfig config = new ParatooProtocolConfig(basalAreaDbhMeasureSurveyConfig) config.setSurveyId(ParatooCollectionId.fromMap([survey_metadata: surveyData.survey_metadata])) @@ -185,8 +181,8 @@ class ParatooProtocolConfigSpec extends Specification { transformData(observation, activityForm, config) expect: - config.getStartDate(observation) == "2024-03-28T03:17:01Z" - config.getEndDate(observation) == "2024-03-28T03:17:01Z" + config.getStartDate(observation) == "2023-09-22T00:59:47Z" + config.getEndDate(observation) == "2023-09-23T00:59:47Z" config.getGeoJson(observation) == [ type : "Feature", geometry : [ @@ -211,9 +207,7 @@ class ParatooProtocolConfigSpec extends Specification { Map opportunisticSurveyConfig = [ apiEndpoint : 'opportunistic-surveys', usesPlotLayout: false, - geometryType : 'Point', - startDatePath : 'start_date_time', - endDatePath : 'end_date_time' + geometryType : 'Point' ] ActivityForm activityForm = new ActivityForm( name: "aParatooForm 1", diff --git a/src/test/resources/paratoo/basalAreaDbhReverseLookup.json b/src/test/resources/paratoo/basalAreaDbhReverseLookup.json index 27145c2ab..15ab3ed80 100644 --- a/src/test/resources/paratoo/basalAreaDbhReverseLookup.json +++ b/src/test/resources/paratoo/basalAreaDbhReverseLookup.json @@ -41,7 +41,8 @@ "system_org": "MERIT" } }, - "start_date": "2023-09-22T00:59:47.807Z", + "start_date_time": "2023-09-22T00:59:47.807Z", + "end_date_time": "2023-09-23T00:59:47.807Z", "createdAt": "2023-09-22T01:05:19.981Z", "updatedAt": "2023-09-22T01:05:19.981Z", "plot_size": { From 3c2b83d23febe3d3677297fe5aa69ed1c1247213 Mon Sep 17 00:00:00 2001 From: chrisala Date: Thu, 2 May 2024 12:02:33 +1000 Subject: [PATCH 05/11] Back to 4.6-SNAPSHOT #927 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 21e18e5cc..56e413bff 100644 --- a/build.gradle +++ b/build.gradle @@ -20,7 +20,7 @@ plugins { id "com.gorylenko.gradle-git-properties" version "2.4.1" } -version "4.6-DATES-SNAPSHOT" +version "4.6-SNAPSHOT" group "au.org.ala" description "Ecodata" From 470974152ff86f893682ef72bad839c0296da75e Mon Sep 17 00:00:00 2001 From: chrisala Date: Thu, 2 May 2024 14:38:10 +1000 Subject: [PATCH 06/11] Allow updates of single data sets #934 --- .../ecodata/DataSetSummaryController.groovy | 30 +++++++ .../au/org/ala/ecodata/ParatooService.groovy | 32 ++----- .../au/org/ala/ecodata/ProjectService.groovy | 83 ++++++++++++++----- .../DataSetSummaryControllerSpec.groovy | 44 ++++++++++ .../org/ala/ecodata/ParatooServiceSpec.groovy | 16 ++-- .../org/ala/ecodata/ProjectServiceSpec.groovy | 45 ++++++++++ 6 files changed, 193 insertions(+), 57 deletions(-) create mode 100644 grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy create mode 100644 src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy diff --git a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy new file mode 100644 index 000000000..d6572b73f --- /dev/null +++ b/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) + } +} diff --git a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy index 267c868e4..0e6add911 100644 --- a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy +++ b/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) } } } diff --git a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy index b1ee890db..ec5467880 100644 --- a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy +++ b/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) + } + } + } \ No newline at end of file diff --git a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy new file mode 100644 index 000000000..f838f87dd --- /dev/null +++ b/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 { + + 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 + } +} diff --git a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy index d4ff42c29..180c1350f 100644 --- a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy @@ -138,19 +138,17 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> 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> [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> [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" && diff --git a/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy index f73b83a3e..dd9b6d1fa 100644 --- a/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy @@ -761,4 +761,49 @@ class ProjectServiceSpec extends MongoSpec implements ServiceUnitTest Date: Thu, 2 May 2024 15:04:59 +1000 Subject: [PATCH 07/11] Fixed controller/test #934 --- .../org/ala/ecodata/DataSetSummaryController.groovy | 11 ++++++----- .../ala/ecodata/DataSetSummaryControllerSpec.groovy | 2 ++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy index d6572b73f..666c8f3bb 100644 --- a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy +++ b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy @@ -13,15 +13,16 @@ class DataSetSummaryController { def update(String projectId) { Map dataSet = request.JSON - if (!projectId && !dataSet.projectId) { - respond status: 400, message: "projectId is required" + if (!projectId) { + projectId = dataSet.projectId + } + if (!projectId) { + render status: HttpStatus.SC_BAD_REQUEST, text: "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" + render status: HttpStatus.SC_BAD_REQUEST, text: "projectId must match the data set projectId" return } diff --git a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy index f838f87dd..eeee899a8 100644 --- a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy @@ -20,6 +20,7 @@ class DataSetSummaryControllerSpec extends Specification implements ControllerUn Map dataSetSummary = [dataSetId:'d1', name:'Data set 1'] when: + request.method = 'POST' request.json = dataSetSummary controller.update(projectId) @@ -34,6 +35,7 @@ class DataSetSummaryControllerSpec extends Specification implements ControllerUn Map dataSetSummary = [dataSetId: 'd1', name: 'Data set 1'] when: + request.method = 'POST' request.json = dataSetSummary controller.update() From f3a59ff4bbe25b303115d95d70d341311ce97e31 Mon Sep 17 00:00:00 2001 From: chrisala Date: Fri, 3 May 2024 09:59:40 +1000 Subject: [PATCH 08/11] Added a delete data set operation #934 --- .../ecodata/DataSetSummaryController.groovy | 14 ++++++++---- .../au/org/ala/ecodata/UrlMappings.groovy | 4 ++++ .../au/org/ala/ecodata/ProjectService.groovy | 18 ++++++++++++++- .../DataSetSummaryControllerSpec.groovy | 14 ++++++++++++ .../org/ala/ecodata/ProjectServiceSpec.groovy | 22 +++++++++++++++++++ 5 files changed, 67 insertions(+), 5 deletions(-) diff --git a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy index 666c8f3bb..7f5b629e2 100644 --- a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy +++ b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy @@ -5,17 +5,15 @@ import org.apache.http.HttpStatus class DataSetSummaryController { static responseFormats = ['json', 'xml'] - static allowedMethods = [update:['POST', 'PUT']] + static allowedMethods = [update:['POST', 'PUT'], delete:'DELETE'] ProjectService projectService /** Updates a single dataset for a project */ def update(String projectId) { Map dataSet = request.JSON + projectId = projectId ?: dataSet.projectId - if (!projectId) { - projectId = dataSet.projectId - } if (!projectId) { render status: HttpStatus.SC_BAD_REQUEST, text: "projectId is required" return @@ -28,4 +26,12 @@ class DataSetSummaryController { respond projectService.updateDataSet(projectId, dataSet) } + + def delete(String projectId, String dataSetId) { + if (!projectId || !dataSetId) { + render status: HttpStatus.SC_BAD_REQUEST, text: "projectId and dataSetId are required" + return + } + respond projectService.deleteDataSet(projectId, dataSetId) + } } diff --git a/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy b/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy index 7f85d0b43..4e2f62986 100644 --- a/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy +++ b/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy @@ -196,6 +196,10 @@ class UrlMappings { "/ws/project/getDefaultFacets"(controller: "project", action: "getDefaultFacets") "/ws/project/$projectId/dataSet/$dataSetId/records"(controller: "project", action: "fetchDataSetRecords") "/ws/admin/initiateSpeciesRematch"(controller: "admin", action: "initiateSpeciesRematch") + "/ws/dataSetSummary/$projectId/$dataSetId?"(controller :'dataSetSummary') { + + action = [POST:'update', PUT:'update', DELETE:'delete'] + } "/ws/document/download"(controller:"document", action:"download") diff --git a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy index ec5467880..41b8b09a0 100644 --- a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy @@ -1077,7 +1077,9 @@ class ProjectService { Map updateDataSet(String projectId, Map dataSet) { synchronized (PROJECT_UPDATE_LOCKS.get(projectId)) { Project project = Project.findByProjectId(projectId) - + if (!project) { + return [status: 'error', error: "No project exists with projectId=${projectId}"] + } if (!dataSet.dataSetId) { dataSet.dataSetId = Identifiers.getNew(true, '') } @@ -1097,4 +1099,18 @@ class ProjectService { } } + Map deleteDataSet(String projectId, String dataSetId) { + synchronized (PROJECT_UPDATE_LOCKS.get(projectId)) { + Project project = Project.findByProjectId(projectId) + + boolean foundMatchingDataSet = project?.custom?.dataSets?.removeAll { it.dataSetId == dataSetId } + if (!foundMatchingDataSet) { + return [status: 'error', error: 'No such data set'] + } + else { + update([custom: project.custom], project.projectId, false) + } + } + } + } \ No newline at end of file diff --git a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy index eeee899a8..46d7fc1e1 100644 --- a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy @@ -43,4 +43,18 @@ class DataSetSummaryControllerSpec extends Specification implements ControllerUn 0 * projectService.updateDataSet(_, _) response.status == HttpStatus.SC_BAD_REQUEST } + + void "The delete method delegates to the projectService"() { + setup: + String projectId = 'p1' + String dataSetSummaryId = 'd1' + + when: + request.method = 'DELETE' + controller.delete(projectId, dataSetSummaryId) + + then: + 1 * projectService.deleteDataSet(projectId, dataSetSummaryId) >> [status:'ok'] + response.json == ['status':'ok'] + } } diff --git a/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy index dd9b6d1fa..856ee7b16 100644 --- a/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy @@ -806,4 +806,26 @@ class ProjectServiceSpec extends MongoSpec implements ServiceUnitTest Date: Sat, 4 May 2024 11:22:38 +1000 Subject: [PATCH 09/11] Merge Project custom property instead of replace in update method #934 --- .../au/org/ala/ecodata/ProjectService.groovy | 5 +++++ .../org/ala/ecodata/ProjectServiceSpec.groovy | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy index 41b8b09a0..8e6840a47 100644 --- a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy @@ -485,6 +485,11 @@ class ProjectService { props = includeProjectActivities(props, projectActivities) try { + // Custom currently holds keys "details" and "dataSets". Only update the "custom" properties + // that are supplied in the update, leaving the others intact. + if (project.custom && props.custom) { + project.custom.putAll(props.remove('custom')) + } bindEmbeddedProperties(project, props) commonService.updateProperties(project, props) if (shouldUpdateCollectory) { diff --git a/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy index 856ee7b16..a5b01f8e7 100644 --- a/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ProjectServiceSpec.groovy @@ -828,4 +828,26 @@ class ProjectServiceSpec extends MongoSpec implements ServiceUnitTest Date: Mon, 6 May 2024 14:58:39 +1000 Subject: [PATCH 10/11] Added bulk update method #934 --- .../ecodata/DataSetSummaryController.groovy | 29 +++++++++++- .../au/org/ala/ecodata/UrlMappings.groovy | 2 + .../au/org/ala/ecodata/ProjectService.groovy | 45 ++++++++++++++----- .../DataSetSummaryControllerSpec.groovy | 15 +++++++ 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy index 7f5b629e2..899399210 100644 --- a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy +++ b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy @@ -5,7 +5,7 @@ import org.apache.http.HttpStatus class DataSetSummaryController { static responseFormats = ['json', 'xml'] - static allowedMethods = [update:['POST', 'PUT'], delete:'DELETE'] + static allowedMethods = [update:['POST', 'PUT'], delete:'DELETE', bulkUpdate: 'POST'] ProjectService projectService @@ -27,6 +27,33 @@ class DataSetSummaryController { respond projectService.updateDataSet(projectId, dataSet) } + /** + * Updates multiple data sets for a project. + * This endpoint exists to support the use case of associating multiple data sets with a + * report and updating their publicationStatus when the report is submitted/approved. + * + * This method expects the projectId to be supplied via the URL and the data sets to be supplied in the request + * body as a JSON object with key="dataSets" and value=List of data sets. + */ + def bulkUpdate(String projectId) { + Map postBody = request.JSON + List dataSets = postBody?.dataSets + + if (!projectId) { + render status: HttpStatus.SC_BAD_REQUEST, text: "projectId is required" + return + } + + dataSets.each { Map dataSet -> + if (dataSet.projectId && dataSet.projectId != projectId) { + render status: HttpStatus.SC_BAD_REQUEST, text: "projectId must match the data set projectId" + return + } + } + + respond projectService.updateDataSets(projectId, dataSets) + } + def delete(String projectId, String dataSetId) { if (!projectId || !dataSetId) { render status: HttpStatus.SC_BAD_REQUEST, text: "projectId and dataSetId are required" diff --git a/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy b/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy index 4e2f62986..c759f7a07 100644 --- a/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy +++ b/grails-app/controllers/au/org/ala/ecodata/UrlMappings.groovy @@ -201,6 +201,8 @@ class UrlMappings { action = [POST:'update', PUT:'update', DELETE:'delete'] } + "/ws/dataSetSummary/bulkUpdate/$projectId"(controller:'dataSetSummary', action:'bulkUpdate') + "/ws/document/download"(controller:"document", action:"download") "/ws/$controller/list"() { action = [GET:'list'] } diff --git a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy index 8e6840a47..8fab9f617 100644 --- a/grails-app/services/au/org/ala/ecodata/ProjectService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ProjectService.groovy @@ -1080,25 +1080,46 @@ class ProjectService { * @return */ Map updateDataSet(String projectId, Map dataSet) { + updateDataSets(projectId, [dataSet]) + } + + /** + * Updates multiple data sets associated with a project at the same time. This method exists to support + * the use case of associating multiple data sets with a report and updating their publicationStatus when + * the report is submitted/approved. + * + * 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 sets to update. + * @return + */ + Map updateDataSets(String projectId, List dataSets) { synchronized (PROJECT_UPDATE_LOCKS.get(projectId)) { Project project = Project.findByProjectId(projectId) if (!project) { return [status: 'error', error: "No project exists with projectId=${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 = [:] + for (Map dataSet in dataSets) { + if (!dataSet.dataSetId) { + dataSet.dataSetId = Identifiers.getNew(true, '') } - if (!project.custom?.dataSets) { - project.custom.dataSets = [] + 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) } - project.custom.dataSets.add(dataSet) } update([custom: project.custom], project.projectId, false) } diff --git a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy index 46d7fc1e1..9b9db1b28 100644 --- a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy @@ -57,4 +57,19 @@ class DataSetSummaryControllerSpec extends Specification implements ControllerUn 1 * projectService.deleteDataSet(projectId, dataSetSummaryId) >> [status:'ok'] response.json == ['status':'ok'] } + + void "The bulkUpdate method delegates to the projectService"() { + setup: + String projectId = 'p1' + Map postBody = [dataSets:[[dataSetId:'d1', name:'Data set 1']]] + + when: + request.method = 'POST' + request.json = postBody + controller.bulkUpdate(projectId) + + then: + 1 * projectService.updateDataSets(projectId, postBody.dataSets) >> [status:'ok'] + response.json == ['status':'ok'] + } } From a143e4ee526795b4278d47a3d3d0329f839ba2b9 Mon Sep 17 00:00:00 2001 From: chrisala Date: Tue, 7 May 2024 09:32:42 +1000 Subject: [PATCH 11/11] Addressed code review issue #934 --- .../ala/ecodata/DataSetSummaryController.groovy | 4 ++-- .../ecodata/DataSetSummaryControllerSpec.groovy | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy index 899399210..7a5d1a292 100644 --- a/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy +++ b/grails-app/controllers/au/org/ala/ecodata/DataSetSummaryController.groovy @@ -44,9 +44,9 @@ class DataSetSummaryController { return } - dataSets.each { Map dataSet -> + for (Map dataSet in dataSets) { if (dataSet.projectId && dataSet.projectId != projectId) { - render status: HttpStatus.SC_BAD_REQUEST, text: "projectId must match the data set projectId" + render status: HttpStatus.SC_BAD_REQUEST, text: "projectId must match the projectId in all supplied data sets" return } } diff --git a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy index 9b9db1b28..d7dc36f56 100644 --- a/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/DataSetSummaryControllerSpec.groovy @@ -72,4 +72,19 @@ class DataSetSummaryControllerSpec extends Specification implements ControllerUn 1 * projectService.updateDataSets(projectId, postBody.dataSets) >> [status:'ok'] response.json == ['status':'ok'] } + + void "If a projectId is present in a dataSet it much match the projectId parameter in bulkUpdate"() { + setup: + String projectId = 'p1' + Map postBody = [dataSets:[[dataSetId:'d1', name:'Data set 1', projectId:'p1'], [dataSetId:'d2', name:'Data set 2', projectId:'p2']]] + + when: + request.method = 'POST' + request.json = postBody + controller.bulkUpdate(projectId) + + then: + 0 * projectService.updateDataSets(_, _) + response.status == HttpStatus.SC_BAD_REQUEST + } }