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

refactor!: generate per/service base exception types #270

Merged
merged 2 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,13 @@ class CodegenVisitor(context: PluginContext) : ShapeVisitor.Default<Unit>() {
writers.flushWriters()
}

override fun getDefault(shape: Shape?) {
}
override fun getDefault(shape: Shape?) { }

override fun structureShape(shape: StructureShape) {
writers.useShapeWriter(shape) { StructureGenerator(model, symbolProvider, it, shape, protocolGenerator).render() }
writers.useShapeWriter(shape) {
val renderingContext = baseGenerationContext.toRenderingContext(it, shape)
StructureGenerator(renderingContext).render()
}
}

override fun stringShape(shape: StringShape) {
Expand All @@ -157,5 +159,11 @@ class CodegenVisitor(context: PluginContext) : ShapeVisitor.Default<Unit>() {
val renderingCtx = baseGenerationContext.toRenderingContext(it, shape)
ServiceGenerator(renderingCtx).render()
}

// render the service (client) base exception type
val baseExceptionSymbol = ExceptionBaseClassGenerator.baseExceptionSymbol(baseGenerationContext.settings)
writers.useFileWriter("${baseExceptionSymbol.name}.kt", baseExceptionSymbol.namespace) {
ExceptionBaseClassGenerator.render(baseGenerationContext, it)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/

package software.amazon.smithy.kotlin.codegen

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.kotlin.codegen.integration.ProtocolGenerator

/**
* Renders the base class that all (modeled) exceptions inherit from.
* Protocol generators are allowed to override this but they MUST inherit from the base `ServiceException`
* with the expected constructors.
*/
object ExceptionBaseClassGenerator {
fun render(ctx: CodegenContext, writer: KotlinWriter) {
val baseException = ctx.protocolGenerator?.exceptionBaseClassSymbol ?: ProtocolGenerator.DefaultServiceExceptionSymbol
writer.addImport(baseException)
val serviceException = baseExceptionSymbol(ctx.settings)

writer.dokka("Base class for all service related exceptions thrown by the ${ctx.settings.sdkId.clientName()} client")
writer.withBlock(
"open class #T : #T {", "}",
serviceException,
baseException
) {
write("constructor() : super()")
write("constructor(message: String?) : super(message)")
write("constructor(message: String?, cause: Throwable?) : super(message, cause)")
write("constructor(cause: Throwable?) : super(cause)")
}
}

/**
* Get the (generated) symbol that constitutes the base class exceptions will inherit from
*/
fun baseExceptionSymbol(settings: KotlinSettings): Symbol = buildSymbol {
val serviceName = settings.sdkId.clientName()
name = "${serviceName}Exception"
namespace = "${settings.pkg.name}.model"
definitionFile = "$name.kt"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ package software.amazon.smithy.kotlin.codegen

import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.kotlin.codegen.integration.ProtocolGenerator
import software.amazon.smithy.kotlin.codegen.lang.KotlinTypes
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.ShapeType
Expand All @@ -23,15 +20,15 @@ import software.amazon.smithy.model.traits.StreamingTrait
* Renders Smithy structure shapes
*/
class StructureGenerator(
val model: Model,
private val symbolProvider: SymbolProvider,
private val writer: KotlinWriter,
private val shape: StructureShape,
private val protocolGenerator: ProtocolGenerator? = null
private val ctx: RenderingContext<StructureShape>
) {
private val shape = requireNotNull(ctx.shape)
private val writer = ctx.writer
private val symbolProvider = ctx.symbolProvider
private val model = ctx.model

fun render() {
val symbol = symbolProvider.toSymbol(shape)
val symbol = ctx.symbolProvider.toSymbol(ctx.shape)
// push context to be used throughout generation of the class
writer.putContext("class.name", symbol.name)

Expand Down Expand Up @@ -299,7 +296,7 @@ class StructureGenerator(

checkForConflictsInHierarchy()

val exceptionBaseClass = protocolGenerator?.exceptionBaseClassSymbol ?: ProtocolGenerator.DefaultServiceExceptionSymbol
val exceptionBaseClass = ExceptionBaseClassGenerator.baseExceptionSymbol(ctx.settings)
writer.addImport(exceptionBaseClass)

writer.openBlock("class #class.name:L private constructor(builder: BuilderImpl) : ${exceptionBaseClass.name}() {")
Expand Down Expand Up @@ -340,7 +337,6 @@ class StructureGenerator(
}
}
writer.write("sdkErrorMetadata.attributes[ServiceErrorMetadata.ErrorType] = $errorType")
writer.addImport(RuntimeTypes.Core.ErrorMetadata)
writer.addImport(RuntimeTypes.Core.ServiceErrorMetadata)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,6 @@ class SymbolVisitor(private val model: Model, private val rootNamespace: String
}
}

// See https://awslabs.github.io/smithy/1.0/spec/aws/aws-core.html#using-sdk-service-id-for-client-naming
fun String.clientName(): String =
split(" ").map { it.toLowerCase().capitalize() }.joinToString(separator = "") { it }

/**
* Mark a symbol as being boxed (nullable) i.e. `T?`
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,42 @@ internal fun Int.nestedDescriptorName(): String = "_c$this"
* Get the value if present otherwise return null
*/
fun <T> Optional<T>.getOrNull(): T? = if (isPresent) get() else null

/**
* Split a string on word boundaries
*/
fun String.splitOnWordBoundaries(): List<String> {
// adapted from Java v2 SDK CodegenNamingUtils.splitOnWordBoundaries
var result = this

// all non-alphanumeric characters: "acm-success"-> "acm success"
result = result.replace(Regex("[^A-Za-z0-9+]"), " ")

// if a number has a standalone v in front of it, separate it out
result = result.replace(Regex("([^a-z]{2,})v([0-9]+)"), "$1 v$2 ") // TESTv4 -> "TEST v4 "
.replace(Regex("([^A-Z]{2,})V([0-9]+)"), "$1 V$2 ") // TestV4 -> "Test V4 "

// add a space between camelCased words
result = result.split(Regex("(?<=[a-z])(?=[A-Z]([a-zA-Z]|[0-9]))")).joinToString(separator = " ") // AcmSuccess -> // "Acm Success"

// add a space after acronyms
result = result.replace(Regex("([A-Z]+)([A-Z][a-z])"), "$1 $2") // "ACMSuccess" -> "ACM Success"

// add space after a number in the middle of a word
result = result.replace(Regex("([0-9])([a-zA-Z])"), "$1 $2") // "s3ec2" -> "s3 ec2"

// remove extra spaces - multiple consecutive ones or those and the beginning/end of words
result = result.replace(Regex("\\s+"), " ") // "Foo Bar" -> "Foo Bar"
.trim() // " Foo " -> "Foo"

return result.split(" ")
}

/**
* Get the generated SDK service client name to use. The target should be a string that represents the `sdkId`
* of the service.
*
* See https://awslabs.github.io/smithy/1.0/spec/aws/aws-core.html#using-sdk-service-id-for-client-naming
*/
fun String.clientName(): String =
splitOnWordBoundaries().joinToString(separator = "") { it.toLowerCase().capitalize() }
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ package software.amazon.smithy.kotlin.codegen.integration

import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.kotlin.codegen.ApplicationProtocol
import software.amazon.smithy.kotlin.codegen.KotlinDelegator
import software.amazon.smithy.kotlin.codegen.KotlinDependency
import software.amazon.smithy.kotlin.codegen.KotlinSettings
import software.amazon.smithy.kotlin.codegen.*
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.ShapeId
Expand All @@ -44,11 +41,10 @@ interface ProtocolGenerator {
return CaseUtils.toCamelCase(s1, true, '_')
}

val DefaultServiceExceptionSymbol: Symbol = Symbol.builder()
.name("ServiceException")
.namespace(KotlinDependency.CLIENT_RT_CORE.namespace, ".")
.addDependency(KotlinDependency.CLIENT_RT_CORE)
.build()
val DefaultServiceExceptionSymbol: Symbol = buildSymbol {
name = "ServiceException"
namespace(KotlinDependency.CLIENT_RT_CORE)
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ package software.amazon.smithy.kotlin.codegen
import io.kotest.matchers.string.shouldContain
import io.kotest.matchers.string.shouldNotContain
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import software.amazon.smithy.codegen.core.CodegenException
import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.codegen.core.SymbolProvider
import software.amazon.smithy.kotlin.codegen.integration.ProtocolGenerator
import software.amazon.smithy.model.shapes.*
import kotlin.test.assertFailsWith

Expand Down Expand Up @@ -55,27 +58,29 @@ class ExceptionGeneratorTest {
val symbolProvider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, "error.test", "Test")
val errorWriter = KotlinWriter("com.error.test")
val clientErrorShape = model.expectShape<StructureShape>("com.error.test#ValidationException")
val clientErrorGenerator = StructureGenerator(model, symbolProvider, errorWriter, clientErrorShape)
clientErrorGenerator.render()
val settings = model.defaultSettings(serviceName = "com.test.error#Foo")
val clientErrorRenderingCtx = RenderingContext(errorWriter, clientErrorShape, model, symbolProvider, settings)
StructureGenerator(clientErrorRenderingCtx).render()

clientErrorTestContents = errorWriter.toString()

val serverErrorWriter = KotlinWriter("com.error.test")
val serverErrorShape = model.expectShape<StructureShape>("com.error.test#InternalServerException")
val serverErrorGenerator = StructureGenerator(model, symbolProvider, serverErrorWriter, serverErrorShape)
serverErrorGenerator.render()
val serverErrorRenderingCtx = RenderingContext(serverErrorWriter, serverErrorShape, model, symbolProvider, settings)
StructureGenerator(serverErrorRenderingCtx).render()
serverErrorTestContents = serverErrorWriter.toString()
}

@Test
fun `error generator extends correctly`() {
val expectedClientClassDecl = """
class ValidationException private constructor(builder: BuilderImpl) : ServiceException() {
class ValidationException private constructor(builder: BuilderImpl) : FooException() {
"""

clientErrorTestContents.shouldContain(expectedClientClassDecl)

val expectedServerClassDecl = """
class InternalServerException private constructor(builder: BuilderImpl) : ServiceException() {
class InternalServerException private constructor(builder: BuilderImpl) : FooException() {
"""

serverErrorTestContents.shouldContain(expectedServerClassDecl)
Expand Down Expand Up @@ -130,10 +135,105 @@ class InternalServerException private constructor(builder: BuilderImpl) : Servic
val symbolProvider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, "error.test", "Test")
val writer = KotlinWriter("com.error.test")
val errorShape = model.expectShape<StructureShape>("com.error.test#ConflictingException")
val renderingCtx = RenderingContext(writer, errorShape, model, symbolProvider, model.defaultSettings())
val ex = assertFailsWith<CodegenException> {
StructureGenerator(model, symbolProvider, writer, errorShape).render()
StructureGenerator(renderingCtx).render()
}

ex.message!!.shouldContain("`sdkErrorMetadata` conflicts with property of same name inherited from SdkBaseException. Apply a rename customization/projection to fix.")
}

@Test
fun `it fails if message property is of wrong type`() {
val model = """
namespace com.error.test

@httpError(500)
@error("server")
structure InternalServerException {
message: Integer
}
""".asSmithyModel()

val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, "test", "Test")
val writer = KotlinWriter("com.test")

val struct = model.expectShape<StructureShape>("com.error.test#InternalServerException")
val renderingCtx = RenderingContext(writer, struct, model, provider, model.defaultSettings())

val e = assertThrows<CodegenException> {
StructureGenerator(renderingCtx).render()
}
e.message.shouldContainOnlyOnceWithDiff("Message is a reserved name for exception types and cannot be used for any other property")
}

class BaseExceptionGeneratorTest {

@Test
fun itGeneratesAnExceptionBaseClass() {
val model = """
namespace com.error.test
""".asSmithyModel()
val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, "test", "Test")
val writer = KotlinWriter("com.test")

val ctx = GenerationContext(model, provider, model.defaultSettings(serviceName = "com.error.test#Foo"))
ExceptionBaseClassGenerator.render(ctx, writer)
val contents = writer.toString()

val expected = """
open class FooException : ServiceException {
constructor() : super()
constructor(message: String?) : super(message)
constructor(message: String?, cause: Throwable?) : super(message, cause)
constructor(cause: Throwable?) : super(cause)
}
""".trimIndent()

contents.shouldContainOnlyOnceWithDiff(expected)
}

@Test
fun itExtendsProtocolGeneratorBaseClass() {
val model = """
namespace com.error.test
""".asSmithyModel()
val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model, "test", "Test")
val writer = KotlinWriter("com.test")

val protocolGenerator = object : ProtocolGenerator {
override val protocol: ShapeId
get() = TODO("Not yet implemented")

override val applicationProtocol: ApplicationProtocol
get() = TODO("Not yet implemented")

override fun generateSerializers(ctx: ProtocolGenerator.GenerationContext) {}
override fun generateDeserializers(ctx: ProtocolGenerator.GenerationContext) {}
override fun generateProtocolUnitTests(ctx: ProtocolGenerator.GenerationContext) {}
override fun generateProtocolClient(ctx: ProtocolGenerator.GenerationContext) {}

override val exceptionBaseClassSymbol: Symbol = buildSymbol {
name = "QuxException"
namespace = "foo.bar"
}
}

val ctx = GenerationContext(model, provider, model.defaultSettings(serviceName = "com.error.test#Foo"), protocolGenerator)
ExceptionBaseClassGenerator.render(ctx, writer)
val contents = writer.toString()

val expected = """
open class FooException : QuxException {
constructor() : super()
constructor(message: String?) : super(message)
constructor(message: String?, cause: Throwable?) : super(message, cause)
constructor(cause: Throwable?) : super(cause)
}
""".trimIndent()

contents.shouldContainOnlyOnceWithDiff(expected)
contents.shouldContainOnlyOnceWithDiff("import foo.bar.QuxException")
}
}
}
Loading