add test and prevent null values on task name setup

app/org/maproulette/provider/ChallengeProvider.scala

@@ -289,7 +289,7 @@ class ChallengeProvider @Inject() (
     * the specified name. If the JsValue represents a collection of features,
     * each feature will be checked and the first OSM id found returned
     */
-  private def featureOSMId(value: JsValue, challenge: Challenge): Option[String] = {
+  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
@@ -324,41 +324,52 @@ class ChallengeProvider @Inject() (
     * of multiple suitable id fields, or finally defaulting to a random UUID if
     * no acceptable field is found
     */
-  private def taskNameFromJsValue(value: JsValue, challenge: Challenge): String = {
-    // Use field/property specified by challenge, if available. Otherwise look
-    // for commonly used id fields/properties
-    if (!challenge.extra.osmIdProperty.getOrElse("").isEmpty) {
-      return featureOSMId(value, challenge) match {
-        case Some(osmId) => osmId
-        case None        => UUID.randomUUID().toString // task does not contain id property
+  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
       }
     }
 
-    val featureList = (value \ "features").asOpt[List[JsValue]]
-    if (featureList.isDefined) {
-      taskNameFromJsValue(featureList.get.head, challenge) // Base name on first feature
-    } else {
-      val nameKeys = List.apply("id", "@id", "osmid", "osm_id", "name")
-      nameKeys.collectFirst {
-        case x if (value \ x).asOpt[JsValue].isDefined =>
-          // Support both string and numeric ids. If it's a string, use it.
-          // Otherwise convert the value to a string
-          (value \ x).asOpt[String] match {
-            case Some(stringValue) => stringValue
-            case None              => (value \ x).asOpt[JsValue].get.toString
-          }
-      } match {
-        case Some(n) => n
-        case None =>
-          (value \ "properties").asOpt[JsObject] match {
-            // See if we can find an id field on the feature properties
-            case Some(properties) => taskNameFromJsValue(properties, challenge)
-            case None             =>
-              // 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
+    // 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
+    challenge.extra.osmIdProperty.flatMap { osmIdProperty =>
+      if (osmIdProperty.nonEmpty) {
+        featureOSMId(value, challenge)
+      } else {
+        None
+      }
+    } getOrElse {
+      (value \ "features").asOpt[List[JsValue]].flatMap(_.headOption).flatMap { firstFeature =>
+        taskNameFromJsValue(firstFeature, challenge) match {
+          case "" => None
+          case id => Some(id)
+        }
+      } 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)
+            }
+          } 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
           }
+        }
       }
     }
   }

test/org/maproulette/provider/ChallengeProviderSpec.scala

@@ -0,0 +1,145 @@
+import org.joda.time.DateTime
+import org.maproulette.framework.model._
+import org.maproulette.provider.ChallengeProvider
+import org.scalatestplus.play.PlaySpec
+import org.scalatestplus.mockito.MockitoSugar
+import play.api.libs.json._
+import java.util.UUID
+
+class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
+  val repository: ChallengeProvider = new ChallengeProvider(null, null, null, null, null)
+
+  val challengeWithOsmId = Challenge(
+    1,
+    "ChallengeWithOsmId",
+    DateTime.now(),
+    DateTime.now(),
+    None,
+    false,
+    None,
+    ChallengeGeneral(101, 1, ""),
+    ChallengeCreation(),
+    ChallengePriority(),
+    ChallengeExtra(osmIdProperty = Some("custom_osm_id"))
+  )
+
+  val challengeWithoutOsmId = Challenge(
+    1,
+    "ChallengeWithoutOsmId",
+    DateTime.now(),
+    DateTime.now(),
+    None,
+    false,
+    None,
+    ChallengeGeneral(101, 1, ""),
+    ChallengeCreation(),
+    ChallengePriority(),
+    ChallengeExtra()
+  )
+
+  "featureOSMId" should {
+    "return OSM ID from root if present and specified in challenge" in {
+      val json = Json.obj("custom_osm_id" -> "singleFeatureId")
+      repository.featureOSMId(json, challengeWithOsmId) mustEqual Some("singleFeatureId")
+    }
+
+    "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 None if OSM ID not found in root or properties" in {
+      val json = Json.obj("otherField" -> "value")
+      repository.featureOSMId(json, challengeWithOsmId) mustEqual None
+    }
+
+    "return OSM ID from first feature in list if specified in challenge" in {
+      val json = Json.obj("features" -> Json.arr(Json.obj("custom_osm_id" -> "featureId1")))
+      repository.featureOSMId(json, challengeWithOsmId) mustEqual Some("featureId1")
+    }
+
+    "return None if features do not contain specified OSM ID field" in {
+      val json = Json.obj("features" -> Json.arr(Json.obj("otherField" -> "value1")))
+      repository.featureOSMId(json, challengeWithOsmId) mustEqual None
+    }
+
+    "return None if challenge does not specify OSM ID property" in {
+      val json = Json.obj("features" -> Json.arr(Json.obj("custom_osm_id" -> "featureId1")))
+      repository.featureOSMId(json, challengeWithoutOsmId) mustEqual None
+    }
+
+    "return None if JSON has no features and challenge does not specify OSM ID property" in {
+      val json = Json.obj()
+      repository.featureOSMId(json, challengeWithoutOsmId) mustEqual None
+    }
+  }
+
+  "taskNameFromJsValue" should {
+    "return OSM ID from root object if present and specified in challenge" in {
+      val json = Json.obj("custom_osm_id" -> "12345")
+      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)
+      assert(UUID.fromString(result).toString == result)
+    }
+
+    "return random UUID if no valid feature ID is found and no challenge-specific ID is available" in {
+      val json   = Json.obj("features" -> Json.arr(Json.obj("otherField" -> "value")))
+      val result = repository.taskNameFromJsValue(json, challengeWithoutOsmId)
+      assert(UUID.fromString(result).toString == result)
+    }
+
+    "return random UUID if no valid ID fields are found" in {
+      val json   = Json.obj()
+      val result = repository.taskNameFromJsValue(json, challengeWithoutOsmId)
+      assert(UUID.fromString(result).toString == result)
+    }
+
+    "return random UUID if features array is empty" in {
+      val json   = Json.obj("features" -> Json.arr())
+      val result = repository.taskNameFromJsValue(json, challengeWithoutOsmId)
+      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"))
+      repository.taskNameFromJsValue(json, challengeWithoutOsmId) mustEqual "idstring"
+    }
+
+    "return field from properties if ID field is 'null' and other fields are valid" in {
+      val json = Json.obj(
+        "name"       -> "string",
+        "properties" -> Json.obj("name" -> "testName", "id" -> "idstring")
+      )
+      repository.taskNameFromJsValue(json, challengeWithoutOsmId) mustEqual "string"
+    }
+  }
+}