Skip to content

Commit

Permalink
refactor: unify content of ActionCoords name (#1646)
Browse files Browse the repository at this point in the history
Before these changes, `ActionCoords#name` sometimes had the actual
action coordinates, so for example `some-name__some-sub`, sometimes it
had the replaced `some-name/some-sub`.
This is not good, as you never know what you get and also led to certain
bugs where wrong things are written to metadata files for example.
This changes unifies the situation, so that the `name` only contains the
repository name and the `path` is in a separate property with slashes.
All other manipulation is then done where needed.
  • Loading branch information
Vampire authored Oct 27, 2024
1 parent a5b842c commit 0343909
Show file tree
Hide file tree
Showing 18 changed files with 188 additions and 127 deletions.
11 changes: 7 additions & 4 deletions action-binding-generator/api/action-binding-generator.api
Original file line number Diff line number Diff line change
@@ -1,21 +1,24 @@
public final class io/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords {
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
public synthetic fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun component1 ()Ljava/lang/String;
public final fun component2 ()Ljava/lang/String;
public final fun component3 ()Ljava/lang/String;
public final fun copy (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;
public static synthetic fun copy$default (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;
public final fun component4 ()Ljava/lang/String;
public final fun copy (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;
public static synthetic fun copy$default (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;
public fun equals (Ljava/lang/Object;)Z
public final fun getName ()Ljava/lang/String;
public final fun getOwner ()Ljava/lang/String;
public final fun getPath ()Ljava/lang/String;
public final fun getVersion ()Ljava/lang/String;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

public final class io/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoordsKt {
public static final fun getFullName (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;)Ljava/lang/String;
public static final fun getPrettyPrint (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;)Ljava/lang/String;
public static final fun getRepoName (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;)Ljava/lang/String;
public static final fun getSubName (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;)Ljava/lang/String;
public static final fun isTopLevel (Lio/github/typesafegithub/workflows/actionbindinggenerator/domain/ActionCoords;)Z
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,39 @@ public data class ActionCoords(
val owner: String,
val name: String,
val version: String,
val path: String? = null,
)

/**
* A top-level action is an action with its `action.y(a)ml` file in the repository root, as opposed to actions stored
* in subdirectories.
*/
public val ActionCoords.isTopLevel: Boolean get() = "/" !in name
public val ActionCoords.isTopLevel: Boolean get() = path == null

public val ActionCoords.prettyPrint: String get() = "$owner/$name@$version"
public val ActionCoords.prettyPrint: String get() = "$owner/$fullName@$version"

/**
* For most actions, it's the same as [ActionCoords.name].
* For actions that aren't executed from the root of the repo, it returns the repo name.
* For most actions, it's empty.
* For actions that aren't executed from the root of the repo, it returns the path relative to the repo root where the
* action lives, starting with a slash.
*/
public val ActionCoords.repoName: String get() =
name.substringBefore("/")
public val ActionCoords.subName: String get() = path?.let { "/$path" } ?: ""

/**
* For most actions, it's empty.
* For actions that aren't executed from the root of the repo, it returns the path relative to the repo root where the
* For most actions, it's equal to [ActionCoords.name].
* For actions that aren't executed from the root of the repo, it returns the path starting with the repo root where the
* action lives.
*/
public val ActionCoords.subName: String get() =
if (isTopLevel) "" else "/${name.substringAfter("/")}"
public val ActionCoords.fullName: String get() = "$name$subName"

internal fun String.toActionCoords(): ActionCoords {
val (ownerAndName, version) = this.split('@')
val (owner, name) = ownerAndName.split('/', limit = 2)
val (coordinates, version) = this.split('@')
val coordinateParts = coordinates.split('/')
val (owner, name) = coordinateParts
return ActionCoords(
owner = owner,
name = name,
version = version,
path = coordinateParts.drop(2).joinToString("/").takeUnless { it.isBlank() },
)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.github.typesafegithub.workflows.actionbindinggenerator.generation

import io.github.typesafegithub.workflows.actionbindinggenerator.domain.ActionCoords
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.fullName
import io.github.typesafegithub.workflows.actionbindinggenerator.utils.toPascalCase

internal fun ActionCoords.buildActionClassName(): String = this.name.toPascalCase()
internal fun ActionCoords.buildActionClassName(): String = this.fullName.toPascalCase()
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import com.squareup.kotlinpoet.buildCodeBlock
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.ActionCoords
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.MetadataRevision
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.TypingActualSource
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.fullName
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.isTopLevel
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.subName
import io.github.typesafegithub.workflows.actionbindinggenerator.generation.Properties.CUSTOM_INPUTS
import io.github.typesafegithub.workflows.actionbindinggenerator.generation.Properties.CUSTOM_VERSION
import io.github.typesafegithub.workflows.actionbindinggenerator.metadata.Input
Expand Down Expand Up @@ -411,7 +414,7 @@ private fun TypeSpec.Builder.inheritsFromRegularAction(
return this
.superclass(superclass)
.addSuperclassConstructorParameter("%S", coords.owner)
.addSuperclassConstructorParameter("%S", coords.name)
.addSuperclassConstructorParameter("%S", coords.fullName)
.addSuperclassConstructorParameter("_customVersion ?: %S", coords.version)
}

Expand Down Expand Up @@ -614,9 +617,7 @@ private fun actionKdoc(
|
|${metadata.description.escapedForComments.removeTrailingWhitespacesForEachLine()}
|
|[Action on GitHub](https://github.com/${coords.owner}/${coords.name.substringBefore(
'/',
)}${if ("/" in coords.name) "/tree/${coords.version}/${coords.name.substringAfter('/')}" else ""})
|[Action on GitHub](https://github.com/${coords.owner}/${coords.name}${if (coords.isTopLevel) "" else "/tree/${coords.version}${coords.subName}"})
""".trimMargin()

private fun Map<String, Typing>?.getInputTyping(key: String) = this?.get(key) ?: StringTyping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.github.typesafegithub.workflows.actionbindinggenerator.domain.ActionCo
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.CommitHash
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.MetadataRevision
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.NewestForVersion
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.subName
import kotlinx.serialization.Serializable
import kotlinx.serialization.decodeFromString
import java.io.IOException
Expand Down Expand Up @@ -35,15 +36,9 @@ public data class Output(
val description: String = "",
)

private fun ActionCoords.actionYmlUrl(gitRef: String) =
"https://raw.githubusercontent.com/$owner/${name.substringBefore(
'/',
)}/$gitRef/${if ("/" in name) "${name.substringAfter('/')}/" else ""}action.yml"
private fun ActionCoords.actionYmlUrl(gitRef: String) = "https://raw.githubusercontent.com/$owner/$name/$gitRef$subName/action.yml"

private fun ActionCoords.actionYamlUrl(gitRef: String) =
"https://raw.githubusercontent.com/$owner/${name.substringBefore(
'/',
)}/$gitRef/${if ("/" in name) "${name.substringAfter('/')}/" else ""}action.yaml"
private fun ActionCoords.actionYamlUrl(gitRef: String) = "https://raw.githubusercontent.com/$owner/$name/$gitRef$subName/action.yaml"

internal val ActionCoords.gitHubUrl: String get() = "https://github.com/$owner/$name"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import io.github.typesafegithub.workflows.actionbindinggenerator.domain.NewestFo
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.TypingActualSource
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.TypingActualSource.ACTION
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.TypingActualSource.TYPING_CATALOG
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.repoName
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.subName
import io.github.typesafegithub.workflows.actionbindinggenerator.metadata.fetchUri
import io.github.typesafegithub.workflows.actionbindinggenerator.utils.toPascalCase
Expand All @@ -29,18 +28,18 @@ internal fun ActionCoords.provideTypes(
?: Pair(emptyMap(), null)

private fun ActionCoords.actionTypesYmlUrl(gitRef: String) =
"https://raw.githubusercontent.com/$owner/$repoName/$gitRef$subName/action-types.yml"
"https://raw.githubusercontent.com/$owner/$name/$gitRef$subName/action-types.yml"

private fun ActionCoords.actionTypesFromCatalog() =
"https://raw.githubusercontent.com/typesafegithub/github-actions-typing-catalog/" +
"main/typings/$owner/$repoName/$version$subName/action-types.yml"
"main/typings/$owner/$name/$version$subName/action-types.yml"

private fun ActionCoords.catalogMetadata() =
"https://raw.githubusercontent.com/typesafegithub/github-actions-typing-catalog/" +
"main/typings/$owner/$repoName/metadata.yml"
"main/typings/$owner/$name/metadata.yml"

private fun ActionCoords.actionTypesYamlUrl(gitRef: String) =
"https://raw.githubusercontent.com/$owner/$repoName/$gitRef$subName/action-types.yaml"
"https://raw.githubusercontent.com/$owner/$name/$gitRef$subName/action-types.yaml"

private fun ActionCoords.fetchTypingMetadata(
metadataRevision: MetadataRevision,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// This file was generated using action-binding-generator. Don't change it by hand, otherwise your
// changes will be overwritten with the next binding code regeneration.
// See https://github.com/typesafegithub/github-workflows-kt for more info.
@file:Suppress(
"DataClassPrivateConstructor",
"UNUSED_PARAMETER",
)

package io.github.typesafegithub.workflows.actions.johnsmith

import io.github.typesafegithub.workflows.domain.actions.Action
import io.github.typesafegithub.workflows.domain.actions.RegularAction
import java.util.LinkedHashMap
import kotlin.ExposedCopyVisibility
import kotlin.String
import kotlin.Suppress
import kotlin.Unit
import kotlin.collections.Map

/**
* Action: Action With No Inputs
*
* Description
*
* [Action on GitHub](https://github.com/john-smith/action-with/tree/v3/sub/action)
*
* @param _customInputs Type-unsafe map where you can put any inputs that are not yet supported by the binding
* @param _customVersion Allows overriding action's version, for example to use a specific minor version, or a newer version that the binding doesn't yet know about
*/
@ExposedCopyVisibility
public data class ActionWithSubAction private constructor(
/**
* Type-unsafe map where you can put any inputs that are not yet supported by the binding
*/
public val _customInputs: Map<String, String> = mapOf(),
/**
* Allows overriding action's version, for example to use a specific minor version, or a newer version that the binding doesn't yet know about
*/
public val _customVersion: String? = null,
) : RegularAction<Action.Outputs>("john-smith", "action-with/sub/action", _customVersion ?: "v3") {
public constructor(
vararg pleaseUseNamedArguments: Unit,
_customInputs: Map<String, String> = mapOf(),
_customVersion: String? = null,
) : this(_customInputs = _customInputs, _customVersion = _customVersion)

@Suppress("SpreadOperator")
override fun toYamlArguments(): LinkedHashMap<String, String> = LinkedHashMap(_customInputs)

override fun buildOutputObject(stepId: String): Action.Outputs = Outputs(stepId)
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class ClassNamingTest :
context("buildActionClassName") {
listOf(
ActionCoords("irrelevant", "some-action-name", "v2") to "SomeActionName",
ActionCoords("irrelevant", "some-action-name/subaction", "v2") to "SomeActionNameSubaction",
ActionCoords("irrelevant", "some-action-name/foo/bar/baz", "v2") to "SomeActionNameFooBarBaz",
ActionCoords("irrelevant", "some-action-name", "v2", "subaction") to "SomeActionNameSubaction",
ActionCoords("irrelevant", "some-action-name", "v2", "foo/bar/baz") to "SomeActionNameFooBarBaz",
).forEach { (input, output) ->
test("should get '$input' and produce '$output'") {
input.buildActionClassName() shouldBe output
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,30 @@ class GenerationTest :
binding.shouldContainAndMatchFile("ActionWithNoInputs.kt")
}

test("subaction") {
// given
val actionManifestHasNoInputs = emptyMap<String, Input>()
val actionManifest =
Metadata(
inputs = actionManifestHasNoInputs,
name = "Action With No Inputs",
description = "Description",
)

val coords = ActionCoords("john-smith", "action-with", "v3", "sub/action")

// when
val binding =
coords.generateBinding(
metadataRevision = NewestForVersion,
metadata = actionManifest,
inputTypings = Pair(emptyMap(), ACTION),
)

// then
binding.shouldContainAndMatchFile("ActionWithSubAction.kt")
}

test("action with deprecated input resolving to the same Kotlin field name") {
// given
val actionManifest =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class TypesProvidingTest :
else -> throw IOException()
}
}
val actionCoord = ActionCoords("some-owner", "some-name/some-sub", "v3")
val actionCoord = ActionCoords("some-owner", "some-name", "v3", "some-sub")

// When
val types = actionCoord.provideTypes(metadataRevision = CommitHash("some-hash"), fetchUri = fetchUri)
Expand Down Expand Up @@ -166,7 +166,7 @@ class TypesProvidingTest :
else -> throw IOException()
}
}
val actionCoord = ActionCoords("some-owner", "some-name/some-sub", "v3")
val actionCoord = ActionCoords("some-owner", "some-name", "v3", "some-sub")

// When
val types = actionCoord.provideTypes(metadataRevision = CommitHash("some-hash"), fetchUri = fetchUri)
Expand Down Expand Up @@ -208,7 +208,7 @@ class TypesProvidingTest :
else -> throw IOException()
}
}
val actionCoord = ActionCoords("some-owner", "some-name/some-sub", "v3")
val actionCoord = ActionCoords("some-owner", "some-name", "v3", "some-sub")

// When
val types = actionCoord.provideTypes(metadataRevision = CommitHash("some-hash"), fetchUri = fetchUri)
Expand Down Expand Up @@ -250,7 +250,7 @@ class TypesProvidingTest :
else -> throw IOException()
}
}
val actionCoord = ActionCoords("some-owner", "some-name/some-sub", "v3")
val actionCoord = ActionCoords("some-owner", "some-name", "v3", "some-sub")

// When
val types = actionCoord.provideTypes(metadataRevision = CommitHash("some-hash"), fetchUri = fetchUri)
Expand Down Expand Up @@ -302,7 +302,7 @@ class TypesProvidingTest :
else -> throw IOException()
}
}
val actionCoord = ActionCoords("some-owner", "some-name/some-sub", "v3")
val actionCoord = ActionCoords("some-owner", "some-name", "v3", "some-sub")

// When
val types = actionCoord.provideTypes(metadataRevision = CommitHash("some-hash"), fetchUri = fetchUri)
Expand Down Expand Up @@ -354,7 +354,7 @@ class TypesProvidingTest :
else -> throw IOException()
}
}
val actionCoord = ActionCoords("some-owner", "some-name/some-sub", "v6")
val actionCoord = ActionCoords("some-owner", "some-name", "v6", "some-sub")

// When
val types = actionCoord.provideTypes(metadataRevision = CommitHash("some-hash"), fetchUri = fetchUri)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,15 @@ private fun Route.metadata(refresh: Boolean = false) {
}

val owner = call.parameters["owner"]!!
val name = call.parameters["name"]!!
val nameAndPath = call.parameters["name"]!!.split("__")
val name = nameAndPath.first()
val file = call.parameters["file"]!!
val actionCoords =
ActionCoords(
owner = owner,
name = name,
version = "irrelevant",
path = nameAndPath.drop(1).joinToString("/").takeUnless { it.isBlank() },
)
val bindingArtifacts = actionCoords.buildPackageArtifacts(githubToken = getGithubToken())
if (file in bindingArtifacts) {
Expand Down Expand Up @@ -143,13 +145,15 @@ private suspend fun ApplicationCall.toBindingArtifacts(
refresh: Boolean,
): Map<String, Artifact>? {
val owner = parameters["owner"]!!
val name = parameters["name"]!!
val nameAndPath = parameters["name"]!!.split("__")
val name = nameAndPath.first()
val version = parameters["version"]!!
val actionCoords =
ActionCoords(
owner = owner,
name = name,
version = version,
path = nameAndPath.drop(1).joinToString("/").takeUnless { it.isBlank() },
)
println("➡️ Requesting ${actionCoords.prettyPrint}")
val bindingArtifacts =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package io.github.typesafegithub.workflows.mavenbinding

import io.github.typesafegithub.workflows.actionbindinggenerator.domain.ActionCoords
import io.github.typesafegithub.workflows.actionbindinggenerator.domain.subName

internal val ActionCoords.mavenName: String get() = "$name${subName.replace("/", "__")}"
Loading

0 comments on commit 0343909

Please sign in to comment.