TC Actions composite name fixes:

* Composite name format validation * Name validation changes * Tests
boris-yakhno · Sep 23, 2024 · eeffeaf · eeffeaf
1 parent dae635d
commit eeffeaf
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 93 deletions.
diff --git a/...rc/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionDescriptor.kt b/...rc/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionDescriptor.kt
@@ -1,6 +1,5 @@
 package com.jetbrains.plugin.structure.teamcity.action
 
-import com.fasterxml.jackson.annotation.JsonIgnore
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties
 import com.fasterxml.jackson.annotation.JsonProperty
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionDescription
@@ -11,7 +10,7 @@ import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionI
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputRequired
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputType
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputs
-import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionName
+import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionCompositeName
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirementType
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirementValue
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirements
@@ -27,7 +26,7 @@ import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionV
 data class TeamCityActionDescriptor(
   @JsonProperty(ActionSpecVersion.NAME)
   val specVersion: String? = null,
-  @JsonProperty(ActionName.NAME)
+  @JsonProperty(ActionCompositeName.NAME)
   val name: String? = null,
   @JsonProperty(ActionVersion.NAME)
   val version: String? = null,
@@ -39,12 +38,7 @@ data class TeamCityActionDescriptor(
   val requirements: List<Map<String, ActionRequirementDescriptor>> = emptyList(),
   @JsonProperty(ActionSteps.NAME)
   val steps: List<ActionStepDescriptor> = emptyList(),
-) {
-  @JsonIgnore
-  fun getNamespace() = name?.substringBefore(ActionName.NAMESPACE_SPLIT_SYMBOL)
-  @JsonIgnore
-  fun getId() = name?.substringAfter(ActionName.NAMESPACE_SPLIT_SYMBOL)
-}
+)
 
 @JsonIgnoreProperties(ignoreUnknown = true)
 data class ActionInputDescriptor(

diff --git a/...main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionPluginManager.kt b/...main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionPluginManager.kt
@@ -99,7 +99,7 @@ private constructor(private val extractDirectory: Path) : PluginManager<TeamCity
         pluginVersion = this.version!!,
         specVersion = this.specVersion!!,
         yamlFile = PluginFile(yamlPath.fileName.toString(), yamlPath.readBytes()),
-        namespace = getNamespace()!!
+        namespace = TeamCityActionSpec.ActionCompositeName.getNamespace(this.name)!!
       )
     }
     return PluginCreationSuccess(plugin, validationResult)

diff --git a/.../src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionProblems.kt b/.../src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionProblems.kt
@@ -46,7 +46,7 @@ class TooShortValueProblem(
   minAllowedLength: Int,
 ) : InvalidPropertyProblem() {
   override val message =
-    "The property <$propertyName> ($propertyDescription) is shorter than $minAllowedLength characters. " +
+    "The property <$propertyName> ($propertyDescription) should not be shorter than $minAllowedLength characters. " +
       "The current number of characters is $currentLength."
 }
 

diff --git a/...ions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionSpec.kt b/...ions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/TeamCityActionSpec.kt
@@ -6,14 +6,15 @@ object TeamCityActionSpec {
     const val DESCRIPTION = "the version of action specification"
   }
 
