feat: update Shortname value object (#2851)

Co-authored-by: Balduin Landolt <33053745+BalduinLandolt@users.noreply.github.com>
dasch-swiss · Sep 25, 2023 · 35187ca · 35187ca
1 parent fde1faf
commit 35187ca
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 62 deletions.
diff --git a/docs/03-endpoints/api-admin/projects.md b/docs/03-endpoints/api-admin/projects.md
@@ -89,8 +89,9 @@ Description: Create a new project.
 Required payload:
 
 - `shortcode` (unique, 4-digits)
-- `shortname` (unique, it should be in the form of a [xsd:NCNAME](https://www.w3.org/TR/xmlschema11-2/#NCName) and it 
-  should be URL safe)
+- `shortname` (unique, 3-20 characters long, can contain small and capital letters, numbers, special characters: `-`
+and `_`, cannot start with number nor allowed special characters, should be in the form of a
+[xsd:NCNAME](https://www.w3.org/TR/xmlschema11-2/#NCName) and URL safe)
 - `description` (collection of descriptions as strings with language tag)
 - `keywords` (collection of keywords)
 - `status` (true, if project is active. false, if project is inactive)

diff --git a/integration/src/test/scala/org/knora/webapi/e2e/admin/ProjectsADME2EZioHttpSpec.scala b/integration/src/test/scala/org/knora/webapi/e2e/admin/ProjectsADME2EZioHttpSpec.scala
@@ -239,7 +239,7 @@ class ProjectsADME2EZioHttpSpec extends E2ESpec with ProjectsADMJsonProtocol wit
         val params =
           s"""{
              |    "id": "$customProjectIri",
-             |    "shortname": "newprojectWithDuplicateIri",
+             |    "shortname": "newWithDuplicateIri",
              |    "shortcode": "2222",
              |    "longname": "new project with a duplicate custom invalid IRI",
              |    "description": [{"value": "a project created with a duplicate custom IRI", "language": "en"}],

diff --git a/integration/src/test/scala/org/knora/webapi/responders/admin/ProjectsResponderADMSpec.scala b/integration/src/test/scala/org/knora/webapi/responders/admin/ProjectsResponderADMSpec.scala
@@ -290,7 +290,7 @@ class ProjectsResponderADMSpec extends CoreSpec with ImplicitSender {
         val keywordWithSpecialCharacter     = "new \\\"keyword\\\""
         appActor ! ProjectCreateRequestADM(
           createRequest = ProjectCreatePayloadADM(
-            shortname = Shortname.make("project_with_character").fold(error => throw error.head, value => value),
+            shortname = Shortname.make("project_with_char").fold(error => throw error.head, value => value),
             shortcode = Shortcode.make("1312").fold(error => throw error.head, value => value), // lower case
             longname = Name.make(Some(longnameWithSpecialCharacter)).fold(error => throw error.head, value => value),
             description = ProjectDescription
@@ -604,7 +604,6 @@ class ProjectsResponderADMSpec extends CoreSpec with ImplicitSender {
     }
 
     "used to query keywords" should {
-
       "return all unique keywords for all projects" in {
         appActor ! ProjectsKeywordsGetRequestADM()
         val received: ProjectsKeywordsGetResponseADM = expectMsgType[ProjectsKeywordsGetResponseADM](timeout)

diff --git a/webapi/src/main/scala/dsp/valueobjects/Project.scala b/webapi/src/main/scala/dsp/valueobjects/Project.scala
@@ -20,21 +20,13 @@ object Project {
   // A regex for matching a string containing the project ID.
   private val ProjectIDRegex: Regex = ("^" + ProjectIDPattern + "$").r
 
-  // A regex sub-pattern for ontology prefix labels and local entity names. According to
-  // <https://www.w3.org/TR/turtle/#prefixed-name>, a prefix label in Turtle must be a valid XML NCName
-  // <https://www.w3.org/TR/1999/REC-xml-names-19990114/#NT-NCName>. Knora also requires a local entity name to
-  // be an XML NCName.
-  private val NCNamePattern: String = """[\p{L}_][\p{L}0-9_.-]*"""
-
-  // A regex for matching a string containing only an ontology prefix label or a local entity name.
-  private val NCNameRegex: Regex = ("^" + NCNamePattern + "$").r
-
-  // A regex sub-pattern matching the random IDs generated by KnoraIdUtil, which are Base64-encoded
-  // using the "URL and Filename safe" Base 64 alphabet, without padding, as specified in Table 2 of
-  // RFC 4648.
-  private val Base64UrlPattern = "[A-Za-z0-9_-]+"
-
-  private val Base64UrlPatternRegex: Regex = ("^" + Base64UrlPattern + "$").r
+  /**
+   * Regex which matches string that:
+   * - is 3-20 characters long,
+   * - contains small and capital letters, numbers, special characters: `-` and `_`,
+   * - cannot start with number nor allowed special characters.
+   */
+  private val shortnameRegex: Regex = "^[a-zA-Z][a-zA-Z0-9_-]{2,19}$".r
 
   /**
    * Check that the string represents a valid project shortname.
@@ -43,9 +35,8 @@ object Project {
    * @return the same string.
    */
   def validateAndEscapeProjectShortname(shortname: String): Option[String] =
-    NCNameRegex
+    shortnameRegex
       .findFirstIn(shortname)
-      .flatMap(Base64UrlPatternRegex.findFirstIn)
       .flatMap(Iri.toSparqlEncodedString)
 
   // TODO-mpro: longname, description, keywords, logo are missing enhanced validation
@@ -77,8 +68,8 @@ object Project {
    */
   sealed abstract case class Shortname private (value: String)
   object Shortname { self =>
-    implicit val decoder: JsonDecoder[Shortname] = JsonDecoder[String].mapOrFail { case value =>
-      Shortname.make(value).toEitherWith(e => e.head.getMessage())
+    implicit val decoder: JsonDecoder[Shortname] = JsonDecoder[String].mapOrFail { value =>
+      Shortname.make(value).toEitherWith(e => e.head.getMessage)
     }
     implicit val encoder: JsonEncoder[Shortname] =
       JsonEncoder[String].contramap((shortname: Shortname) => shortname.value)

diff --git a/webapi/src/test/scala/dsp/valueobjects/ProjectSpec.scala b/webapi/src/test/scala/dsp/valueobjects/ProjectSpec.scala
@@ -19,8 +19,6 @@ import dsp.valueobjects.Project._
 object ProjectSpec extends ZIOSpecDefault {
   private val validShortcode   = "1234"
   private val invalidShortcode = "12345"
-  private val validShortname1  = "valid-shortname"
-  private val validShortname2  = "valid_1111"
   private val validName        = "That is the project longname"
   private val validDescription = Seq(
     V2.StringLiteralV2(value = "Valid project description", language = Some("en"))
@@ -60,49 +58,43 @@ object ProjectSpec extends ZIOSpecDefault {
   )
 
   private val shortnameTest = suite("ProjectSpec - Shortname")(
-    test("pass an empty value and return an error") {
+    test("pass an empty value and return validation error") {
       assertTrue(
         Shortname.make("") == Validation.fail(ValidationException(ProjectErrorMessages.ShortnameMissing))
       )
     },
-    test("pass an invalid value and return an error") {
-      assertTrue(
-        Shortname.make("invalid:shortname") == Validation.fail(
-          ValidationException(ProjectErrorMessages.ShortnameInvalid("invalid:shortname"))
-        ),
-        Shortname.make("-invalidshortname") == Validation.fail(
-          ValidationException(ProjectErrorMessages.ShortnameInvalid("-invalidshortname"))
-        ),
-        Shortname.make(".invalidshortname") == Validation.fail(
-          ValidationException(ProjectErrorMessages.ShortnameInvalid(".invalidshortname"))
-        ),
-        Shortname.make("invalid/shortname") == Validation.fail(
-          ValidationException(ProjectErrorMessages.ShortnameInvalid("invalid/shortname"))
-        ),
-        Shortname.make("invalid@shortname") == Validation.fail(
-          ValidationException(ProjectErrorMessages.ShortnameInvalid("invalid@shortname"))
-        ),
-        Shortname.make(Some("invalid:shortname")) == Validation.fail(
-          ValidationException(ProjectErrorMessages.ShortnameInvalid("invalid:shortname"))
+    test("pass invalid values and return validation error") {
+      val gen = Gen.fromIterable(
+        Seq(
+          "invalid:shortname",
+          "-invalidshortname",
+          "_invalidshortname",
+          ".invalidshortname",
+          "invalid/shortname",
+          "invalid@shortname",
+          "1invalidshortname",
+          " invalidshortname",
+          "invalid shortname",
+          "a",
+          "ab",
+          "just-21char-shortname"
         )
       )
+      check(gen) { param =>
+        assertTrue(
+          Shortname.make(param) == Validation.fail(ValidationException(ProjectErrorMessages.ShortnameInvalid(param)))
+        )
+      }
     },
-    test("pass a valid value and successfully create value object") {
-      for {
-        shortname1           <- Shortname.make(validShortname1).toZIO
-        optionalShortname1   <- Shortname.make(Option(validShortname1)).toZIO
-        shortnameFromOption1 <- ZIO.fromOption(optionalShortname1)
-        shortname2           <- Shortname.make(validShortname2).toZIO
-        optionalShortname2   <- Shortname.make(Option(validShortname2)).toZIO
-        shortnameFromOption2 <- ZIO.fromOption(optionalShortname2)
-      } yield assertTrue(
-        shortname1.value == validShortname1,
-        shortnameFromOption1.value == validShortname1,
-        shortname2.value == validShortname2,
-        shortnameFromOption2.value == validShortname2
-      ) &&
-        assert(optionalShortname1)(isSome(isSubtype[Shortname](Assertion.anything))) &&
-        assert(optionalShortname2)(isSome(isSubtype[Shortname](Assertion.anything)))
+    test("pass valid values and successfully create value objects") {
+      val gen = Gen.fromIterable(
+        Seq("valid-shortname", "valid_shortname", "validshortname-", "validshortname_", "abc", "a20charLongShortname")
+      )
+      check(gen) { param =>
+        assertTrue(
+          Shortname.make(param).map(_.value) == Validation.succeed(param)
+        ) && assert(Shortname.make(param).toOption)(isSome(isSubtype[Shortname](Assertion.anything)))
+      }
     },
     test("successfully validate passing None") {
       assertTrue(