Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc: add visibility build setting #951

Merged
merged 18 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/b6288fbe-a409-473c-a6ac-12c3c310b963.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "b6288fbe-a409-473c-a6ac-12c3c310b963",
"type": "misc",
"description": "Generate internal-only clients with `internal` visibility"
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ private const val PACKAGE_NAME = "name"
private const val PACKAGE_VERSION = "version"
private const val PACKAGE_DESCRIPTION = "description"
private const val BUILD_SETTINGS = "build"
private const val API_SETTINGS = "api"

// Optional specification of sdkId for models that provide them, otherwise Service's shape id name is used
private const val SDK_ID = "sdkId"
Expand All @@ -37,6 +38,7 @@ data class KotlinSettings(
val pkg: PackageSettings,
val sdkId: String,
val build: BuildSettings = BuildSettings.Default,
val api: ApiSettings = ApiSettings.Default,
) {

/**
Expand Down Expand Up @@ -91,11 +93,13 @@ data class KotlinSettings(
// Load the sdk id from configurations that define it, fall back to service name for those that don't.
val sdkId = config.getStringMemberOrDefault(SDK_ID, serviceId.name)
val build = config.getObjectMember(BUILD_SETTINGS)
val api = config.getObjectMember(API_SETTINGS)
return KotlinSettings(
serviceId,
PackageSettings(packageName, version, desc),
sdkId,
BuildSettings.fromNode(build),
ApiSettings.fromNode(api),
)
}
}
Expand Down Expand Up @@ -170,8 +174,7 @@ data class BuildSettings(
fun fromNode(node: Optional<ObjectNode>): BuildSettings = node.map {
val generateFullProject = node.get().getBooleanMemberOrDefault(ROOT_PROJECT, false)
val generateBuildFiles = node.get().getBooleanMemberOrDefault(GENERATE_DEFAULT_BUILD_FILES, true)
val generateMultiplatformProject =
node.get().getBooleanMemberOrDefault(GENERATE_MULTIPLATFORM_MODULE, false)
val generateMultiplatformProject = node.get().getBooleanMemberOrDefault(GENERATE_MULTIPLATFORM_MODULE, false)
val annotations = node.get().getArrayMember(ANNOTATIONS).map {
it.elements.mapNotNull { node ->
node.asStringNode().map { stringNode ->
Expand All @@ -193,3 +196,42 @@ data class BuildSettings(
class UnresolvableProtocolException(message: String) : CodegenException(message)

private fun <T> Optional<T>.orNull(): T? = if (isPresent) get() else null

/**
* The visibility of code-generated classes, objects, interfaces, etc.
* Valid values are `public` and `internal`. `private` not supported because codegen would not compile with private classes.
*/
enum class Visibility(val value: String) {
PUBLIC("public"),
INTERNAL("internal"),
;

companion object {
public fun fromValue(value: String): Visibility = when (value.lowercase()) {
"internal" -> INTERNAL
else -> PUBLIC
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness: This means silently ignoring the caller's intent when we don't understand it, leading to typos like "itnernal" being treated as PUBLIC. I think we should throw an exception if we don't understand the input value.

}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider overriding toString to return value so that you don't have to call .value so many places in codegen.


/**
* Contains API settings for a Kotlin project
* @param visibility Enum representing the visibility of code-generated classes, objects, interfaces, etc.
*/
data class ApiSettings(
val visibility: Visibility = Visibility.PUBLIC,
) {
companion object {
const val VISIBILITY = "visibility"

fun fromNode(node: Optional<ObjectNode>): ApiSettings = node.map {
val visibility = Visibility.fromValue(node.get().getStringMemberOrDefault(VISIBILITY, "public"))
ApiSettings(visibility)
}.orElse(Default)

/**
* Default build settings
*/
val Default: ApiSettings = ApiSettings()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ object ExceptionBaseClassGenerator {
val name = clientName(ctx.settings.sdkId)
writer.dokka("Base class for all service related exceptions thrown by the $name client")
writer.withBlock(
"public open class #T : #T {",
"#L open class #T : #T {",
"}",
ctx.settings.api.visibility.value,
serviceException,
baseException,
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {

fun render() {
writer.write("\n\n")
writer.write("public const val ServiceId: String = #S", ctx.settings.sdkId)
writer.write("public const val SdkVersion: String = #S", ctx.settings.pkg.version)
writer.write("#L const val ServiceId: String = #S", ctx.settings.api.visibility.value, ctx.settings.sdkId)
writer.write("#L const val SdkVersion: String = #S", ctx.settings.api.visibility.value, ctx.settings.pkg.version)
writer.write("\n\n")

writer.putContext("service.name", ctx.settings.sdkId)
Expand All @@ -82,7 +82,11 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {

writer.renderDocumentation(service)
writer.renderAnnotations(service)
writer.openBlock("public interface ${serviceSymbol.name} : #T {", RuntimeTypes.SmithyClient.SdkClient)
writer.openBlock(
"#L interface ${serviceSymbol.name} : #T {",
ctx.settings.api.visibility.value,
RuntimeTypes.SmithyClient.SdkClient,
)
.call {
// allow access to client's Config
writer.dokka("${serviceSymbol.name}'s configuration")
Expand Down Expand Up @@ -198,7 +202,12 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
write("Any resources created on your behalf will be shared between clients, and will only be closed when ALL clients using them are closed.")
write("If you provide a resource (e.g. [HttpClientEngine]) to the SDK, you are responsible for managing the lifetime of that resource.")
}
writer.withBlock("public fun #1T.withConfig(block: #1T.Config.Builder.() -> Unit): #1T {", "}", serviceSymbol) {
writer.withBlock(
"#1L fun #2T.withConfig(block: #2T.Config.Builder.() -> Unit): #2T {",
"}",
ctx.settings.api.visibility.value,
serviceSymbol,
) {
write("val newConfig = config.toBuilder().apply(block).build()")
write("return Default#L(newConfig)", serviceSymbol.name)
}
Expand All @@ -219,7 +228,8 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) {
writer.renderDocumentation(op)
writer.renderAnnotations(op)
writer.write(
"public suspend inline fun #T.#L(crossinline block: #T.Builder.() -> Unit): #T = #L(#T.Builder().apply(block).build())",
"#L suspend inline fun #T.#L(crossinline block: #T.Builder.() -> Unit): #T = #L(#T.Builder().apply(block).build())",
ctx.settings.api.visibility.value,
serviceSymbol,
operationName,
inputSymbol,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ class StructureGenerator(
* Renders a normal (non-error) Smithy structure to a Kotlin class
*/
private fun renderStructure() {
writer.openBlock("public class #T private constructor(builder: Builder) {", symbol)
writer.openBlock(
"#L class #T private constructor(builder: Builder) {",
ctx.settings.api.visibility.value,
symbol,
)
.call { renderImmutableProperties() }
.write("")
.call { renderCompanionObject() }
Expand Down Expand Up @@ -299,7 +303,11 @@ class StructureGenerator(
val exceptionBaseClass = ExceptionBaseClassGenerator.baseExceptionSymbol(ctx.settings)
writer.addImport(exceptionBaseClass)

writer.openBlock("public class #T private constructor(builder: Builder) : ${exceptionBaseClass.name}() {", symbol)
writer.openBlock(
"#L class #T private constructor(builder: Builder) : ${exceptionBaseClass.name}() {",
ctx.settings.api.visibility.value,
symbol,
)
.write("")
.call { renderImmutableProperties() }
.write("")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ class AuthSchemeParametersGenerator : AbstractConfigGenerator() {
val implSymbol = getImplementationSymbol(ctx.settings)

ctx.delegator.useSymbolWriter(symbol) { writer ->
writer.withBlock("public interface #T {", "}", symbol) {
writer.withBlock(
"#L interface #T {",
"}",
ctx.settings.api.visibility.value,
symbol,
) {
dokka("The name of the operation currently being invoked.")
write("public val operationName: String")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ open class AuthSchemeProviderGenerator {
write("See [#T] for the default SDK behavior of this interface.", getDefaultSymbol(ctx.settings))
}
writer.write(
"public interface #T : #T<#T>",
"#L interface #T : #T<#T>",
ctx.settings.api.visibility.value,
symbol,
RuntimeTypes.Auth.Identity.AuthSchemeProvider,
paramsSymbol,
Expand All @@ -64,8 +65,9 @@ open class AuthSchemeProviderGenerator {
private fun renderDefaultImpl(ctx: ProtocolGenerator.GenerationContext, writer: KotlinWriter) {
// FIXME - probably can't remain an object
writer.withBlock(
"public object #T : #T {",
"#L object #T : #T {",
"}",
ctx.settings.api.visibility.value,
getDefaultSymbol(ctx.settings),
getSymbol(ctx.settings),
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class DefaultEndpointProviderGenerator(
private val defaultProviderSymbol: Symbol,
private val interfaceSymbol: Symbol,
private val paramsSymbol: Symbol,
private val settings: KotlinSettings,
private val externalFunctions: Map<String, Symbol> = emptyMap(),
private val propertyRenderers: Map<String, EndpointPropertyRenderer> = emptyMap(),
) : ExpressionRenderer {
Expand All @@ -78,7 +79,13 @@ class DefaultEndpointProviderGenerator(

fun render() {
renderDocumentation()
writer.withBlock("public class #T: #T {", "}", defaultProviderSymbol, interfaceSymbol) {
writer.withBlock(
"#L class #T: #T {",
"}",
settings.api.visibility.value,
defaultProviderSymbol,
interfaceSymbol,
) {
renderResolve()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ interface EndpointDelegator {
val defaultProviderSymbol = DefaultEndpointProviderGenerator.getSymbol(ctx.settings)

ctx.delegator.useFileWriter(providerSymbol) {
EndpointProviderGenerator(it, providerSymbol, paramsSymbol).render()
EndpointProviderGenerator(it, ctx.settings, providerSymbol, paramsSymbol).render()
}

if (rules != null) {
ctx.delegator.useFileWriter(defaultProviderSymbol) {
DefaultEndpointProviderGenerator(it, rules, defaultProviderSymbol, providerSymbol, paramsSymbol).render()
DefaultEndpointProviderGenerator(it, rules, defaultProviderSymbol, providerSymbol, paramsSymbol, ctx.settings).render()
}
}
}
Expand All @@ -49,7 +49,7 @@ interface EndpointDelegator {
fun generateEndpointParameters(ctx: ProtocolGenerator.GenerationContext, rules: EndpointRuleSet?) {
val paramsSymbol = EndpointParametersGenerator.getSymbol(ctx.settings)
ctx.delegator.useFileWriter(paramsSymbol) {
EndpointParametersGenerator(it, rules, paramsSymbol).render()
EndpointParametersGenerator(it, ctx.settings, rules, paramsSymbol).render()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ private const val DEFAULT_DEPRECATED_MESSAGE =
*/
class EndpointParametersGenerator(
private val writer: KotlinWriter,
private val settings: KotlinSettings,
rules: EndpointRuleSet?,
private val paramsSymbol: Symbol,
) {
Expand Down Expand Up @@ -51,7 +52,12 @@ class EndpointParametersGenerator(
fun render() {
renderDocumentation()
// FIXME - this should probably be an interface
writer.withBlock("public class #T private constructor(builder: Builder) {", "}", paramsSymbol) {
writer.withBlock(
"#L class #T private constructor(builder: Builder) {",
"}",
settings.api.visibility.value,
paramsSymbol,
) {
renderFields()
renderCompanionObject()
write("")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import software.amazon.smithy.kotlin.codegen.model.buildSymbol
*/
class EndpointProviderGenerator(
private val writer: KotlinWriter,
private val settings: KotlinSettings,
private val providerSymbol: Symbol,
private val paramsSymbol: Symbol,
) {
Expand All @@ -33,7 +34,8 @@ class EndpointProviderGenerator(
fun render() {
renderDocumentation()
writer.write(
"public fun interface #T: #T<#T>",
"#L fun interface #T: #T<#T>",
settings.api.visibility.value,
providerSymbol,
RuntimeTypes.SmithyClient.Endpoints.EndpointProvider,
paramsSymbol,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ class EndpointDiscovererGenerator(private val ctx: CodegenContext, private val d
calls.
""".trimIndent(),
)
withBlock("public class #T {", "}", symbol) {
withBlock(
"#L class #T {",
"}",
ctx.settings.api.visibility.value,
symbol,
) {
write(
"private val cache = #T<DiscoveryParams, #T>(10.#T, #T.System)",
RuntimeTypes.Core.Utils.ReadThroughCache,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,82 @@ class KotlinSettingsTest {
Node.parse(contents).expectObjectNode(),
)
}

@Test
fun `supports internal visibility`() {
val model = javaClass.getResource("simple-service.smithy")!!.toSmithyModel()

val contents = """
{
"package": {
"name": "aws.sdk.kotlin.runtime.protocoltest.awsrestjson",
"version": "1.0.0"
},
"build": {
"optInAnnotations": ["foo", "bar"]
},
"api": {
"visibility": "internal"
}
}
""".trimIndent()

val settings = KotlinSettings.from(
model,
Node.parse(contents).expectObjectNode(),
)

assertEquals(Visibility.INTERNAL, settings.api.visibility)
}

@Test
fun `defaults to public visibility`() {
val model = javaClass.getResource("simple-service.smithy")!!.toSmithyModel()

val contents = """
{
"package": {
"name": "aws.sdk.kotlin.runtime.protocoltest.awsrestjson",
"version": "1.0.0"
},
"build": {
"optInAnnotations": ["foo", "bar"]
}
}
""".trimIndent()

val settings = KotlinSettings.from(
model,
Node.parse(contents).expectObjectNode(),
)

assertEquals(Visibility.PUBLIC, settings.api.visibility)
}

@Test
fun `works with unsupported visibility values`() {
val model = javaClass.getResource("simple-service.smithy")!!.toSmithyModel()

val contents = """
{
"package": {
"name": "aws.sdk.kotlin.runtime.protocoltest.awsrestjson",
"version": "1.0.0"
},
"build": {
"optInAnnotations": ["foo", "bar"]
},
"api": {
"visibility": "I don't know, just make it visible"
}
}
""".trimIndent()

val settings = KotlinSettings.from(
model,
Node.parse(contents).expectObjectNode(),
)

assertEquals(Visibility.PUBLIC, settings.api.visibility)
}
}
Loading
Loading