-  object ActionName {
+  object ActionCompositeName {
     const val NAME = "name"
-    const val DESCRIPTION = "the composite action name that consists of two parts: the `namespace` and the `ID` divided by the `/` symbol"
+    const val DESCRIPTION = "the composite action name in the 'namespace/name' format"
 
-    const val NAMESPACE_SPLIT_SYMBOL = "/"
+    // Regular expression pattern for the action's composite name – the namespace and the name separated by '/'
+    private const val COMPOSITE_NAME_PATTERN = "^([^/]+)/([^/]+)$"
 
     /**
-     * Regular expression pattern for both action namespace and id.
+     * Regular expression pattern for both the action's namespace and the action's name.
      *
      * The pattern enforces the following rules for both namespace and id:
      * - cannot be empty.
@@ -22,21 +23,34 @@ object TeamCityActionSpec {
      * - cannot contain several consecutive dashes or underscores.
      */
     private const val ID_AND_NAMESPACE_PATTERN = "^[a-zA-Z0-9]+([_-][a-zA-Z0-9]+)*\$"
+    val compositeNameRegex: Regex = Regex(COMPOSITE_NAME_PATTERN)
     val idAndNamespaceRegex: Regex = Regex(ID_AND_NAMESPACE_PATTERN)
 
     object Namespace {
       const val NAME = "namespace"
-      const val DESCRIPTION = "the first part of the composite `${ActionName.NAME}` field"
+      const val DESCRIPTION = "the first part of the composite `${ActionCompositeName.NAME}` field"
       const val MIN_LENGTH = 5
       const val MAX_LENGTH = 30
     }
 
-    object ID {
-      const val NAME = "id"
-      const val DESCRIPTION = "the second part of the composite `${ActionName.NAME}` field"
+    object Name {
+      const val NAME = "name"
+      const val DESCRIPTION = "the second part of the composite `${ActionCompositeName.NAME}` field"
       const val MIN_LENGTH = 5
       const val MAX_LENGTH = 30
     }
+
+    fun getNamespace(actionName: String?): String? {
+      if (actionName == null) return null
+      val matchResult = compositeNameRegex.matchEntire(actionName)
+      return matchResult?.groupValues?.get(1)
+    }
+
+    fun getNameInNamespace(actionName: String?): String? {
+      if (actionName == null) return null
+      val matchResult = compositeNameRegex.matchEntire(actionName)
+      return matchResult?.groupValues?.get(2)
+    }
   }
 
   object ActionVersion {

diff --git a/...mcity-actions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/Validator.kt b/...mcity-actions/src/main/kotlin/com/jetbrains/plugin/structure/teamcity/action/Validator.kt
@@ -11,7 +11,7 @@ import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionI
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputOptions
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputRequired
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputType
-import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionName
+import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionCompositeName
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirementName
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirementValue
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionSpecVersion
@@ -24,29 +24,9 @@ import com.vdurmont.semver4j.Semver
 import com.vdurmont.semver4j.SemverException
 
 internal fun validateTeamCityAction(descriptor: TeamCityActionDescriptor) = sequence {
-  validateSpecVersion(descriptor.specVersion, ActionSpecVersion.NAME, ActionSpecVersion.DESCRIPTION)
-
-  validateExists(descriptor.name, ActionName.NAME, ActionName.DESCRIPTION)
-
-  validateExists(descriptor.getNamespace(), ActionName.Namespace.NAME, ActionName.Namespace.DESCRIPTION)
-  validateNotEmptyIfExists(descriptor.getNamespace(), ActionName.Namespace.NAME, ActionName.Namespace.DESCRIPTION)
-  validateMinLength(descriptor.getNamespace(), ActionName.Namespace.NAME, ActionName.Namespace.DESCRIPTION, ActionName.Namespace.MIN_LENGTH)
-  validateMaxLength(descriptor.getNamespace(), ActionName.Namespace.NAME, ActionName.Namespace.DESCRIPTION, ActionName.Namespace.MAX_LENGTH)
-  validateMatchesRegexIfExistsAndNotEmpty(
-    descriptor.getNamespace(), ActionName.idAndNamespaceRegex, ActionName.Namespace.NAME, ActionName.Namespace.DESCRIPTION,
-    "should only contain latin letters, numbers, dashes and underscores. " +
-        "The property cannot start or end with a dash or underscore, and cannot contain several consecutive dashes and underscores."
-  )
+  validateName(descriptor.name)
 
-  validateExists(descriptor.getId(), ActionName.ID.NAME, ActionName.ID.DESCRIPTION)
-  validateNotEmptyIfExists(descriptor.getId(), ActionName.ID.NAME, ActionName.ID.DESCRIPTION)
-  validateMinLength(descriptor.getId(), ActionName.ID.NAME, ActionName.ID.DESCRIPTION, ActionName.ID.MIN_LENGTH)
-  validateMaxLength(descriptor.getId(), ActionName.ID.NAME, ActionName.ID.DESCRIPTION, ActionName.ID.MAX_LENGTH)
-  validateMatchesRegexIfExistsAndNotEmpty(
-    descriptor.getId(), ActionName.idAndNamespaceRegex, ActionName.Namespace.NAME, ActionName.Namespace.DESCRIPTION,
-    "should only contain latin letters, numbers, dashes and underscores. " +
-      "The property cannot start or end with a dash or underscore, and cannot contain several consecutive dashes and underscores."
-  )
+  validateSpecVersion(descriptor.specVersion, ActionSpecVersion.NAME, ActionSpecVersion.DESCRIPTION)
 
   validateExistsAndNotEmpty(descriptor.version, ActionVersion.NAME, ActionVersion.DESCRIPTION)
   validateSemver(descriptor.version, ActionVersion.NAME)
@@ -65,6 +45,37 @@ internal fun validateTeamCityAction(descriptor: TeamCityActionDescriptor) = sequ
   for (step in descriptor.steps) validateActionStep(step)
 }.toList()
 
+private suspend fun SequenceScope<PluginProblem>.validateName(name: String?) {
+  validateExists(name, ActionCompositeName.NAME, ActionCompositeName.DESCRIPTION)
+  validateNotEmptyIfExists(name, ActionCompositeName.NAME, ActionCompositeName.DESCRIPTION)
+  validateMatchesRegexIfExistsAndNotEmpty(
+    name, ActionCompositeName.compositeNameRegex, ActionCompositeName.NAME, ActionCompositeName.DESCRIPTION,
+    "should consist of namespace and name parts. Both parts should only contain latin letters, numbers, dashes and underscores."
+  )
+
+  val namespace = ActionCompositeName.getNamespace(name)
+  if (namespace != null) {
+    validateMinLength(namespace, ActionCompositeName.Namespace.NAME, ActionCompositeName.Namespace.DESCRIPTION, ActionCompositeName.Namespace.MIN_LENGTH)
+    validateMaxLength(namespace, ActionCompositeName.Namespace.NAME, ActionCompositeName.Namespace.DESCRIPTION, ActionCompositeName.Namespace.MAX_LENGTH)
+    validateMatchesRegexIfExistsAndNotEmpty(
+      namespace, ActionCompositeName.idAndNamespaceRegex, ActionCompositeName.Namespace.NAME, ActionCompositeName.Namespace.DESCRIPTION,
+      "should only contain latin letters, numbers, dashes and underscores. " +
+              "The property cannot start or end with a dash or underscore, and cannot contain several consecutive dashes and underscores."
+    )
+  }
+
+  val nameInNamespace = ActionCompositeName.getNameInNamespace(name)
+  if (nameInNamespace != null) {
+    validateMinLength(nameInNamespace, ActionCompositeName.Name.NAME, ActionCompositeName.Name.DESCRIPTION, ActionCompositeName.Name.MIN_LENGTH)
+    validateMaxLength(nameInNamespace, ActionCompositeName.Name.NAME, ActionCompositeName.Name.DESCRIPTION, ActionCompositeName.Name.MAX_LENGTH)
+    validateMatchesRegexIfExistsAndNotEmpty(
+      nameInNamespace, ActionCompositeName.idAndNamespaceRegex, ActionCompositeName.Name.NAME, ActionCompositeName.Name.DESCRIPTION,
+      "should only contain latin letters, numbers, dashes and underscores. " +
+              "The property cannot start or end with a dash or underscore, and cannot contain several consecutive dashes and underscores."
+    )
+  }
+}
+
 private suspend fun SequenceScope<PluginProblem>.validateActionInput(input: Map<String, ActionInputDescriptor>) {
   if (input.size != 1) {
     yield(InvalidPropertyValueProblem("Wrong action input format. The input should consist of a name and body."))

diff --git a/...src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseInvalidActionTests.kt b/...src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseInvalidActionTests.kt
@@ -40,9 +40,7 @@ class ParseInvalidActionTests(
         )
       ),
       listOf(
-        MissingValueProblem("name", "the composite action name that consists of two parts: the `namespace` and the `ID` divided by the `/` symbol"),
-        MissingValueProblem("namespace", "the first part of the composite `name` field"),
-        MissingValueProblem("id", "the second part of the composite `name` field"),
+        MissingValueProblem("name", "the composite action name in the 'namespace/name' format"),
         MissingValueProblem("version", "action version"),
         EmptyValueProblem("description", "action description"),
         EmptyCollectionProblem("steps", "action steps"),
@@ -51,19 +49,36 @@ class ParseInvalidActionTests(
   }
 
   @Test
-  fun `action without name`() {
+  fun `action without a composite name`() {
     assertProblematicPlugin(
       prepareActionYaml(someAction.copy(name = null)),
       listOf(
-        MissingValueProblem("name", "the composite action name that consists of two parts: the `namespace` and the `ID` divided by the `/` symbol"),
-        MissingValueProblem("namespace", "the first part of the composite `name` field"),
-        MissingValueProblem("id", "the second part of the composite `name` field"),
+        MissingValueProblem("name", "the composite action name in the 'namespace/name' format")
       ),
     )
   }
 
   @Test
-  fun `action with non-empty invalid namespace`() {
+  fun `action with the composite name in an invalid format`() {
+    val invalidActionNamesProvider = arrayOf(
+      "aaaaabbbbb", "/aaaaabbbbb", "aaaaabbbbb/", "/", "aaaaa/bbbbb/ccccc", "aaaaa//bbbbb", "aaaaa\\bbbbb"
+    )
+    invalidActionNamesProvider.forEach { actionName ->
+      Files.walk(temporaryFolder.root).filter { it.isFile }.forEach { Files.delete(it) }
+      assertProblematicPlugin(
+        prepareActionYaml(someAction.copy(name = actionName)),
+        listOf(
+          InvalidPropertyValueProblem(
+            "The property <name> (the composite action name in the 'namespace/name' format) " +
+                    "should consist of namespace and name parts. Both parts should only contain latin letters, numbers, dashes and underscores."
+          )
+        ),
+      )
+    }
+  }
+
+  @Test
+  fun `action with an invalid namespace`() {
     val invalidActionNamesProvider = arrayOf(
       "-aaaaa/aaaaaa", "_aaaaa/aaaaaa", "aaaaaa-/aaaaa", "aaaaa_/aaaaa", "a--aa/aaaaa", "a__aa/aaaaa", "a+aaa/aaaaa", "абв23/aaaaa",
     )
@@ -83,7 +98,7 @@ class ParseInvalidActionTests(
   }
 
   @Test
-  fun `action with non-empty invalid id`() {
+  fun `action with an invalid name`() {
     val invalidActionNamesProvider = arrayOf(
       "aaaaaa/-aaaaa", "aaaaaa/_aaaaa", "aaaaaa/aaaaaa-", "aaaaaa/aaaaa_", "aaaaaa/aa--a", "aaaaaa/aa__a", "aaaaaa/aa+aa", "aaaaaa/абв23"
     )
@@ -93,7 +108,7 @@ class ParseInvalidActionTests(
         prepareActionYaml(someAction.copy(name = actionName)),
         listOf(
           InvalidPropertyValueProblem(
-            "The property <namespace> (the first part of the composite `name` field) should only contain latin letters, "
+            "The property <name> (the second part of the composite `name` field) should only contain latin letters, "
               + "numbers, dashes and underscores. The property cannot start or end with a dash or underscore, and "
               + "cannot contain several consecutive dashes and underscores."
           )
@@ -103,19 +118,7 @@ class ParseInvalidActionTests(
   }
 
   @Test
-  fun `action with too short or empty id or namespace`() {
-    assertProblematicPlugin(
-      prepareActionYaml(someAction.copy(name = "/${randomAlphanumeric(10)}")),
-      listOf(
-        EmptyValueProblem("namespace", "the first part of the composite `name` field"),
-        TooShortValueProblem(
-          propertyName = "namespace",
-          propertyDescription = "the first part of the composite `name` field",
-          currentLength = 0,
-          minAllowedLength = 5
-        )
-      )
-    )
+  fun `action with a namespace that is too short`() {
     assertProblematicPlugin(
       prepareActionYaml(someAction.copy(name = "aaaa/${randomAlphanumeric(10)}")),
       listOf(TooShortValueProblem(
@@ -125,49 +128,44 @@ class ParseInvalidActionTests(
         minAllowedLength = 5
       )),
     )
+  }
 
+  @Test
+  fun `action with a namespace that is too long`() {
     assertProblematicPlugin(
-      prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(10)}/")),
-      listOf(
-        EmptyValueProblem("id", "the second part of the composite `name` field"),
-        TooShortValueProblem(
-          propertyName = "id",
-          propertyDescription = "the second part of the composite `name` field",
-          currentLength = 0,
-          minAllowedLength = 5
-        )
-      )
+      prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(31)}/${randomAlphanumeric(10)}")),
+      listOf(TooLongValueProblem(
+        propertyName = "namespace",
+        propertyDescription = "the first part of the composite `name` field",
+        currentLength = 31,
+        maxAllowedLength = 30
+      )),
     )
+  }
+
+  @Test
+  fun `action with a name that is too short`() {
     assertProblematicPlugin(
       prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(10)}/aaaa")),
       listOf(TooShortValueProblem(
-        propertyName = "id",
+        propertyName = "name",
         propertyDescription = "the second part of the composite `name` field",
         currentLength = 4,
         minAllowedLength = 5
-      ))
+      )),
     )
   }
 
   @Test
-  fun `action with too long id or namespace`() {
-    assertProblematicPlugin(
-      prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(31)}/${randomAlphanumeric(10)}")),
-      listOf(TooLongValueProblem(
-        propertyName = "namespace",
-        propertyDescription = "the first part of the composite `name` field",
-        currentLength = 31,
-        maxAllowedLength = 30
-      )),
-    )
+  fun `action with a name that is too long`() {
     assertProblematicPlugin(
       prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(10)}/${randomAlphanumeric(31)}")),
       listOf(TooLongValueProblem(
-        propertyName = "id",
+        propertyName = "name",
         propertyDescription = "the second part of the composite `name` field",
         currentLength = 31,
         maxAllowedLength = 30
-      ))
+      )),
     )
   }
 

diff --git a/...rc/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseValidFullActionTest.kt b/...rc/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/ParseValidFullActionTest.kt
@@ -19,7 +19,7 @@ class ParseValidFullActionTest(
     """
     ---
     spec-version: 1.0.0
-    name: simple-action
+    name: namespace/simple-action
     version: 1.2.3
     description: this is a simple action
     inputs:
@@ -118,7 +118,8 @@ class ParseValidFullActionTest(
       assertEquals(fileName, this.yamlFile.fileName)
       assertArrayEquals(actionYaml.toByteArray(), this.yamlFile.content)
       assertEquals("1.0.0", this.specVersion)
-      assertEquals("simple-action", this.pluginName)
+      assertEquals("namespace/simple-action", this.pluginName)
+      assertEquals("namespace", this.namespace)
       assertEquals("1.2.3", this.pluginVersion)
       assertEquals("this is a simple action", this.description)
     }

diff --git a/...tructure/tests/src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/TestData.kt b/...tructure/tests/src/test/kotlin/com/jetbrains/plugin/structure/teamcity/action/TestData.kt
@@ -9,7 +9,7 @@ import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionI
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputRequired
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputType
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionInputs
-import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionName
+import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionCompositeName
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirementType
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirementValue
 import com.jetbrains.plugin.structure.teamcity.action.TeamCityActionSpec.ActionRequirements
@@ -72,7 +72,7 @@ object Steps {
 data class TeamCityActionBuilder(
   @JsonProperty(ActionSpecVersion.NAME)
   var specVersion: String? = null,
-  @JsonProperty(ActionName.NAME)
+  @JsonProperty(ActionCompositeName.NAME)
   var name: String? = null,
   @JsonProperty(ActionVersion.NAME)
   var version: String? = null,