Skip to content

Commit

Permalink
TC actions name as composite id namespace/id (#1159)
Browse files Browse the repository at this point in the history
* TC actions name as composite id `namespace/id`

* TC Actions composite name fixes:
* Composite name format validation
* Name validation changes
* Tests

---------

Co-authored-by: Boris Yakhno <boris.yakhno@jetbrains.com>
  • Loading branch information
kesarevs and boris-yakhno authored Sep 25, 2024
1 parent 7a73323 commit 2dbc2e2
Show file tree
Hide file tree
Showing 10 changed files with 216 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,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
Expand All @@ -26,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ data class TeamCityActionPlugin(
override val thirdPartyDependencies: List<ThirdPartyDependency> = emptyList(),
override val url: String? = null,
override val changeNotes: String? = null,
val namespace: String,
val specVersion: String,
val yamlFile: PluginFile
) : Plugin
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,13 @@ private constructor(private val extractDirectory: Path) : PluginManager<TeamCity
val plugin = with(descriptor) {
TeamCityActionPlugin(
// All the fields are expected to be non-null due to the validations above
pluginId = this.name!!, // TODO: It is a temporary way of the pluginID creation, must be changed
pluginId = this.name!!, // composite id
pluginName = this.name,
description = this.description!!,
pluginVersion = this.version!!,
specVersion = this.specVersion!!,
yamlFile = PluginFile(yamlPath.fileName.toString(), yamlPath.readBytes())
yamlFile = PluginFile(yamlPath.fileName.toString(), yamlPath.readBytes()),
namespace = TeamCityActionSpec.ActionCompositeName.getNamespace(this.name)!!
)
}
return PluginCreationSuccess(plugin, validationResult)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,15 @@ class TooLongValueProblem(
"The current number of characters is $currentLength."
}

class TooShortValueProblem(
propertyName: String,
propertyDescription: String,
currentLength: Int,
minAllowedLength: Int,
) : InvalidPropertyProblem() {
override val message =
"The property <$propertyName> ($propertyDescription) should not be shorter than $minAllowedLength characters. " +
"The current number of characters is $currentLength."
}

class PropertiesCombinationProblem(override val message: String) : InvalidPropertyProblem()
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,51 @@ object TeamCityActionSpec {
const val DESCRIPTION = "the version of action specification"
}

object ActionName {
object ActionCompositeName {
const val NAME = "name"
const val DESCRIPTION = "action name"
const val MAX_LENGTH = 30
const val DESCRIPTION = "the composite action name in the 'namespace/name' format"

// 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 the action name.
* Regular expression pattern for both the action's namespace and the action's name.
*
* The pattern enforces the following rules:
* - Name cannot be empty.
* – Name can only contain latin letters, dashes and underscores.
* - Name cannot start or end with a dash or underscore.
* - Name cannot contain several consecutive dashes or underscores.
* The pattern enforces the following rules for both namespace and id:
* - cannot be empty.
* – can only contain latin letters, dashes and underscores.
* - cannot start or end with a dash or underscore.
* - cannot contain several consecutive dashes or underscores.
*/
val nameRegex: Regex = Regex("^[a-zA-Z0-9]+([_-][a-zA-Z0-9]+)*\$")
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 `${ActionCompositeName.NAME}` field"
const val MIN_LENGTH = 5
const val MAX_LENGTH = 30
}

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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -24,16 +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)
validateName(descriptor.name)

validateExists(descriptor.name, ActionName.NAME, ActionName.DESCRIPTION)
validateNotEmptyIfExists(descriptor.name, ActionName.NAME, ActionName.DESCRIPTION)
validateMaxLength(descriptor.name, ActionName.NAME, ActionName.DESCRIPTION, ActionName.MAX_LENGTH)
validateMatchesRegexIfExistsAndNotEmpty(
descriptor.name, ActionName.nameRegex, ActionName.NAME, ActionName.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)
Expand All @@ -52,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."))
Expand Down Expand Up @@ -230,6 +254,17 @@ private suspend fun SequenceScope<PluginProblem>.validateExistsAndNotEmpty(
}
}

private suspend fun SequenceScope<PluginProblem>.validateMinLength(
propertyValue: String?,
propertyName: String,
propertyDescription: String,
minAllowedLength: Int,
) {
if (propertyValue != null && propertyValue.length < minAllowedLength) {
yield(TooShortValueProblem(propertyName, propertyDescription, propertyValue.length, minAllowedLength))
}
}

