From 72645d4368a2d971568e65f751130ec1ef86d2b3 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Sat, 11 May 2024 18:12:46 -0500 Subject: [PATCH 01/18] Add feature id filter (#1108) * add feature id table filter * update review table CSV to include feature id filter * fix test * fix test again * formatting * remove invert variable from filters as its never used and not needed * remove invert filter test and fix test output format --- .../controller/TaskReviewController.scala | 3 +++ .../framework/mixins/ReviewSearchMixin.scala | 1 + .../mixins/SearchParametersMixin.scala | 19 +++++++++++++++++++ .../dal/mixin/SearchParametersMixin.scala | 5 +++++ .../session/SearchParameters.scala | 7 +++++++ conf/v2_route/review.api | 2 +- .../mixins/SearchParametersMixinSpec.scala | 9 +++++++++ 7 files changed, 45 insertions(+), 1 deletion(-) diff --git a/app/org/maproulette/framework/controller/TaskReviewController.scala b/app/org/maproulette/framework/controller/TaskReviewController.scala index eac57530f..22bd58eb2 100644 --- a/app/org/maproulette/framework/controller/TaskReviewController.scala +++ b/app/org/maproulette/framework/controller/TaskReviewController.scala @@ -272,6 +272,7 @@ class TaskReviewController @Inject() ( */ def extractReviewTableData( taskId: String, + featureId: String, reviewStatus: String, mapper: String, challengeId: String, @@ -303,6 +304,7 @@ class TaskReviewController @Inject() ( val projectIdFilter = parseParameterLong(projectId) val challengeIdsFilter = parseParameterLong(challengeId) val taskIdFilter = parseParameterLong(taskId).map(_.head) + val taskFeatureIdFilter = parseParameterString(featureId).map(_.head) val mappedOnFilter = parseParameterString(mappedOn).map(_.head) val mapperFilter = parseParameterString(mapper).map(_.head) val metaReviewedByFilter = parseParameterString(metaReviewedBy).map(_.head) @@ -318,6 +320,7 @@ class TaskReviewController @Inject() ( ), taskParams = params.taskParams.copy( taskId = taskIdFilter, + taskFeatureId = taskFeatureIdFilter, taskStatus = statusFilter, taskReviewStatus = reviewStatusFilter, taskPriorities = priorityFilter, diff --git a/app/org/maproulette/framework/mixins/ReviewSearchMixin.scala b/app/org/maproulette/framework/mixins/ReviewSearchMixin.scala index 269dd410c..aea32156c 100644 --- a/app/org/maproulette/framework/mixins/ReviewSearchMixin.scala +++ b/app/org/maproulette/framework/mixins/ReviewSearchMixin.scala @@ -192,6 +192,7 @@ trait ReviewSearchMixin extends SearchParametersMixin { .addFilterGroup(this.filterLocation(searchParameters)) .addFilterGroup(this.filterProjectSearch(searchParameters)) .addFilterGroup(this.filterTaskId(searchParameters)) + .addFilterGroup(this.filterTaskFeatureId(searchParameters)) .addFilterGroup(this.filterPriority(searchParameters)) .addFilterGroup(this.filterTaskTags(searchParameters)) .addFilterGroup(this.filterReviewDate(searchParameters)) diff --git a/app/org/maproulette/framework/mixins/SearchParametersMixin.scala b/app/org/maproulette/framework/mixins/SearchParametersMixin.scala index 9dad049cd..a3de8b53d 100644 --- a/app/org/maproulette/framework/mixins/SearchParametersMixin.scala +++ b/app/org/maproulette/framework/mixins/SearchParametersMixin.scala @@ -25,6 +25,7 @@ trait SearchParametersMixin { this.filterBounding(params), this.filterTaskStatus(params), this.filterTaskId(params), + this.filterTaskFeatureId(params), this.filterProjectSearch(params), this.filterTaskReviewStatus(params), this.filterMetaReviewStatus(params), @@ -305,6 +306,24 @@ trait SearchParametersMixin { } } + /** + * Filters by tasks.name + * @param params with inverting on 'fid' + */ + def filterTaskFeatureId(params: SearchParameters): FilterGroup = { + params.taskParams.taskFeatureId match { + case Some(fid) => + FilterGroup( + List( + CustomParameter( + s"LOWER(TRIM(${Task.TABLE}.${Task.FIELD_NAME}::TEXT)) LIKE LOWER('%${fid.trim}%')" + ) + ) + ) + case None => FilterGroup(List()) + } + } + /** * Filters by tasks.priority * @param params with inverting on 'priorities' diff --git a/app/org/maproulette/models/dal/mixin/SearchParametersMixin.scala b/app/org/maproulette/models/dal/mixin/SearchParametersMixin.scala index 3a3a8ebb2..ff885bc75 100644 --- a/app/org/maproulette/models/dal/mixin/SearchParametersMixin.scala +++ b/app/org/maproulette/models/dal/mixin/SearchParametersMixin.scala @@ -33,6 +33,7 @@ trait SearchParametersMixin this.paramsBounding(params, whereClause) this.paramsTaskStatus(params, whereClause) this.paramsTaskId(params, whereClause) + this.paramsTaskFeatureId(params, whereClause) this.paramsProjectSearch(params, whereClause) this.paramsTaskReviewStatus(params, whereClause) this.paramsMetaReviewStatus(params, whereClause) @@ -105,6 +106,10 @@ trait SearchParametersMixin this.appendInWhereClause(whereClause, this.filterTaskId(params).sql()) } + def paramsTaskFeatureId(params: SearchParameters, whereClause: StringBuilder): Unit = { + this.appendInWhereClause(whereClause, this.filterTaskFeatureId(params).sql()) + } + def paramsTaskPriorities(params: SearchParameters, whereClause: StringBuilder): Unit = { this.appendInWhereClause(whereClause, this.filterTaskPriorities(params).sql()) } diff --git a/app/org/maproulette/session/SearchParameters.scala b/app/org/maproulette/session/SearchParameters.scala index bf5042a88..4d5070f93 100644 --- a/app/org/maproulette/session/SearchParameters.scala +++ b/app/org/maproulette/session/SearchParameters.scala @@ -49,6 +49,7 @@ case class SearchTaskParameters( taskSearch: Option[String] = None, taskStatus: Option[List[Int]] = None, taskId: Option[Long] = None, + taskFeatureId: Option[String] = None, taskReviewStatus: Option[List[Int]] = None, taskProperties: Option[Map[String, String]] = None, taskPropertySearchType: Option[String] = None, @@ -245,6 +246,10 @@ object SearchParameters { case Some(c) => Utils.insertIntoJson(updated, "taskId", c, true) case None => updated } + updated = o.taskParams.taskFeatureId match { + case Some(c) => Utils.insertIntoJson(updated, "taskFeatureId", c, true) + case None => updated + } updated = o.taskParams.taskReviewStatus match { case Some(c) => Utils.insertIntoJson(updated, "taskReviewStatus", c, true) case None => updated @@ -448,6 +453,8 @@ object SearchParameters { }, //taskIds this.getLongParameter(request.getQueryString("tid"), params.taskParams.taskId), + //taskFeatureId + this.getStringParameter(request.getQueryString("fid"), params.taskParams.taskFeatureId), //taskReviewStatus request.getQueryString("trStatus") match { case Some(v) => Utils.toIntList(v) diff --git a/conf/v2_route/review.api b/conf/v2_route/review.api index 03e931eb2..e5d61ff37 100644 --- a/conf/v2_route/review.api +++ b/conf/v2_route/review.api @@ -376,7 +376,7 @@ GET /tasks/review/mappers/export @org.maproulette.f # in: query # description: Reviews to equivalent onlySaved. ### -GET /tasks/review/reviewTable/export @org.maproulette.framework.controller.TaskReviewController.extractReviewTableData(taskId: String ?= "", reviewStatus: String ?= "0,1,2,3,4,5,6,7,-1", mapper: String ?= "", challengeId: String ?= "", projectId: String ?= "", mappedOn: String ?= "", reviewedBy: String ?= "", reviewedAt: String ?= "", metaReviewedBy: String ?= "", metaReviewStatus: String ?= "2,0,1,2,3,6", status: String ?= "0,1,2,3,4,5,6,9", priority: String ?= "0,1,2", tagFilter: String ?= "", sortBy: String ?= "mapped_on", direction: String ?= "ASC", displayedColumns: String ?= "Internal Id,Review Status,Mapper,Challenge,Project,Mapped On,Reviewer,Reviewed On,Status,Priority,Actions,Additional Reviewers", invertedFilters: String ?= "", onlySaved: Boolean ?= false) +GET /tasks/review/reviewTable/export @org.maproulette.framework.controller.TaskReviewController.extractReviewTableData(taskId: String ?= "", featureId: String ?= "", reviewStatus: String ?= "0,1,2,3,4,5,6,7,-1", mapper: String ?= "", challengeId: String ?= "", projectId: String ?= "", mappedOn: String ?= "", reviewedBy: String ?= "", reviewedAt: String ?= "", metaReviewedBy: String ?= "", metaReviewStatus: String ?= "2,0,1,2,3,6", status: String ?= "0,1,2,3,4,5,6,9", priority: String ?= "0,1,2", tagFilter: String ?= "", sortBy: String ?= "mapped_on", direction: String ?= "ASC", displayedColumns: String ?= "Internal Id,Review Status,Mapper,Challenge,Project,Mapped On,Reviewer,Reviewed On,Status,Priority,Actions,Additional Reviewers", invertedFilters: String ?= "", onlySaved: Boolean ?= false) ### # tags: [ Review ] # summary: Retrieve a summary of meta-review coverage for reviewers diff --git a/test/org/maproulette/framework/mixins/SearchParametersMixinSpec.scala b/test/org/maproulette/framework/mixins/SearchParametersMixinSpec.scala index 5204669c4..50db79b60 100644 --- a/test/org/maproulette/framework/mixins/SearchParametersMixinSpec.scala +++ b/test/org/maproulette/framework/mixins/SearchParametersMixinSpec.scala @@ -156,6 +156,15 @@ class SearchParametersMixinSpec() extends PlaySpec with SearchParametersMixin { } } + "filterTaskFeatureId" should { + "match on task feature id" in { + val params = SearchParameters(taskParams = SearchTaskParameters(taskFeatureId = Some("123"))) + this + .filterTaskFeatureId(params) + .sql() mustEqual s"LOWER(TRIM(tasks.name::TEXT)) LIKE LOWER('%123%')" + } + } + "filterTaskPriorities" should { "match on task priority" in { val params = From c61da9077cee7ba025a167bde06fc179e52079c1 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Sat, 11 May 2024 18:13:21 -0500 Subject: [PATCH 02/18] add support for relations in overpass queries (#1106) * add support for relations in overpass queries * account for nodes when building center point for relations * formatting * add compatibility for relations in relations in relations... * remove backslash * fix relations geojson structure * formatting * change element to member --- .../provider/ChallengeProvider.scala | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/app/org/maproulette/provider/ChallengeProvider.scala b/app/org/maproulette/provider/ChallengeProvider.scala index 5ee518f0a..b9bf5e0d0 100644 --- a/app/org/maproulette/provider/ChallengeProvider.scala +++ b/app/org/maproulette/provider/ChallengeProvider.scala @@ -485,6 +485,13 @@ class ChallengeProvider @Inject() ( "Element type 'way' does not match target type of '" + targetType + "'" ) } + case Some("relation") => + if (targetType != "relation") { + targetTypeFailed = true + throw new InvalidException( + "Element type 'relation' does not match target type of '" + targetType + "'" + ) + } case Some("node") => if (targetType != "node") { targetTypeFailed = true @@ -522,6 +529,60 @@ class ChallengeProvider @Inject() ( List((geom \ "lon").as[Double], (geom \ "lat").as[Double]) } Some(Json.obj("type" -> "LineString", "coordinates" -> points)) + case Some("relation") => + // Function to recursively extract geometries from relations + def extractGeometries(member: JsValue): Option[JsObject] = { + (member \ "type").asOpt[String] match { + case Some("way") => + val points = (member \ "geometry").as[List[JsValue]].map { + geom => + List((geom \ "lon").as[Double], (geom \ "lat").as[Double]) + } + Some(Json.obj("type" -> "LineString", "coordinates" -> points)) + + case Some("node") => + Some( + Json.obj( + "type" -> "Point", + "coordinates" -> List( + (member \ "lon").as[Double], + (member \ "lat").as[Double] + ) + ) + ) + + case Some("relation") => + // If it's another relation, recursively extract geometries from it + val geometries = (member \ "members").as[List[JsValue]].map { + member => + extractGeometries(member) + } + val geometryCollection = Json.obj( + "type" -> "GeometryCollection", + "geometries" -> geometries + ) + + Some(geometryCollection) + + case _ => + None + } + } + + // Extract geometries from each member of the relation + val geometries = (element \ "members").as[List[JsValue]].map { + member => + extractGeometries(member) + } + + // Create a GeometryCollection + val geometryCollection = Json.obj( + "type" -> "GeometryCollection", + "geometries" -> geometries + ) + + Some(geometryCollection) + case Some("node") => Some( Json.obj( From 70416fe077ea07d31f4fd0af107224ae84c690a2 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Sat, 11 May 2024 20:42:02 -0500 Subject: [PATCH 03/18] fix nearby task looping too hard tasks issue (#1110) * fix nearby task looping too hard tasks issue * make the hour interval base on the modified column instead of the created column. * set skipped tasks and too hard tasks set by the user as the last tasks to be selected * simplify conditions --- app/org/maproulette/models/dal/TaskDAL.scala | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/app/org/maproulette/models/dal/TaskDAL.scala b/app/org/maproulette/models/dal/TaskDAL.scala index 9c74fa215..84cb7d2d6 100644 --- a/app/org/maproulette/models/dal/TaskDAL.scala +++ b/app/org/maproulette/models/dal/TaskDAL.scala @@ -1063,7 +1063,17 @@ class TaskDAL @Inject() ( case Failure(f) => List.empty } if (mediumPriorityTasks.isEmpty) { - this.getRandomTasks(user, params, limit, Some(Challenge.PRIORITY_LOW), proximityId) + val lowPriorityTasks = Try( + this.getRandomTasks(user, params, limit, Some(Challenge.PRIORITY_LOW), proximityId) + ) match { + case Success(res) => res + case Failure(f) => List.empty + } + if (lowPriorityTasks.isEmpty) { + this.getRandomTasks(user, params, limit, None, proximityId) + } else { + lowPriorityTasks + } } else { mediumPriorityTasks } @@ -1148,8 +1158,12 @@ class TaskDAL @Inject() ( ) priority match { - case Some(p) => appendInWhereClause(whereClause, s"tasks.priority = $p") - case None => //Ignore + case Some(p) => + appendInWhereClause( + whereClause, + s"tasks.priority = $p AND (tasks.completed_by != ${user.id} OR tasks.completed_by IS NULL)" + ) + case None => // Ignore } if (taskTagIds.nonEmpty) { From 2e643dfca408eb8dc3e93399bf630a93597f49af Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Sat, 11 May 2024 21:21:51 -0500 Subject: [PATCH 04/18] add resetTaskBundle endpoint (#1107) * add resetTaskBundle endpoint * add bundleTasks endpoint and fix bugs with bundle resets and simplify bundle creation endpoint * fix task bundle reset to setup up tasks added back with the proper review_status needed for reviewing * formatting, and fix test to allow task in bundle to be unlocked * fix some variables in the task bundling related tests * revert task locking on fetch * add needed value in tasks that were reset and added back into the task_review table * change the fetch bundle endpoint from a GET to a POST * split bundle tasks functionality into its own function --- .../controller/TaskBundleController.scala | 44 ++- .../mixins/SearchParametersMixin.scala | 17 ++ .../repository/TaskBundleRepository.scala | 277 ++++++++++++++---- .../framework/service/TaskBundleService.scala | 57 +++- .../session/SearchParameters.scala | 7 + conf/v2_route/bundle.api | 34 ++- .../service/TaskBundleServiceSpec.scala | 62 +++- 7 files changed, 394 insertions(+), 104 deletions(-) diff --git a/app/org/maproulette/framework/controller/TaskBundleController.scala b/app/org/maproulette/framework/controller/TaskBundleController.scala index 579d15826..522210102 100644 --- a/app/org/maproulette/framework/controller/TaskBundleController.scala +++ b/app/org/maproulette/framework/controller/TaskBundleController.scala @@ -237,12 +237,13 @@ class TaskBundleController @Inject() ( */ def createTaskBundle(): Action[JsValue] = Action.async(bodyParsers.json) { implicit request => this.sessionManager.authenticatedRequest { implicit user => - val name = (request.body \ "name").asOpt[String].getOrElse("") + val name = (request.body \ "name").asOpt[String].getOrElse("") + val primaryId = (request.body \ "primaryId").asOpt[Long] val taskIds = (request.body \ "taskIds").asOpt[List[Long]] match { case Some(tasks) => tasks case None => throw new InvalidException("No task ids provided for task bundle") } - val bundle = this.serviceManager.taskBundle.createTaskBundle(user, name, taskIds) + val bundle = this.serviceManager.taskBundle.createTaskBundle(user, name, primaryId, taskIds) Created(Json.toJson(bundle)) } } @@ -253,8 +254,25 @@ class TaskBundleController @Inject() ( * @param id The id for the bundle * @return Task Bundle */ - def getTaskBundle(id: Long): Action[AnyContent] = Action.async { implicit request => + def getTaskBundle(id: Long, lockTasks: Boolean): Action[AnyContent] = Action.async { + implicit request => + this.sessionManager.authenticatedRequest { implicit user => + Ok(Json.toJson(this.serviceManager.taskBundle.getTaskBundle(user, id, lockTasks))) + } + } + + /** + * Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle + * + * @param bundleId The id of the bundle + * @param taskIds The task ids the bundle will reset to + */ + def resetTaskBundle( + id: Long, + taskIds: List[Long] + ): Action[AnyContent] = Action.async { implicit request => this.sessionManager.authenticatedRequest { implicit user => + this.serviceManager.taskBundle.resetTaskBundle(user, id, taskIds) Ok(Json.toJson(this.serviceManager.taskBundle.getTaskBundle(user, id))) } } @@ -266,24 +284,26 @@ class TaskBundleController @Inject() ( * @param taskIds List of task ids to remove * @return Task Bundle */ - def unbundleTasks(id: Long, taskIds: List[Long]): Action[AnyContent] = Action.async { - implicit request => - this.sessionManager.authenticatedRequest { implicit user => - this.serviceManager.taskBundle.unbundleTasks(user, id, taskIds) - Ok(Json.toJson(this.serviceManager.taskBundle.getTaskBundle(user, id))) - } + def unbundleTasks( + id: Long, + taskIds: List[Long], + preventTaskIdUnlocks: List[Long] + ): Action[AnyContent] = Action.async { implicit request => + this.sessionManager.authenticatedRequest { implicit user => + this.serviceManager.taskBundle.unbundleTasks(user, id, taskIds, preventTaskIdUnlocks) + Ok(Json.toJson(this.serviceManager.taskBundle.getTaskBundle(user, id))) + } } /** * Delete bundle. * * @param id The id for the bundle - * @param primaryId optional task id to no unlock after deleting this bundle */ - def deleteTaskBundle(id: Long, primaryId: Option[Long] = None): Action[AnyContent] = + def deleteTaskBundle(id: Long): Action[AnyContent] = Action.async { implicit request => this.sessionManager.authenticatedRequest { implicit user => - this.serviceManager.taskBundle.deleteTaskBundle(user, id, primaryId) + this.serviceManager.taskBundle.deleteTaskBundle(user, id) Ok } } diff --git a/app/org/maproulette/framework/mixins/SearchParametersMixin.scala b/app/org/maproulette/framework/mixins/SearchParametersMixin.scala index a3de8b53d..4e5891431 100644 --- a/app/org/maproulette/framework/mixins/SearchParametersMixin.scala +++ b/app/org/maproulette/framework/mixins/SearchParametersMixin.scala @@ -40,6 +40,7 @@ trait SearchParametersMixin { this.filterChallengeStatus(params), this.filterChallengeRequiresLocal(params), this.filterBoundingGeometries(params), + this.filterBundleId(params), // For efficiency can only query on task properties with a parent challenge id this.filterTaskProps(params), this.filterChallenges(params), @@ -324,6 +325,22 @@ trait SearchParametersMixin { } } + /** + * Filters by bundle id + * @param params with inverting on 'bid' + */ + def filterBundleId(params: SearchParameters): FilterGroup = { + params.taskParams.bundleId match { + case Some(bid) => + FilterGroup( + List( + CustomParameter(s"${Task.TABLE}.${Task.FIELD_BUNDLE_ID} = $bid") + ) + ) + case _ => FilterGroup(List()) + } + } + /** * Filters by tasks.priority * @param params with inverting on 'priorities' diff --git a/app/org/maproulette/framework/repository/TaskBundleRepository.scala b/app/org/maproulette/framework/repository/TaskBundleRepository.scala index 8f457fcfc..3789546d0 100644 --- a/app/org/maproulette/framework/repository/TaskBundleRepository.scala +++ b/app/org/maproulette/framework/repository/TaskBundleRepository.scala @@ -8,6 +8,7 @@ package org.maproulette.framework.repository import org.slf4j.LoggerFactory import anorm.ToParameterValue +import anorm.SqlParser.scalar import anorm._, postgresql._ import javax.inject.{Inject, Singleton} import org.maproulette.exception.InvalidException @@ -16,6 +17,7 @@ import org.maproulette.framework.psql.Query import org.maproulette.framework.psql.filter.BaseParameter import org.maproulette.framework.model.{Task, TaskBundle, User} import org.maproulette.framework.mixins.{TaskParserMixin, Locking} +import org.maproulette.framework.model.Task.STATUS_CREATED import org.maproulette.data.TaskType import play.api.db.Database @@ -33,7 +35,6 @@ class TaskBundleRepository @Inject() ( with Locking[Task] { protected val logger = LoggerFactory.getLogger(this.getClass) implicit val baseTable: String = Task.TABLE - val cacheManager = this.taskRepository.cacheManager /** * Inserts a new task bundle with the given tasks, assigning ownership of @@ -46,6 +47,7 @@ class TaskBundleRepository @Inject() ( def insert( user: User, name: String, + primaryId: Option[Long], taskIds: List[Long], verifyTasks: (List[Task]) => Unit ): TaskBundle = { @@ -55,6 +57,7 @@ class TaskBundleRepository @Inject() ( } val failedTaskIds = taskIds.diff(lockedTasks.map(_.id)) + // Checks to see if there where any tasks that were locked when the user tried to bundle them. if (failedTaskIds.nonEmpty) { throw new InvalidException( s"Bundle creation failed because the following task IDs were locked: ${failedTaskIds.mkString(", ")}" @@ -63,24 +66,40 @@ class TaskBundleRepository @Inject() ( verifyTasks(lockedTasks) - for (task <- lockedTasks) { - try { - this.lockItem(user, task) - } catch { - case e: Exception => this.logger.warn(e.getMessage) - } - } - val rowId = SQL"""INSERT INTO bundles (owner_id, name) VALUES (${user.id}, ${name})""".executeInsert() + rowId match { - case Some(bundleId) => - val sqlQuery = + case Some(bundleId: Long) => + // Update the task object to bind it to the bundle + SQL(s"""UPDATE tasks SET bundle_id = $bundleId + WHERE id IN ({inList})""") + .on( + "inList" -> taskIds + ) + .executeUpdate() + + primaryId match { + case Some(id) => + val sqlQuery = s"""UPDATE tasks SET is_bundle_primary = true WHERE id = $id""" + SQL(sqlQuery).executeUpdate() + case None => // Handle the case where primaryId is None + } + + val sqlInsertTaskBundles = s"""INSERT INTO task_bundles (task_id, bundle_id) VALUES ({taskId}, $bundleId)""" - val parameters = lockedTasks.map(task => { - Seq[NamedParameter]("taskId" -> task.id) - }) - BatchSql(sqlQuery, parameters.head, parameters.tail: _*).execute() + val parameters = lockedTasks.map(task => Seq[NamedParameter]("taskId" -> task.id)) + BatchSql(sqlInsertTaskBundles, parameters.head, parameters.tail: _*).execute() + + // Lock each of the new tasks to indicate they are part of the bundle + for (task <- lockedTasks) { + try { + this.lockItem(user, task) + } catch { + case e: Exception => this.logger.warn(e.getMessage) + } + } + TaskBundle(bundleId, user.id, lockedTasks.map(task => { task.id }), Some(lockedTasks)) @@ -91,12 +110,122 @@ class TaskBundleRepository @Inject() ( } } + /** + * Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle + * + * @param bundleId The id of the bundle + * @param taskIds The task ids the bundle will reset to + */ + def resetTaskBundle( + user: User, + bundleId: Long, + taskIds: List[Long] + ): Unit = { + withMRTransaction { implicit c => + // Retrieve all the task ids currently in the bundle + val currentTaskIds = this + .retrieveTasks( + Query.simple(List(BaseParameter("bundle_id", bundleId, table = Some("tb")))) + ) + .map(_.id) + + // Remove previous tasks from the bundle join table and unlock them if necessary + val tasksToRemove = currentTaskIds.filter(taskId => !taskIds.contains(taskId)) + + if (tasksToRemove.nonEmpty) { + this.unbundleTasks(user, bundleId, tasksToRemove, List.empty) + } + + // Filter for tasks that need to be added back to the bundle. + val tasksToAdd = taskIds.filterNot(currentTaskIds.contains) + + if (tasksToAdd.nonEmpty) { + this.bundleTasks(user, bundleId, tasksToAdd) + } + } + } + + /** + * Adds tasks to a bundle. + * + * @param bundleId The id of the bundle + */ + def bundleTasks( + user: User, + bundleId: Long, + taskIds: List[Long] + ): Unit = { + this.withMRConnection { implicit c => + val sqlQuery = + s"""INSERT INTO task_bundles (bundle_id, task_id) VALUES ($bundleId, {taskId})""" + val parameters = taskIds.map(taskId => Seq[NamedParameter]("taskId" -> taskId)) + + BatchSql(sqlQuery, parameters.head, parameters.tail: _*).execute() + val primaryTaskId = SQL( + """SELECT id FROM tasks WHERE bundle_id = {bundleId} AND is_bundle_primary = true""" + ).on("bundleId" -> bundleId) + .as(scalar[Int].singleOpt) + .getOrElse(0) + + val primaryTaskStatus: Int = SQL("""SELECT status FROM tasks WHERE id = {primaryTaskId}""") + .on("primaryTaskId" -> primaryTaskId) + .as(scalar[Int].singleOpt) + .getOrElse(0) + + SQL( + s"""UPDATE tasks SET bundle_id = {bundleId}, status = $primaryTaskStatus + WHERE bundle_id IS NULL AND id IN ({inList})""" + ).on( + "bundleId" -> bundleId, + "inList" -> taskIds + ) + .executeUpdate() + + val taskReviewQuery = SQL( + """SELECT review_status, review_requested_by FROM task_review WHERE task_id = {primaryTaskId}""" + ).on("primaryTaskId" -> primaryTaskId) + .as((scalar[Int] ~ scalar[Int]).singleOpt) + + taskReviewQuery match { + case Some((primaryTaskReviewStatus ~ primaryTaskReviewRequestedBy)) => + SQL( + """INSERT INTO task_review (task_id, review_status, review_requested_by) + SELECT id, {reviewStatus}, {reviewRequestedBy} + FROM tasks WHERE id IN ({inList})""" + ).on( + "reviewStatus" -> primaryTaskReviewStatus, + "reviewRequestedBy" -> primaryTaskReviewRequestedBy, + "inList" -> taskIds + ) + .executeUpdate() + case None => + } + + val lockedTasks = this.withListLocking(user, Some(TaskType())) { () => + this.taskDAL.retrieveListById(-1, 0)(taskIds) + } + lockedTasks.foreach { task => + try { + this.lockItem(user, task) + } catch { + case e: Exception => + this.logger.warn(e.getMessage) + } + } + } + } + /** * Removes tasks from a bundle. * * @param bundleId The id of the bundle */ - def unbundleTasks(user: User, bundleId: Long, taskIds: List[Long])(): Unit = { + def unbundleTasks( + user: User, + bundleId: Long, + taskIds: List[Long], + preventTaskIdUnlocks: List[Long] + ): Unit = { this.withMRConnection { implicit c => // Unset any bundle_id on individual tasks (this is set when task is completed) SQL(s"""UPDATE tasks SET bundle_id = NULL @@ -123,13 +252,28 @@ class TaskBundleRepository @Inject() ( taskIds.find(id => id == task.id) match { case Some(_) => SQL(s"""DELETE FROM task_bundles - WHERE bundle_id = ${bundleId} AND task_id = ${task.id}""").executeUpdate() - - try { - this.unlockItem(user, task) - } catch { - case e: Exception => this.logger.warn(e.getMessage) + WHERE bundle_id = $bundleId AND task_id = ${task.id}""").executeUpdate() + if (!preventTaskIdUnlocks.contains(task.id)) { + try { + this.unlockItem(user, task) + } catch { + case e: Exception => this.logger.warn(e.getMessage) + } } + // This is in order to pass the filters so the task is displayed as "available" in task searching and maps. + SQL(s"DELETE FROM task_review tr WHERE tr.task_id = ${task.id}").executeUpdate() + + SQL( + """UPDATE tasks + SET status = {status} + WHERE id = {taskId} + """ + ).on( + "taskId" -> task.id, + "status" -> STATUS_CREATED + ) + .executeUpdate() + case None => // do nothing } } @@ -142,57 +286,43 @@ class TaskBundleRepository @Inject() ( * * @param bundleId The id of the bundle */ - def deleteTaskBundle(user: User, bundle: TaskBundle, primaryTaskId: Option[Long] = None): Unit = { + def deleteTaskBundle(user: User, bundleId: Long): Unit = { this.withMRConnection { implicit c => + // Update tasks to set bundle_id and is_bundle_primary to NULL SQL( """UPDATE tasks - SET bundle_id = NULL, - is_bundle_primary = NULL - WHERE bundle_id = {bundleId} OR id = {primaryTaskId}""" - ).on( - Symbol("bundleId") -> bundle.bundleId, - Symbol("primaryTaskId") -> primaryTaskId - ) + SET bundle_id = NULL, + is_bundle_primary = NULL + WHERE bundle_id = {bundleId}""" + ).on("bundleId" -> bundleId) .executeUpdate() - if (primaryTaskId != None) { - // unlock tasks (everything but the primary task id) - val tasks = bundle.tasks match { - case Some(t) => - for (task <- t) { - if (task.id != primaryTaskId.getOrElse(0)) { - try { - this.unlockItem(user, task) - } catch { - case e: Exception => this.logger.warn(e.getMessage) - } - } - } - case None => // no tasks in bundle - } - } - - // Update cache for each task in the bundle - bundle.tasks match { - case Some(t) => - for (task <- t) { - this.cacheManager.withOptionCaching { () => - Some( - task.copy( - bundleId = None, - isBundlePrimary = None - ) - ) - } - } - - case None => // no tasks in bundle - } - // Delete from task_bundles which will also cascade delete from bundles SQL("DELETE FROM task_bundles WHERE bundle_id = {bundleId}") - .on(Symbol("bundleId") -> bundle.bundleId) + .on("bundleId" -> bundleId) .executeUpdate() + + // Get the primary task ID + val primaryTaskId = SQL( + """SELECT id FROM tasks WHERE bundle_id = {bundleId} AND is_bundle_primary = true""" + ).on("bundleId" -> bundleId) + .as(scalar[Option[Long]].singleOpt) + .getOrElse(0) + + // Retrieve tasks + val tasks = this.retrieveTasks( + Query.simple(List(BaseParameter("bundle_id", bundleId, table = Some("tb")))) + ) + + tasks.foreach { task => + if (task.id != primaryTaskId) { + try { + this.unlockItem(user, task) + } catch { + case e: Exception => this.logger.warn(e.getMessage) + } + } + } } } @@ -225,4 +355,21 @@ class TaskBundleRepository @Inject() ( """).as(this.getTaskParser(this.taskRepository.updateAndRetrieve).*) } } + + /** + * Locks tasks on bundle fetch if task is in an editable status + * + * @param bundleId The id of the bundle + */ + def lockBundledTasks(user: User, tasks: List[Task]) = { + this.withMRConnection { implicit c => + for (task <- tasks) { + try { + this.lockItem(user, task) + } catch { + case e: Exception => this.logger.warn(e.getMessage) + } + } + } + } } diff --git a/app/org/maproulette/framework/service/TaskBundleService.scala b/app/org/maproulette/framework/service/TaskBundleService.scala index 7bc2b84a0..318a6c546 100644 --- a/app/org/maproulette/framework/service/TaskBundleService.scala +++ b/app/org/maproulette/framework/service/TaskBundleService.scala @@ -39,11 +39,17 @@ class TaskBundleService @Inject() ( * @param name The name of the task bundle * @param taskIds The tasks to be added to the bundle */ - def createTaskBundle(user: User, name: String, taskIds: List[Long]): TaskBundle = { + def createTaskBundle( + user: User, + name: String, + bundlePrimary: Option[Long], + taskIds: List[Long] + ): TaskBundle = { this.repository.insert( user, name, + bundlePrimary, taskIds, (tasks: List[Task]) => { if (tasks.length < 1) { @@ -83,12 +89,40 @@ class TaskBundleService @Inject() ( ) } + /** + * Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle + * + * @param bundleId The id of the bundle + * @param taskIds The task ids the bundle will reset to + */ + def resetTaskBundle( + user: User, + bundleId: Long, + taskIds: List[Long] + ): TaskBundle = { + val bundle = this.getTaskBundle(user, bundleId) + + if (!permission.isSuperUser(user) && bundle.ownerId != user.id) { + throw new IllegalAccessException( + "Only a super user or the original user can reset this bundle." + ) + } + + this.repository.resetTaskBundle(user, bundleId, taskIds) + this.getTaskBundle(user, bundleId) + } + /** * Removes tasks from a bundle. * * @param bundleId The id of the bundle */ - def unbundleTasks(user: User, bundleId: Long, taskIds: List[Long])(): TaskBundle = { + def unbundleTasks( + user: User, + bundleId: Long, + taskIds: List[Long], + preventTaskIdUnlocks: List[Long] + )(): TaskBundle = { val bundle = this.getTaskBundle(user, bundleId) // Verify permissions to modify this bundle @@ -98,7 +132,7 @@ class TaskBundleService @Inject() ( ) } - this.repository.unbundleTasks(user, bundleId, taskIds) + this.repository.unbundleTasks(user, bundleId, taskIds, preventTaskIdUnlocks) this.getTaskBundle(user, bundleId) } @@ -107,7 +141,7 @@ class TaskBundleService @Inject() ( * * @param bundleId The id of the bundle */ - def deleteTaskBundle(user: User, bundleId: Long, primaryTaskId: Option[Long] = None): Unit = { + def deleteTaskBundle(user: User, bundleId: Long): Unit = { val bundle = this.getTaskBundle(user, bundleId) // Verify permissions to delete this bundle @@ -117,7 +151,7 @@ class TaskBundleService @Inject() ( this.permission.hasObjectWriteAccess(challenge.get, user) } - this.repository.deleteTaskBundle(user, bundle, primaryTaskId) + this.repository.deleteTaskBundle(user, bundle.bundleId) } /** @@ -125,7 +159,7 @@ class TaskBundleService @Inject() ( * * @param bundleId The id of the bundle */ - def getTaskBundle(user: User, bundleId: Long): TaskBundle = { + def getTaskBundle(user: User, bundleId: Long, lockTasks: Boolean = false): TaskBundle = { val filterQuery = Query.simple( List( @@ -136,12 +170,13 @@ class TaskBundleService @Inject() ( val ownerId = this.repository.retrieveOwner(filterQuery) val tasks = this.repository.retrieveTasks(filterQuery) - if (ownerId == None) { - throw new NotFoundException(s"Task Bundle not found with id ${bundleId}.") + if (ownerId.isEmpty) { + throw new NotFoundException(s"Task Bundle not found with id $bundleId.") + } + if (lockTasks) { + this.repository.lockBundledTasks(user, tasks) } - TaskBundle(bundleId, ownerId.get, tasks.map(task => { - task.id - }), Some(tasks)) + TaskBundle(bundleId, ownerId.get, tasks.map(_.id), Some(tasks)) } } diff --git a/app/org/maproulette/session/SearchParameters.scala b/app/org/maproulette/session/SearchParameters.scala index 4d5070f93..10ef98e30 100644 --- a/app/org/maproulette/session/SearchParameters.scala +++ b/app/org/maproulette/session/SearchParameters.scala @@ -50,6 +50,7 @@ case class SearchTaskParameters( taskStatus: Option[List[Int]] = None, taskId: Option[Long] = None, taskFeatureId: Option[String] = None, + bundleId: Option[Long] = None, taskReviewStatus: Option[List[Int]] = None, taskProperties: Option[Map[String, String]] = None, taskPropertySearchType: Option[String] = None, @@ -250,6 +251,10 @@ object SearchParameters { case Some(c) => Utils.insertIntoJson(updated, "taskFeatureId", c, true) case None => updated } + updated = o.taskParams.bundleId match { + case Some(c) => Utils.insertIntoJson(updated, "bundleId", c, true) + case None => updated + } updated = o.taskParams.taskReviewStatus match { case Some(c) => Utils.insertIntoJson(updated, "taskReviewStatus", c, true) case None => updated @@ -455,6 +460,8 @@ object SearchParameters { this.getLongParameter(request.getQueryString("tid"), params.taskParams.taskId), //taskFeatureId this.getStringParameter(request.getQueryString("fid"), params.taskParams.taskFeatureId), + //bundleId + this.getLongParameter(request.getQueryString("bid"), params.taskParams.bundleId), //taskReviewStatus request.getQueryString("trStatus") match { case Some(v) => Utils.toIntList(v) diff --git a/conf/v2_route/bundle.api b/conf/v2_route/bundle.api index 389f8fb82..567afd2d4 100644 --- a/conf/v2_route/bundle.api +++ b/conf/v2_route/bundle.api @@ -40,8 +40,30 @@ POST /taskBundle @org.maproulette.framework.c # in: path # description: The id of the Task Bundle # required: true +# - name: lockTasks +# in: query +# description: The tasks in the bundle will be locked by the user. ### -GET /taskBundle/:id @org.maproulette.framework.controller.TaskBundleController.getTaskBundle(id:Long) +POST /taskBundle/:id @org.maproulette.framework.controller.TaskBundleController.getTaskBundle(id:Long, lockTasks:Boolean ?= false) +### +# tags: [ Bundle ] +# summary: Resets a Task Bundle +# description: Resets the bundle to the tasks provided, and unlock all tasks removed from current bundle +# responses: +# '200': +# description: Ok with empty body +# '401': +# description: The user is not authorized to make this request +# parameters: +# - name: id +# in: path +# description: The id of the Task Bundle +# required: true +# - name: taskIds +# in: query +# description: The task ids the bundle will reset to +### +POST /taskBundle/:id/reset @org.maproulette.framework.controller.TaskBundleController.resetTaskBundle(id: Long, taskIds: List[Long]) ### # tags: [ Bundle ] # summary: Deletes a Task Bundle @@ -56,10 +78,8 @@ GET /taskBundle/:id @org.maproulette.framework.c # in: path # description: The id of the Task Bundle # required: true -# - name: primaryId -# in: query ### -DELETE /taskBundle/:id @org.maproulette.framework.controller.TaskBundleController.deleteTaskBundle(id:Long, primaryId:Option[Long]) +DELETE /taskBundle/:id @org.maproulette.framework.controller.TaskBundleController.deleteTaskBundle(id:Long) ### # tags: [ Bundle ] # summary: Unbundles tasks from Task Bundle @@ -84,5 +104,9 @@ DELETE /taskBundle/:id @org.maproulette.framework.c # in: query # description: The list of task ids to remove from the bundle # required: true +# - name: preventTaskIdUnlocks +# in: query +# description: The list of task ids to keep locked when removed from bundle +# required: true ### -POST /taskBundle/:id/unbundle @org.maproulette.framework.controller.TaskBundleController.unbundleTasks(id:Long, taskIds:List[Long]) +POST /taskBundle/:id/unbundle @org.maproulette.framework.controller.TaskBundleController.unbundleTasks(id:Long, taskIds:List[Long], preventTaskIdUnlocks:List[Long]) diff --git a/test/org/maproulette/framework/service/TaskBundleServiceSpec.scala b/test/org/maproulette/framework/service/TaskBundleServiceSpec.scala index 7529a851b..f3eb038b6 100644 --- a/test/org/maproulette/framework/service/TaskBundleServiceSpec.scala +++ b/test/org/maproulette/framework/service/TaskBundleServiceSpec.scala @@ -35,7 +35,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame ) val response = - this.service.createTaskBundle(User.superUser, "my bundle", List(task1.id, task2.id)) + this.service.createTaskBundle( + User.superUser, + "my bundle", + Some(task1.id), + List(task1.id, task2.id) + ) response.taskIds.length mustEqual 2 // tasks.bundle_id is NOT set until setTaskStatus is called!!! @@ -51,14 +56,19 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame // Cannot create a bundle with tasks already assigned intercept[InvalidException] { - this.service.createTaskBundle(User.superUser, "my bundle again", List(task1.id, task2.id)) + this.service.createTaskBundle( + User.superUser, + "my bundle again", + Some(task1.id), + List(task1.id, task2.id) + ) } } "not create a task Bundle with no tasks" taggedAs (TaskTag) in { // Cannot create a bundle with no tasks intercept[InvalidException] { - this.service.createTaskBundle(User.superUser, "my bundle again", List()) + this.service.createTaskBundle(User.superUser, "my bundle again", Some(0), List()) } } @@ -93,7 +103,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame // Cannot create a bundle with tasks from different challenges intercept[InvalidException] { - this.service.createTaskBundle(User.superUser, "bad bundle", List(task1.id, task2.id)) + this.service.createTaskBundle( + User.superUser, + "bad bundle", + Some(task1.id), + List(task1.id, task2.id) + ) } } @@ -110,7 +125,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame ) val bundle = - this.service.createTaskBundle(User.superUser, "my bundle for get", List(task1.id, task2.id)) + this.service.createTaskBundle( + User.superUser, + "my bundle for get", + Some(task1.id), + List(task1.id, task2.id) + ) val response = this.service.getTaskBundle(User.superUser, bundle.bundleId) response.bundleId mustEqual bundle.bundleId @@ -130,7 +150,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame ) val bundle = this.service - .createTaskBundle(User.superUser, "my bundle for delete", List(task1.id, task2.id)) + .createTaskBundle( + User.superUser, + "my bundle for delete", + Some(task1.id), + List(task1.id, task2.id) + ) // tasks.bundle_id is NOT set until setTaskStatus is called taskDAL.setTaskStatus( @@ -159,7 +184,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame ) val bundle = this.service - .createTaskBundle(User.superUser, "my bundle for delete", List(task1.id, task2.id)) + .createTaskBundle( + User.superUser, + "my bundle for delete", + Some(task1.id), + List(task1.id, task2.id) + ) // tasks.bundle_id is NOT set until setTaskStatus is called taskDAL.setTaskStatus( @@ -193,7 +223,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame ) val bundle = this.service - .createTaskBundle(User.superUser, "my bundle for unbundle", List(task1.id, task2.id)) + .createTaskBundle( + User.superUser, + "my bundle for unbundle", + Some(task1.id), + List(task1.id, task2.id) + ) // tasks.bundle_id is NOT set until setTaskStatus is called taskDAL.setTaskStatus( @@ -204,7 +239,7 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame primaryTaskId = Some(task1.id) ) - this.service.unbundleTasks(User.superUser, bundle.bundleId, List(task2.id))() + this.service.unbundleTasks(User.superUser, bundle.bundleId, List(task2.id), List(task1.id))() val response = this.service.getTaskBundle(User.superUser, bundle.bundleId) response.taskIds.length mustEqual 1 response.taskIds.head mustEqual task1.id @@ -223,7 +258,12 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame ) val bundle = this.service - .createTaskBundle(User.superUser, "my bundle for unbundle", List(task1.id, task2.id)) + .createTaskBundle( + User.superUser, + "my bundle for unbundle", + Some(task1.id), + List(task1.id, task2.id) + ) // tasks.bundle_id is NOT set until setTaskStatus is called taskDAL.setTaskStatus( @@ -241,7 +281,7 @@ class TaskBundleServiceSpec(implicit val application: Application) extends Frame // Random user is not allowed to delete this bundle an[IllegalAccessException] should be thrownBy - this.service.unbundleTasks(randomUser, bundle.bundleId, List(task2.id))() + this.service.unbundleTasks(randomUser, bundle.bundleId, List(task2.id), List())() } } From f15d23385225b124520b72e36b1e7197811cf179 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Tue, 14 May 2024 21:25:39 -0500 Subject: [PATCH 05/18] simplify bundling endpoints and fix variable ordering (#1112) --- .../repository/TaskBundleRepository.scala | 49 +++++++------------ 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/app/org/maproulette/framework/repository/TaskBundleRepository.scala b/app/org/maproulette/framework/repository/TaskBundleRepository.scala index 3789546d0..b9819f985 100644 --- a/app/org/maproulette/framework/repository/TaskBundleRepository.scala +++ b/app/org/maproulette/framework/repository/TaskBundleRepository.scala @@ -204,6 +204,7 @@ class TaskBundleRepository @Inject() ( val lockedTasks = this.withListLocking(user, Some(TaskType())) { () => this.taskDAL.retrieveListById(-1, 0)(taskIds) } + lockedTasks.foreach { task => try { this.lockItem(user, task) @@ -227,6 +228,10 @@ class TaskBundleRepository @Inject() ( preventTaskIdUnlocks: List[Long] ): Unit = { this.withMRConnection { implicit c => + val tasks = this.retrieveTasks( + Query.simple(List(BaseParameter("bundle_id", bundleId, table = Some("tb")))) + ) + // Unset any bundle_id on individual tasks (this is set when task is completed) SQL(s"""UPDATE tasks SET bundle_id = NULL WHERE bundle_id = {bundleId} @@ -238,28 +243,12 @@ class TaskBundleRepository @Inject() ( ) .executeUpdate() - // Remove task from bundle join table. - val tasks = this.retrieveTasks( - Query.simple( - List( - BaseParameter("bundle_id", bundleId, table = Some("tb")) - ) - ) - ) - for (task <- tasks) { if (!task.isBundlePrimary.getOrElse(false)) { - taskIds.find(id => id == task.id) match { + taskIds.find(_ == task.id) match { case Some(_) => SQL(s"""DELETE FROM task_bundles WHERE bundle_id = $bundleId AND task_id = ${task.id}""").executeUpdate() - if (!preventTaskIdUnlocks.contains(task.id)) { - try { - this.unlockItem(user, task) - } catch { - case e: Exception => this.logger.warn(e.getMessage) - } - } // This is in order to pass the filters so the task is displayed as "available" in task searching and maps. SQL(s"DELETE FROM task_review tr WHERE tr.task_id = ${task.id}").executeUpdate() @@ -274,6 +263,13 @@ class TaskBundleRepository @Inject() ( ) .executeUpdate() + if (!preventTaskIdUnlocks.contains(task.id)) { + try { + this.unlockItem(user, task) + } catch { + case e: Exception => this.logger.warn(e.getMessage) + } + } case None => // do nothing } } @@ -288,6 +284,11 @@ class TaskBundleRepository @Inject() ( */ def deleteTaskBundle(user: User, bundleId: Long): Unit = { this.withMRConnection { implicit c => + // Retrieve tasks + val tasks = this.retrieveTasks( + Query.simple(List(BaseParameter("bundle_id", bundleId, table = Some("tb")))) + ) + // Update tasks to set bundle_id and is_bundle_primary to NULL SQL( """UPDATE tasks @@ -302,20 +303,8 @@ class TaskBundleRepository @Inject() ( .on("bundleId" -> bundleId) .executeUpdate() - // Get the primary task ID - val primaryTaskId = SQL( - """SELECT id FROM tasks WHERE bundle_id = {bundleId} AND is_bundle_primary = true""" - ).on("bundleId" -> bundleId) - .as(scalar[Option[Long]].singleOpt) - .getOrElse(0) - - // Retrieve tasks - val tasks = this.retrieveTasks( - Query.simple(List(BaseParameter("bundle_id", bundleId, table = Some("tb")))) - ) - tasks.foreach { task => - if (task.id != primaryTaskId) { + if (!task.isBundlePrimary.getOrElse(false)) { try { this.unlockItem(user, task) } catch { From 420ff21a88215ae4f3d91db685b62bc3b1e15ef8 Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 18 May 2024 13:32:33 -0500 Subject: [PATCH 06/18] Upgrade anorm, postgresql, and postgis-jdbc dependencies This commit updates the versions of the following dependencies in build.sbt: - anorm and anorm-postgres from 2.6.10 to 2.7.0 - postgresql from 42.5.0 to 42.7.3 - postgis-jdbc from 2021.1.0 to 2023.1.0 The upgrade of these dependencies ensures that we are using the latest stable releases, which include various improvements and bug fixes. The URLs for the release notes of postgresql and postgis-jdbc have been added for easy reference. This change is part of the ongoing effort to keep the project dependencies up-to-date. --- build.sbt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/build.sbt b/build.sbt index 32e33cf26..30eb685c1 100644 --- a/build.sbt +++ b/build.sbt @@ -92,11 +92,14 @@ libraryDependencies ++= Seq( // and are served by the webserver at route '/assets/lib/swagger-ui/'. We have a few customized swagger files in dir // 'public/swagger'. "org.webjars" % "swagger-ui" % "5.10.3", - "org.playframework.anorm" %% "anorm" % "2.6.10", - "org.playframework.anorm" %% "anorm-postgres" % "2.6.10", - "org.postgresql" % "postgresql" % "42.5.0", - "net.postgis" % "postgis-jdbc" % "2021.1.0", - "joda-time" % "joda-time" % "2.12.0", + "org.playframework.anorm" %% "anorm" % "2.7.0", + "org.playframework.anorm" %% "anorm-postgres" % "2.7.0", + "org.postgresql" % "postgresql" % "42.7.3", // https://github.com/pgjdbc/pgjdbc/releases + // slf4j-api version 2.0.x and later use the ServiceLoader mechanism, so exclude the slf4j-api from the postgis-jdbc. + // At some point other libraries will need to be updated to use the ServiceLoader mechanism and we can remove this. + // See https://www.slf4j.org/codes.html#ignoredBindings + "net.postgis" % "postgis-jdbc" % "2023.1.0" exclude ("org.slf4j", "slf4j-api"), // https://github.com/postgis/postgis-java/releases + "joda-time" % "joda-time" % "2.12.0", // TODO(ljdelight): The vividsolutions package was moved to the Eclipse Foundation as LocationTech. // See the upgrade guide https://github.com/locationtech/jts/blob/master/MIGRATION.md "com.vividsolutions" % "jts" % "1.13", From ff2597c31d2aaa13278f6623877a50f5d9cb9302 Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 18 May 2024 15:31:36 -0500 Subject: [PATCH 07/18] Migrate JTS package from vividsolutions to Eclipse Foundation - Update the JTS (Java Topology Suite) dependency from `com.vividsolutions:jts:1.13` to `org.locationtech.jts:jts-core:1.19.0` due to the package's move to the Eclipse Foundation as LocationTech. - Update`org.wololo:jts2geojson` to 0.18.1, which has a dependency on jts-core:1.18.1. --- .../framework/repository/UserRepository.scala | 12 ++++++------ build.sbt | 7 +++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/org/maproulette/framework/repository/UserRepository.scala b/app/org/maproulette/framework/repository/UserRepository.scala index 58f0379f6..eba79107d 100644 --- a/app/org/maproulette/framework/repository/UserRepository.scala +++ b/app/org/maproulette/framework/repository/UserRepository.scala @@ -5,23 +5,23 @@ package org.maproulette.framework.repository -import java.sql.Connection - import anorm.SqlParser._ import anorm._ -import com.vividsolutions.jts.geom.{Coordinate, GeometryFactory, Point} -import com.vividsolutions.jts.io.WKTReader -import javax.inject.{Inject, Singleton} import org.joda.time.DateTime +import org.locationtech.jts.geom.{Coordinate, GeometryFactory, Point} +import org.locationtech.jts.io.WKTReader import org.maproulette.Config import org.maproulette.framework.model._ -import org.maproulette.framework.service.{ServiceManager, GrantService} import org.maproulette.framework.psql.filter._ import org.maproulette.framework.psql.{Query, SQLUtils} +import org.maproulette.framework.service.{GrantService, ServiceManager} import org.maproulette.models.dal.ChallengeDAL import play.api.db.Database import play.api.libs.json.{JsResultException, Json} +import java.sql.Connection +import javax.inject.{Inject, Singleton} + /** * The User repository handles all the sql queries that are executed against the database for the * User object diff --git a/build.sbt b/build.sbt index 30eb685c1..c12f51c0a 100644 --- a/build.sbt +++ b/build.sbt @@ -100,10 +100,9 @@ libraryDependencies ++= Seq( // See https://www.slf4j.org/codes.html#ignoredBindings "net.postgis" % "postgis-jdbc" % "2023.1.0" exclude ("org.slf4j", "slf4j-api"), // https://github.com/postgis/postgis-java/releases "joda-time" % "joda-time" % "2.12.0", - // TODO(ljdelight): The vividsolutions package was moved to the Eclipse Foundation as LocationTech. - // See the upgrade guide https://github.com/locationtech/jts/blob/master/MIGRATION.md - "com.vividsolutions" % "jts" % "1.13", - "org.wololo" % "jts2geojson" % "0.14.3", + // https://github.com/locationtech/jts/releases + "org.locationtech.jts" % "jts-core" % "1.19.0", + "org.wololo" % "jts2geojson" % "0.18.1", // https://github.com/bjornharrtell/jts2geojson/releases "org.apache.commons" % "commons-lang3" % "3.12.0", "commons-codec" % "commons-codec" % "1.14", "com.typesafe.play" %% "play-mailer" % "8.0.1", From f6ae362929b28271922871a456df7216fab8152c Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 18 May 2024 17:03:09 -0500 Subject: [PATCH 08/18] Update to the latest Play 2.8.x (#1116) The play 2.8 branch is EOL by May 31 2024. MapRoulette need to use a newer version and the first step is to update to the latest 2.8 and then step to 2.9. See here for more details . This is a step in concluding #1079. --- build.sbt | 6 +++--- project/build.properties | 1 + project/plugins.sbt | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/build.sbt b/build.sbt index c12f51c0a..179badf1d 100644 --- a/build.sbt +++ b/build.sbt @@ -107,9 +107,9 @@ libraryDependencies ++= Seq( "commons-codec" % "commons-codec" % "1.14", "com.typesafe.play" %% "play-mailer" % "8.0.1", "com.typesafe.play" %% "play-mailer-guice" % "8.0.1", - "com.typesafe.akka" %% "akka-cluster-tools" % "2.6.20", - "com.typesafe.akka" %% "akka-cluster-typed" % "2.6.20", - "com.typesafe.akka" %% "akka-slf4j" % "2.6.20", + "com.typesafe.akka" %% "akka-cluster-tools" % "2.6.21", + "com.typesafe.akka" %% "akka-cluster-typed" % "2.6.21", + "com.typesafe.akka" %% "akka-slf4j" % "2.6.21", "net.debasishg" %% "redisclient" % "3.42", "com.github.blemale" %% "scaffeine" % "5.2.1", "com.github.tototoshi" %% "scala-csv" % "1.3.10" diff --git a/project/build.properties b/project/build.properties index 563a014da..d50194b24 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1 +1,2 @@ +# https://github.com/sbt/sbt/releases sbt.version=1.7.2 diff --git a/project/plugins.sbt b/project/plugins.sbt index d9c312b4b..ded621e57 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -2,7 +2,8 @@ logLevel := Level.Warn addDependencyTreePlugin -addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.8.18") +// https://github.com/playframework/playframework/releases +addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.8.21") addSbtPlugin("com.typesafe.sbt" % "sbt-gzip" % "1.0.2") From 46de840d6d01cdba7eec147df84f153fb91c7caa Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 25 May 2024 15:42:05 -0500 Subject: [PATCH 09/18] Update to Play 2.9 (#1122) Update sbt, plugins, and other dependencies to support running with Play 2.9 because Play 2.8 is out of support at the end of May 2024. Folks that run MapRoulette locally or in some environment will need to update the APPLICATION_SECRET if it's not at lease 32 bytes. This is detailed in issue and also For more information about Play 2.9, see here . Specifically the updates are: - `build.properties` sbt version is updated from `1.7.2` to `1.9.9` - `project/plugins.sbt` Play sbt plugin is updated from `2.8.21` to `2.9.3`. - `build.sbt` supporting dependencies - `sangria-play-json` 2.0.1 -> `sangria-play-json-play29` 2.0.3 - `sangria`: 2.0.1 -> 4.0.2 - `play-json-joda`: 2.8.2 -> 2.10.5 - `play-json`: 2.8.2 -> 2.10.5 - `play-mailer`: 8.0.1 -> 9.0.0 - `play-mailer-guice`: 8.0.1 -> 9.0.0 - `joda-time`: 2.12.0 -> 2.12.7 --- .github/workflows/scala.yml | 13 +++++++++++++ build.sbt | 18 +++++++++--------- project/build.properties | 2 +- project/plugins.sbt | 7 ++++++- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/.github/workflows/scala.yml b/.github/workflows/scala.yml index b9f1692be..ca4351bc2 100644 --- a/.github/workflows/scala.yml +++ b/.github/workflows/scala.yml @@ -5,6 +5,15 @@ on: pull_request: jobs: + generate_app_secret: + runs-on: ubuntu-latest + outputs: + application_secret: ${{ steps.generate_app_secret.outputs.application_app_secret }} + steps: + - name: Generate Playframework APPLICATION_SECRET + id: generate_app_secret + run: echo "application_app_secret=$(openssl rand -base64 32)" >> "$GITHUB_OUTPUT" + sbt_formatChecks_dependencyTree: runs-on: ubuntu-latest steps: @@ -27,6 +36,7 @@ jobs: sbt_tests_jacoco: runs-on: ubuntu-latest + needs: generate_app_secret services: postgis11: image: postgis/postgis:13-3.3 @@ -48,6 +58,7 @@ jobs: distribution: 'temurin' - name: Run sbt tests with jacoco analysis env: + APPLICATION_SECRET: ${{ needs.generate_app_secret.outputs.application_secret }} CI: true MR_TEST_DB_NAME: "mr_test" MR_TEST_DB_USER: "osm" @@ -57,6 +68,7 @@ jobs: build: runs-on: ubuntu-latest + needs: generate_app_secret services: postgis11: image: postgis/postgis:13-3.3 @@ -95,6 +107,7 @@ jobs: - name: Run maproulette and the maproulette-java-client integration tests env: # maproulette overrides + APPLICATION_SECRET: ${{ needs.generate_app_secret.outputs.application_secret }} CI: true SBT_OPTS: "-Xms512M -Xmx1024M -Xss2M -XX:MaxMetaspaceSize=1024M" MR_SUPER_KEY: 1234 diff --git a/build.sbt b/build.sbt index 179badf1d..8080b4f1e 100644 --- a/build.sbt +++ b/build.sbt @@ -82,12 +82,12 @@ libraryDependencies ++= Seq( guice, // NOTE: Be careful upgrading sangria and play-json as binary incompatibilities can break graphql and the entire UI. // See the compatibility matrix here https://github.com/sangria-graphql/sangria-play-json - "org.sangria-graphql" %% "sangria-play-json" % "2.0.1", - "org.sangria-graphql" %% "sangria" % "2.0.1", - "com.typesafe.play" %% "play-json-joda" % "2.8.2", - "com.typesafe.play" %% "play-json" % "2.8.2", - "org.scalatestplus.play" %% "scalatestplus-play" % "5.1.0" % Test, - "org.scalatestplus" %% "mockito-4-5" % "3.2.12.0" % Test, + "org.sangria-graphql" %% "sangria-play-json-play29" % "2.0.3", + "org.sangria-graphql" %% "sangria" % "4.0.2", + "com.typesafe.play" %% "play-json-joda" % "2.10.5", + "com.typesafe.play" %% "play-json" % "2.10.5", + "org.scalatestplus.play" %% "scalatestplus-play" % "5.1.0" % Test, + "org.scalatestplus" %% "mockito-4-5" % "3.2.12.0" % Test, // NOTE: The swagger-ui package is used to obtain the static distribution of swagger-ui, the files included at runtime // and are served by the webserver at route '/assets/lib/swagger-ui/'. We have a few customized swagger files in dir // 'public/swagger'. @@ -99,14 +99,14 @@ libraryDependencies ++= Seq( // At some point other libraries will need to be updated to use the ServiceLoader mechanism and we can remove this. // See https://www.slf4j.org/codes.html#ignoredBindings "net.postgis" % "postgis-jdbc" % "2023.1.0" exclude ("org.slf4j", "slf4j-api"), // https://github.com/postgis/postgis-java/releases - "joda-time" % "joda-time" % "2.12.0", + "joda-time" % "joda-time" % "2.12.7", // https://github.com/locationtech/jts/releases "org.locationtech.jts" % "jts-core" % "1.19.0", "org.wololo" % "jts2geojson" % "0.18.1", // https://github.com/bjornharrtell/jts2geojson/releases "org.apache.commons" % "commons-lang3" % "3.12.0", "commons-codec" % "commons-codec" % "1.14", - "com.typesafe.play" %% "play-mailer" % "8.0.1", - "com.typesafe.play" %% "play-mailer-guice" % "8.0.1", + "com.typesafe.play" %% "play-mailer" % "9.0.0", + "com.typesafe.play" %% "play-mailer-guice" % "9.0.0", "com.typesafe.akka" %% "akka-cluster-tools" % "2.6.21", "com.typesafe.akka" %% "akka-cluster-typed" % "2.6.21", "com.typesafe.akka" %% "akka-slf4j" % "2.6.21", diff --git a/project/build.properties b/project/build.properties index d50194b24..569ab8f74 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1,2 +1,2 @@ # https://github.com/sbt/sbt/releases -sbt.version=1.7.2 +sbt.version=1.9.9 diff --git a/project/plugins.sbt b/project/plugins.sbt index ded621e57..6dc6c59de 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -3,7 +3,7 @@ logLevel := Level.Warn addDependencyTreePlugin // https://github.com/playframework/playframework/releases -addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.8.21") +addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.9.3") addSbtPlugin("com.typesafe.sbt" % "sbt-gzip" % "1.0.2") @@ -20,3 +20,8 @@ addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.10.3") addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.11.0") addSbtPlugin("com.github.sbt" % "sbt-git" % "2.0.1") + +// https://github.com/sbt/sbt/issues/6997 +ThisBuild / libraryDependencySchemes ++= Seq( + "org.scala-lang.modules" %% "scala-xml" % VersionScheme.Always +) From 891210e10decd432c8bec44f33337c3531d8fcf7 Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 25 May 2024 16:07:05 -0500 Subject: [PATCH 10/18] Remove 'connectionTestQuery' from application.conf (#1123) Remove application.conf 'db.default.hikaricp.connectionTestQuery' to allow the driver to use the Connection.isValid() API, which improves performance. [The HikariCP documentation](https://github.com/brettwooldridge/HikariCP) strongly recommends that we do not set `connectionTestQuery`: > If your driver supports JDBC4 we strongly recommend not setting this property. This is for "legacy" drivers that do not support the JDBC4 Connection.isValid() API. This is the query that will be executed just before a connection is given to you from the pool to validate that the connection to the database is still alive. Again, try running the pool without this property, HikariCP will log an error if your driver is not JDBC4 compliant to let you know. Default: none --- conf/application.conf | 1 - 1 file changed, 1 deletion(-) diff --git a/conf/application.conf b/conf/application.conf index 88bce228b..2063e0cf8 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -41,7 +41,6 @@ db { password=${?MR_DATABASE_PASSWORD} hikaricp { - connectionTestQuery="SELECT 1" # The database connection pool size can be tweaked based on available system resources and needed throughput. # Increasing this value causes parallel database transactions at the cost of more RAM, more CPU. # Note: From 9dfe6bfa6c9b3d85219ddac5595cb9fe3afbc4f7 Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sun, 26 May 2024 16:09:44 -0500 Subject: [PATCH 11/18] Update GH Action versions and test against PostGIS 13-3.3 and 16-3.4 (#1124) --- .github/workflows/scala.yml | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/.github/workflows/scala.yml b/.github/workflows/scala.yml index ca4351bc2..e30277aca 100644 --- a/.github/workflows/scala.yml +++ b/.github/workflows/scala.yml @@ -1,4 +1,4 @@ -name: Scala CI +name: MapRoulette Backend CI on: push: @@ -17,18 +17,18 @@ jobs: sbt_formatChecks_dependencyTree: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 - - name: Set up JDK 11 - uses: actions/setup-java@v3 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: java-version: 11 distribution: 'temurin' + cache: sbt - name: Create sbt dependencyTree env: CI: true run: | - sbt -Dsbt.log.format=false 'set asciiGraphWidth := 10000' 'dependencyTree' - - name: Verify scalafix passes + sbt -Dsbt.log.format=false 'set asciiGraphWidth := 10000' 'dependencyTree' 'evicted' + - name: Verify code format checks pass env: CI: true run: | @@ -38,8 +38,8 @@ jobs: runs-on: ubuntu-latest needs: generate_app_secret services: - postgis11: - image: postgis/postgis:13-3.3 + postgis: + image: postgis/postgis:16-3.4 ports: - 5432:5432 env: @@ -50,12 +50,13 @@ jobs: matrix: java: [ 11 ] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up JDK ${{ matrix.java }} - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: ${{ matrix.java }} distribution: 'temurin' + cache: sbt - name: Run sbt tests with jacoco analysis env: APPLICATION_SECRET: ${{ needs.generate_app_secret.outputs.application_secret }} @@ -70,7 +71,7 @@ jobs: runs-on: ubuntu-latest needs: generate_app_secret services: - postgis11: + postgis: image: postgis/postgis:13-3.3 ports: - 5432:5432 @@ -82,16 +83,17 @@ jobs: matrix: java: [11] steps: - - uses: actions/checkout@v3 - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 + - uses: actions/checkout@v4 with: repository: 'osmlab/maproulette-java-client' path: 'java-client' - name: Set up JDK ${{ matrix.java }} - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: ${{ matrix.java }} distribution: 'temurin' + cache: sbt - name: Run sbt compile env: CI: true From e896fc331efc2a87ae01b395518badf096c03231 Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sun, 26 May 2024 17:31:04 -0500 Subject: [PATCH 12/18] Introduce Java 17 to MapRoulette-backend CI tests (#1125) With Play framework 3.x planning to stop supporting Java 11, we're adding Java 17 support to MapRoulette-backend. The recent upgrade to Play 2.9 has helped us overcome earlier issues with Java 17. However, Java 21 is not supported yet. This update keeps Java 11 support while adding Java 17, ensuring our code works with both versions. --- .github/workflows/scala.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/scala.yml b/.github/workflows/scala.yml index e30277aca..07bf68e05 100644 --- a/.github/workflows/scala.yml +++ b/.github/workflows/scala.yml @@ -20,7 +20,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-java@v4 with: - java-version: 11 + java-version: 17 distribution: 'temurin' cache: sbt - name: Create sbt dependencyTree @@ -48,7 +48,7 @@ jobs: POSTGRES_PASSWORD: osm strategy: matrix: - java: [ 11 ] + java: [ 11, 17 ] steps: - uses: actions/checkout@v4 - name: Set up JDK ${{ matrix.java }} @@ -81,7 +81,7 @@ jobs: POSTGRES_PASSWORD: osm strategy: matrix: - java: [11] + java: [17] steps: - uses: actions/checkout@v4 - uses: actions/checkout@v4 @@ -91,6 +91,7 @@ jobs: - name: Set up JDK ${{ matrix.java }} uses: actions/setup-java@v4 with: + # https://github.com/actions/setup-java?tab=readme-ov-file#install-multiple-jdks java-version: ${{ matrix.java }} distribution: 'temurin' cache: sbt From 9026e61da03f386e39c0a61b5730b1efd091b395 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Mon, 27 May 2024 18:23:33 -0500 Subject: [PATCH 13/18] fix changeset submittion bug for tag fix tasks with relations (#1121) --- .../maproulette/provider/osm/objects/VersionedObjects.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/org/maproulette/provider/osm/objects/VersionedObjects.scala b/app/org/maproulette/provider/osm/objects/VersionedObjects.scala index 6ee54c968..7d2767301 100644 --- a/app/org/maproulette/provider/osm/objects/VersionedObjects.scala +++ b/app/org/maproulette/provider/osm/objects/VersionedObjects.scala @@ -164,7 +164,7 @@ case class VersionedRelation( ) extends VersionedObject { override def toChangeElement(changesetId: Int): Elem = { - + { for (member <- members) yield % Attribute( @@ -176,7 +176,7 @@ case class VersionedRelation( for (tagKV <- tags) yield % Attribute("k", Text(tagKV._1), Attribute("v", Text(tagKV._2), Null)) } - % Attribute( + % Attribute( "visible", Text(visible.toString), Attribute( From b130e5641ba231a401015b87c0ddd256b9ae3e3b Mon Sep 17 00:00:00 2001 From: Taylor Smock <45215054+tsmock@users.noreply.github.com> Date: Sat, 1 Jun 2024 14:03:20 -0600 Subject: [PATCH 14/18] Store new API key instead of old API key on API key reset (#1127) --- app/controllers/AuthController.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/AuthController.scala b/app/controllers/AuthController.scala index 56c856158..caf5ae1ea 100644 --- a/app/controllers/AuthController.scala +++ b/app/controllers/AuthController.scala @@ -306,7 +306,7 @@ class AuthController @Inject() ( case Some(updated) => updated.apiKey match { case Some(api) => { - Future(storeAPIKeyInOSM(user)) + Future(storeAPIKeyInOSM(updated)) Ok(api) } case None => NoContent From c6d2bf328a39e6bc9cd692244efbb9f8f5da4f4c Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 1 Jun 2024 19:11:28 -0500 Subject: [PATCH 15/18] Introduce maproulette.secret.key conf and MAPROULETTE_SECRET_KEY env (#1128) MapRoulette API had a dependency on the Play framework's secret key, and this patch separates those keys. The MapRoulette secret is used to encrypt things specific to maproulette and it no longer depends on the Play application key. Play 2.9 introduced a change where its application key must be 32 bytes or longer, and this impacted MapRoulette's cryptography with its own internal items. It's easier to manage these separately, similar to how the OSM secrets are not used to encrypt data stored within MapRoulette. --- app/org/maproulette/framework/model/User.scala | 7 ++++--- app/org/maproulette/utils/Crypto.scala | 2 +- conf/application.conf | 6 ++++++ conf/dev.conf.example | 12 ++++++++++++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app/org/maproulette/framework/model/User.scala b/app/org/maproulette/framework/model/User.scala index 25c7a0f1d..5aa535dd1 100644 --- a/app/org/maproulette/framework/model/User.scala +++ b/app/org/maproulette/framework/model/User.scala @@ -283,6 +283,7 @@ object User extends CommonField { implicit val osmReads: Reads[OSMProfile] = Json.reads[OSMProfile] implicit val searchResultWrites: Writes[UserSearchResult] = Json.writes[UserSearchResult] implicit val projectManagerWrites: Writes[ProjectManager] = Json.writes[ProjectManager] + val logger = LoggerFactory.getLogger(this.getClass) val TABLE = "users" val FIELD_OSM_ID = "osm_id" @@ -416,9 +417,9 @@ object User extends CommonField { user.copy(apiKey = decryptedAPIKey) } catch { case _: BadPaddingException | _: IllegalBlockSizeException => - LoggerFactory - .getLogger(this.getClass) - .debug("Invalid key found, could be that the application secret on server changed.") + logger.debug( + "Invalid key found, could be that the application secret on server changed." + ) user case e: Throwable => throw e } diff --git a/app/org/maproulette/utils/Crypto.scala b/app/org/maproulette/utils/Crypto.scala index 6cd8b1860..3079ce400 100644 --- a/app/org/maproulette/utils/Crypto.scala +++ b/app/org/maproulette/utils/Crypto.scala @@ -18,7 +18,7 @@ import org.maproulette.Config */ @Singleton class Crypto @Inject() (config: Config) { - val key = config.config.get[String]("play.http.secret.key") + val key: String = config.config.get[String]("maproulette.secret.key") def encrypt(value: String): String = { val cipher: Cipher = Cipher.getInstance("AES/ECB/PKCS5Padding") diff --git a/conf/application.conf b/conf/application.conf index 2063e0cf8..e7a26d6a3 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -178,6 +178,12 @@ maproulette { publicOrigin="https://maproulette.org" emailFrom="maproulette@example.com" + # The MapRoulette API secret key used to encrypt/decrypt sensitive things from the database, like user API Keys. + # Do not use the default value in production, generate a new key and set it via conf or 'MAPROULETTE_SECRET_KEY' env. + # A secure way to get a distinct key is to run 'openssl rand -base64 32' and set the output as the secret key. + secret.key = "%APPLICATION_SECRET%" + secret.key = "${?MAPROULETTE_SECRET_KEY}" + # redirect for OSM frontend="http://127.0.0.1:3000" diff --git a/conf/dev.conf.example b/conf/dev.conf.example index 82eaee65c..b6f0861b9 100644 --- a/conf/dev.conf.example +++ b/conf/dev.conf.example @@ -1,5 +1,11 @@ include "application.conf" +# The Play application secret key. It's okay for localhost dev conf to be public, but not for any public environment. +# A secure way to get a distinct key is to run 'openssl rand -base64 32' and set the output as the secret key. +# Play 2.9 requires a key of at least 32 characters https://github.com/maproulette/maproulette-backend/issues/1117 +play.http.secret.key = "DEVLOCAL_1z8rvducX6AaMTXQl4olw71YHj3MCFpRXXTB73TNnTc=" +play.http.secret.key = "${?APPLICATION_SECRET}" + db.default { url="jdbc:postgresql://localhost:5432/mp_dev" url=${?MR_DATABASE_URL} @@ -16,6 +22,12 @@ maproulette { debug=true bootstrap=true + # The MapRoulette API secret key used to encrypt/decrypt sensitive things from the database, like user API Keys. + # Do not use the default value in production, generate a new key and set it via conf or 'MAPROULETTE_SECRET_KEY' env. + # A secure way to get a distinct key is to run 'openssl rand -base64 32' and set the output as the secret key. + secret.key = "DEVLOCAL_Jw8W2PMl434eL85+IRvoT7DA+eNR9a9N3ZK2Gfx4ecs=" + secret.key = "${?MAPROULETTE_SECRET_KEY}" + scheduler { startTimeJitterForMinuteTasks = "15 seconds" startTimeJitterForHourTasks = "30 seconds" From f1e9b83f3a67f965c60ec91a09b93ee06b7d50a0 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Sat, 1 Jun 2024 21:06:05 -0500 Subject: [PATCH 16/18] update task caching for task bundling endpoints (#1126) --- .../framework/repository/TaskBundleRepository.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/org/maproulette/framework/repository/TaskBundleRepository.scala b/app/org/maproulette/framework/repository/TaskBundleRepository.scala index b9819f985..1e322d7c4 100644 --- a/app/org/maproulette/framework/repository/TaskBundleRepository.scala +++ b/app/org/maproulette/framework/repository/TaskBundleRepository.scala @@ -98,6 +98,7 @@ class TaskBundleRepository @Inject() ( } catch { case e: Exception => this.logger.warn(e.getMessage) } + taskRepository.cacheManager.cache.remove(task.id) } TaskBundle(bundleId, user.id, lockedTasks.map(task => { @@ -212,6 +213,7 @@ class TaskBundleRepository @Inject() ( case e: Exception => this.logger.warn(e.getMessage) } + taskRepository.cacheManager.cache.remove(task.id) } } } @@ -270,6 +272,7 @@ class TaskBundleRepository @Inject() ( case e: Exception => this.logger.warn(e.getMessage) } } + taskRepository.cacheManager.cache.remove(task.id) case None => // do nothing } } @@ -311,6 +314,7 @@ class TaskBundleRepository @Inject() ( case e: Exception => this.logger.warn(e.getMessage) } } + taskRepository.cacheManager.cache.remove(task.id) } } } From 60fbbd8c6e830413476c01d4bd4c68a31169db52 Mon Sep 17 00:00:00 2001 From: Lucas Burson Date: Sat, 8 Jun 2024 21:55:05 -0500 Subject: [PATCH 17/18] Remove quotes around HOCON substitutions (#1133) In HOCON, variable substitutions are not performed if the replacement part is in quotes. From the manual: Substitutions are not parsed inside quoted strings. To get a string containing a substitution, you must use value concatenation with the substitution in the unquoted portion: key : ${animal.favorite} is my favorite animal --- conf/application.conf | 2 +- conf/dev.conf.example | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/conf/application.conf b/conf/application.conf index e7a26d6a3..d2a90bbfe 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -182,7 +182,7 @@ maproulette { # Do not use the default value in production, generate a new key and set it via conf or 'MAPROULETTE_SECRET_KEY' env. # A secure way to get a distinct key is to run 'openssl rand -base64 32' and set the output as the secret key. secret.key = "%APPLICATION_SECRET%" - secret.key = "${?MAPROULETTE_SECRET_KEY}" + secret.key = ${?MAPROULETTE_SECRET_KEY} # redirect for OSM frontend="http://127.0.0.1:3000" diff --git a/conf/dev.conf.example b/conf/dev.conf.example index b6f0861b9..987966eff 100644 --- a/conf/dev.conf.example +++ b/conf/dev.conf.example @@ -4,7 +4,7 @@ include "application.conf" # A secure way to get a distinct key is to run 'openssl rand -base64 32' and set the output as the secret key. # Play 2.9 requires a key of at least 32 characters https://github.com/maproulette/maproulette-backend/issues/1117 play.http.secret.key = "DEVLOCAL_1z8rvducX6AaMTXQl4olw71YHj3MCFpRXXTB73TNnTc=" -play.http.secret.key = "${?APPLICATION_SECRET}" +play.http.secret.key = ${?APPLICATION_SECRET} db.default { url="jdbc:postgresql://localhost:5432/mp_dev" @@ -26,7 +26,7 @@ maproulette { # Do not use the default value in production, generate a new key and set it via conf or 'MAPROULETTE_SECRET_KEY' env. # A secure way to get a distinct key is to run 'openssl rand -base64 32' and set the output as the secret key. secret.key = "DEVLOCAL_Jw8W2PMl434eL85+IRvoT7DA+eNR9a9N3ZK2Gfx4ecs=" - secret.key = "${?MAPROULETTE_SECRET_KEY}" + secret.key = ${?MAPROULETTE_SECRET_KEY} scheduler { startTimeJitterForMinuteTasks = "15 seconds" From 5949dc0636120e29a9397a52b8087c4efb319f59 Mon Sep 17 00:00:00 2001 From: Collin Beczak <88843144+CollinBeczak@users.noreply.github.com> Date: Mon, 10 Jun 2024 19:23:05 -0500 Subject: [PATCH 18/18] update tasks_remaining column in challenge table (#1131) --- .../framework/repository/ChallengeRepository.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/org/maproulette/framework/repository/ChallengeRepository.scala b/app/org/maproulette/framework/repository/ChallengeRepository.scala index 55e5c5909..f4d8efdac 100644 --- a/app/org/maproulette/framework/repository/ChallengeRepository.scala +++ b/app/org/maproulette/framework/repository/ChallengeRepository.scala @@ -173,11 +173,12 @@ class ChallengeRepository @Inject() (override val db: Database) extends Reposito SQL( s""" |UPDATE challenges - |SET completion_percentage = new_completion_percentage + |SET completion_percentage = new_completion_percentage, tasks_remaining = new_tasks_remaining |FROM ( | SELECT | challenges.id AS challenge_id, - | completed_task_counts.completed_tasks * 100 / total_task_counts.total_tasks AS new_completion_percentage + | completed_task_counts.completed_tasks * 100 / total_task_counts.total_tasks AS new_completion_percentage, + | total_task_counts.total_tasks - completed_task_counts.completed_tasks AS new_tasks_remaining | FROM | challenges | JOIN (