Skip to content

Commit

Permalink
refine tests and prevent falsy values on osmIdProperty
Browse files Browse the repository at this point in the history
  • Loading branch information
@CollinBeczak
CollinBeczak committed Aug 29, 2024
1 parent 003183f commit 241f7b6
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 101 deletions.
122 changes: 53 additions & 69 deletions app/org/maproulette/provider/ChallengeProvider.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,97 +281,81 @@ class ChallengeProvider @Inject() (
}
}
}
// Helper function to retrieve a non-null, non-empty string from a field
// Supports both string and numeric IDs.
def getNonNullString(value: JsValue, fieldName: String): Option[String] = {
(value \ fieldName).asOpt[JsValue].flatMap {
case JsString(str) if str.nonEmpty => Some(str)
case JsNumber(num) => Some(num.toString)
case _ => None
}
}

/**
* Checks for the first valid ID in the specified list of field names.
*/
def findName(value: JsValue, fields: List[String]): Option[String] = {
fields.iterator.flatMap(fieldName => getNonNullString(value, fieldName)).toList.headOption
}

/**
* Extracts the OSM id from the given JsValue based on the `osmIdProperty`
* Extracts the OSM ID from the given JsValue based on the `osmIdProperty`
* challenge field. Returns None if either the challenge has not specified an
* osmIdProperty or if the JsValue contains neither a field nor property with
* the specified name. If the JsValue represents a collection of features,
* each feature will be checked and the first OSM id found returned
* each feature will be checked and the first OSM ID found will be returned.
*/
def featureOSMId(value: JsValue, challenge: Challenge): Option[String] = {
challenge.extra.osmIdProperty match {
case Some(osmIdName) =>
// Whether `value` represents multiple features or just one, process as List
val features = (value \ "features").asOpt[List[JsValue]].getOrElse(List(value))
features
.map(feature =>
// First look for a matching field on the feature itself. If not found, then
// look at the feature's properties
(feature \ osmIdName).asOpt[String] match {
case Some(matchingIdField) => Some(matchingIdField)
case None =>
(feature \ "properties").asOpt[JsObject] match {
case Some(properties) =>
(properties \ osmIdName).asOpt[String] match {
case Some(matchingIdProperty) => Some(matchingIdProperty)
case None => None // feature doesn't have the id property
}
case None => None // feature doesn't have any properties
}
}
)
.find(_.isDefined) match { // first feature that has a match
case Some(featureWithId) => featureWithId
case None => None // No features found with matching id field or property
challenge.extra.osmIdProperty.flatMap { osmIdName =>
// Whether `value` represents multiple features or just one, process as List
val features = (value \ "features").asOpt[List[JsValue]].getOrElse(List(value))

features.flatMap { feature =>
// First look for a matching field on the feature itself
getNonNullString(feature, osmIdName).orElse {
// If not found on the feature, then look at the feature's properties
(feature \ "properties").asOpt[JsObject].flatMap { properties =>
getNonNullString(properties, osmIdName)
}
}
case None => None // No osmIdProperty defined on challenge
}.headOption // Get the first non-empty match
}
}

/**
* Extracts an appropriate task name from the given JsValue, looking for any
* of multiple suitable id fields, or finally defaulting to a random UUID if
* no acceptable field is found
* of multiple suitable ID fields, or finally defaulting to a random UUID if
* no acceptable field is found.
*/
def taskNameFromJsValue(value: JsValue, challenge: Challenge): String = {

// Helper function to retrieve a non-null, non-empty string from a field
// Supports both string and numeric ids.
def getNonNullString(fieldName: String): Option[String] = {
(value \ fieldName).asOpt[JsValue].flatMap {
case JsString(str) if str.nonEmpty => Some(str)
case JsNumber(num) => Some(num.toString)
case _ => None
}
}

// Check for the first valid ID in the specified list of field names
def findName(fields: List[String]): Option[String] = {
fields.iterator.flatMap(fieldName => getNonNullString(fieldName)).toList.headOption
// Use field/property specified by challenge, if available. Otherwise look
// for commonly used ID fields/properties
if (challenge.extra.osmIdProperty.exists(_.nonEmpty)) {
return featureOSMId(value, challenge).getOrElse(UUID.randomUUID().toString)
}

// Use field/property specified by challenge, if available. Otherwise look
// for commonly used id fields/properties
challenge.extra.osmIdProperty.flatMap { osmIdProperty =>
if (osmIdProperty.nonEmpty) {
featureOSMId(value, challenge)
} else {
None
(value \ "features")
.asOpt[List[JsValue]]
.flatMap(_.headOption)
.flatMap { firstFeature =>
val name = taskNameFromJsValue(firstFeature, challenge)
if (name.nonEmpty) Some(name) else None
}
} getOrElse {
(value \ "features").asOpt[List[JsValue]].flatMap(_.headOption).flatMap { firstFeature =>
taskNameFromJsValue(firstFeature, challenge) match {
case "" => None
case id => Some(id)
}
} getOrElse {
.getOrElse {
val nameKeys = List("id", "@id", "osmid", "osm_id", "name")
findName(nameKeys).getOrElse {
(value \ "properties").asOpt[JsObject].flatMap { properties =>
taskNameFromJsValue(properties, challenge) match {
case "" => None
case id => Some(id)
findName(value, nameKeys).getOrElse {
(value \ "properties")
.asOpt[JsObject]
.flatMap { properties =>
val name = taskNameFromJsValue(properties, challenge)
if (name.nonEmpty) Some(name) else None
}
.getOrElse {
// If we still don't find anything, create a UUID for it.
UUID.randomUUID().toString
}
} getOrElse {
// if we still don't find anything, create a UUID for it. The
// caveat to this is that if you upload the same file again, it
// will create duplicate tasks
UUID.randomUUID().toString
}
}
}
}
}

/**
Expand Down
59 changes: 27 additions & 32 deletions test/org/maproulette/provider/ChallengeProviderSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,21 @@ class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
repository.featureOSMId(json, challengeWithOsmId) mustEqual Some("singleFeatureId")
}

"return OSM ID from root if present and specified in challenge (numeric)" in {
val json = Json.obj("custom_osm_id" -> 9999999)
repository.featureOSMId(json, challengeWithOsmId) mustEqual Some("9999999")
}

"return OSM ID from properties if specified in challenge" in {
val json = Json.obj("properties" -> Json.obj("custom_osm_id" -> "propertyId"))
repository.featureOSMId(json, challengeWithOsmId) mustEqual Some("propertyId")
}

"return OSM ID from properties if specified in challenge (numeric)" in {
val json = Json.obj("properties" -> Json.obj("custom_osm_id" -> 9999999))
repository.featureOSMId(json, challengeWithOsmId) mustEqual Some("9999999")
}

"return None if OSM ID not found in root or properties" in {
val json = Json.obj("otherField" -> "value")
repository.featureOSMId(json, challengeWithOsmId) mustEqual None
Expand All @@ -69,7 +79,13 @@ class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
}

"return None if JSON has no features and challenge does not specify OSM ID property" in {
val json = Json.obj()
val json = Json.obj("custom_osm_id" -> null, "fillerProperty" -> "string")
repository.featureOSMId(json, challengeWithoutOsmId) mustEqual None
}

"return None if properties object is empty and challenge does not specify OSM ID property" in {
val json =
Json.obj("properties" -> Json.obj("custom_osm_id" -> null, "fillerProperty" -> "string"))
repository.featureOSMId(json, challengeWithoutOsmId) mustEqual None
}
}
Expand All @@ -80,11 +96,6 @@ class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
repository.taskNameFromJsValue(json, challengeWithOsmId) mustEqual "12345"
}

"return OSM ID from first feature if available and specified in challenge" in {
val json = Json.obj("features" -> Json.arr(Json.obj("custom_osm_id" -> "featureId123")))
repository.taskNameFromJsValue(json, challengeWithOsmId) mustEqual "featureId123"
}

"return random UUID if OSM ID field is specified but not found" in {
val json = Json.obj("otherField" -> "value")
val result = repository.taskNameFromJsValue(json, challengeWithOsmId)
Expand All @@ -109,37 +120,21 @@ class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
assert(UUID.fromString(result).toString == result)
}

"return random UUID if properties object is empty and no ID fields are found" in {
val json = Json.obj("properties" -> Json.obj())
val result = repository.taskNameFromJsValue(json, challengeWithoutOsmId)
assert(UUID.fromString(result).toString == result)
}

"return ID field from root object or properties if available" in {
val jsonRoot = Json.obj("id" -> "testId")
val jsonProps = Json.obj("properties" -> Json.obj("id" -> "testId"))
repository.taskNameFromJsValue(jsonRoot, challengeWithoutOsmId) mustEqual "testId"
repository.taskNameFromJsValue(jsonProps, challengeWithoutOsmId) mustEqual "testId"
}

"return UUID if ID field is 'null'" in {
val json = Json.obj("id" -> null)
val result = repository.taskNameFromJsValue(json, challengeWithoutOsmId)
assert(UUID.fromString(result).toString == result)
}

"return field from properties if ID field is null and other fields are valid" in {
val json =
Json.obj("id" -> null, "properties" -> Json.obj("name" -> "testName", "id" -> "idstring"))
"return field from properties if ID field is 'null' and other fields are valid" in {
val json = Json.obj(
"id" -> null,
"properties" -> Json
.obj("name" -> "testName", "id" -> "idstring", "fillerProperty" -> "string")
)
repository.taskNameFromJsValue(json, challengeWithoutOsmId) mustEqual "idstring"
}

"return field from properties if ID field is 'null' and other fields are valid" in {
"return field from properties if ID field is null and properties contain valid IDs" in {
val json = Json.obj(
"name" -> "string",
"properties" -> Json.obj("name" -> "testName", "id" -> "idstring")
"fillerProperty" -> "string",
"properties" -> Json.obj("fillerProperty" -> "string", "id" -> null, "name" -> "testName")
)
repository.taskNameFromJsValue(json, challengeWithoutOsmId) mustEqual "string"
repository.taskNameFromJsValue(json, challengeWithoutOsmId) mustEqual "testName"
}
}
}

0 comments on commit 241f7b6

Please sign in to comment.