private suspend fun SequenceScope<PluginProblem>.validateMaxLength(
propertyValue: String?,
propertyName: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ParseInvalidActionTests(
)
),
listOf(
MissingValueProblem("name", "action name"),
MissingValueProblem("name", "the composite action name in the 'namespace/name' format"),
MissingValueProblem("version", "action version"),
EmptyValueProblem("description", "action description"),
EmptyCollectionProblem("steps", "action steps"),
Expand All @@ -49,52 +49,123 @@ class ParseInvalidActionTests(
}

@Test
fun `action without name`() {
fun `action without a composite name`() {
assertProblematicPlugin(
prepareActionYaml(someAction.copy(name = null)),
listOf(MissingValueProblem("name", "action name")),
listOf(
MissingValueProblem("name", "the composite action name in the 'namespace/name' format")
),
)
}

@Test
fun `action with non-null but empty name`() {
assertProblematicPlugin(
prepareActionYaml(someAction.copy(name = "")),
listOf(EmptyValueProblem("name", "action name")),
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",
)
invalidActionNamesProvider.forEach { actionName ->
Files.walk(temporaryFolder.root).filter { it.isFile }.forEach { Files.delete(it) }
assertProblematicPlugin(
prepareActionYaml(someAction.copy(name = actionName)),
listOf(
InvalidPropertyValueProblem(
"The property <namespace> (the first 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."
)
),
)
}
}

@Test
fun `action with non-empty invalid name`() {
fun `action with an invalid name`() {
val invalidActionNamesProvider = arrayOf(
"-a",
"_a",
"a-",
"a_",
"a--a",
"a__a",
"a+a",
"абв"
"aaaaaa/-aaaaa", "aaaaaa/_aaaaa", "aaaaaa/aaaaaa-", "aaaaaa/aaaaa_", "aaaaaa/aa--a", "aaaaaa/aa__a", "aaaaaa/aa+aa", "aaaaaa/абв23"
)
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> (action name) 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."
"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."
)
),
)
}
}

@Test
fun `action with too long name`() {
fun `action with a namespace that is too short`() {
assertProblematicPlugin(
prepareActionYaml(someAction.copy(name = "aaaa/${randomAlphanumeric(10)}")),
listOf(TooShortValueProblem(
propertyName = "namespace",
propertyDescription = "the first part of the composite `name` field",
currentLength = 4,
minAllowedLength = 5
)),
)
}

@Test
fun `action with a namespace that is too long`() {
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
)),
)
}

@Test
fun `action with a name that is too short`() {
assertProblematicPlugin(
prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(10)}/aaaa")),
listOf(TooShortValueProblem(
propertyName = "name",
propertyDescription = "the second part of the composite `name` field",
currentLength = 4,
minAllowedLength = 5
)),
)
}

@Test
fun `action with a name that is too long`() {
assertProblematicPlugin(
prepareActionYaml(someAction.copy(name = randomAlphanumeric(31))),
listOf(TooLongValueProblem("name", "action name", 31, 30)),
prepareActionYaml(someAction.copy(name = "${randomAlphanumeric(10)}/${randomAlphanumeric(31)}")),
listOf(TooLongValueProblem(
propertyName = "name",
propertyDescription = "the second part of the composite `name` field",
currentLength = 31,
maxAllowedLength = 30
)),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class ParseValidActionTests(

@Test
fun `parse action with valid name`() {
val validActionNamesProvider = arrayOf("a", "aa", "a-a_a")
val validActionNamesProvider = arrayOf("aaaaa/aaaaa", "aaaaa/a-a_a", "a-a_a/aaaaa", "${randomAlphanumeric(30)}/${randomAlphanumeric(30)}")
validActionNamesProvider.forEach { actionName ->
Files.walk(temporaryFolder.root).filter { it.isFile }.forEach { Files.delete(it) }
val result = createPluginSuccessfully(prepareActionYaml(someAction.copy(name = actionName)))
Expand All @@ -35,6 +35,21 @@ class ParseValidActionTests(
}
}

@Test
fun `parse action id and namespace`() {
val expectedNamespace = "jetbrains"
val expectedActionId = "test_action"
val expectedActionName = "$expectedNamespace/$expectedActionId"

Files.walk(temporaryFolder.root).filter { it.isFile }.forEach { Files.delete(it) }
val result = createPluginSuccessfully(prepareActionYaml(someAction.copy(name = expectedActionName)))
with(result) {
assertEquals(expectedActionName, this.plugin.pluginName)
assertEquals(expectedActionName, this.plugin.pluginId)
assertEquals(expectedNamespace, this.plugin.namespace)
}
}

@Test
fun `parse action with runner-based step`() {
val step = someWithStep.copy(with = "runner/runnerName")
Expand Down
Loading

0 comments on commit 2dbc2e2

Please sign in to comment.