-
Notifications
You must be signed in to change notification settings - Fork 28
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
Changes from 7 commits
a4f1190
2440471
3515130
46bbf12
51d6df8
f88aaa5
5621498
8733525
7f14f8d
506da38
245559f
7c43ce9
816d6bf
c5d14d4
c3b41cf
a249bef
c0ec3a3
4b58f2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 visibility" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 VISIBILITY_SETTINGS = "visibility" | ||
|
||
// Optional specification of sdkId for models that provide them, otherwise Service's shape id name is used | ||
private const val SDK_ID = "sdkId" | ||
|
@@ -160,6 +161,7 @@ data class BuildSettings( | |
val generateDefaultBuildFiles: Boolean = true, | ||
val optInAnnotations: List<String>? = null, | ||
val generateMultiplatformProject: Boolean = false, | ||
val visibility: VisibilitySettings = VisibilitySettings.Default, | ||
) { | ||
companion object { | ||
const val ROOT_PROJECT = "rootProject" | ||
|
@@ -170,17 +172,17 @@ 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 -> | ||
stringNode.value | ||
}.orNull() | ||
} | ||
}.orNull() | ||
val visibility = VisibilitySettings.fromNode(node.get().getObjectMember(VISIBILITY_SETTINGS)) | ||
|
||
BuildSettings(generateFullProject, generateBuildFiles, annotations, generateMultiplatformProject) | ||
BuildSettings(generateFullProject, generateBuildFiles, annotations, generateMultiplatformProject, visibility) | ||
}.orElse(Default) | ||
|
||
/** | ||
|
@@ -193,3 +195,27 @@ data class BuildSettings( | |
class UnresolvableProtocolException(message: String) : CodegenException(message) | ||
|
||
private fun <T> Optional<T>.orNull(): T? = if (isPresent) get() else null | ||
|
||
data class VisibilitySettings( | ||
val serviceClient: String = "public", | ||
val structure: String = "public", | ||
val error: String = "public", | ||
) { | ||
companion object { | ||
const val SERVICE_CLIENT = "serviceClient" | ||
const val STRUCTURE = "structure" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. correctness I'm not sure we want to model visibility of structure and error settings separate from the service client. If you set structure or error visibility to From where I'm sitting at the moment I think we can get away with the same visibility for all of these (which is either There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this is one thing I expected discussion about. I started with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend using an enum too instead of a string. |
||
const val ERROR = "error" | ||
|
||
fun fromNode(node: Optional<ObjectNode>): VisibilitySettings = node.map { | ||
val serviceClient = node.get().getStringMemberOrDefault(SERVICE_CLIENT, "public") | ||
val structure = node.get().getStringMemberOrDefault(STRUCTURE, "public") | ||
val error = node.get().getStringMemberOrDefault(ERROR, "public") | ||
VisibilitySettings(serviceClient, structure, error) | ||
}.orElse(Default) | ||
|
||
/** | ||
* Default visibility settings | ||
*/ | ||
val Default: VisibilitySettings = VisibilitySettings() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,11 +67,12 @@ class ServiceClientGenerator(private val ctx: RenderingContext<ServiceShape>) { | |
requireNotNull(ctx.shape) { "ServiceShape is required to render a service client" } | ||
private val serviceSymbol = ctx.symbolProvider.toSymbol(service) | ||
private val writer = ctx.writer | ||
private val visibility = ctx.settings.build.visibility.serviceClient | ||
|
||
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("$visibility const val ServiceId: String = #S", ctx.settings.sdkId) | ||
writer.write("$visibility const val SdkVersion: String = #S", ctx.settings.pkg.version) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Mix templating styles makes codegen harder to read. I recommend using |
||
writer.write("\n\n") | ||
|
||
writer.putContext("service.name", ctx.settings.sdkId) | ||
|
@@ -82,7 +83,7 @@ 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("$visibility interface ${serviceSymbol.name} : #T {", RuntimeTypes.SmithyClient.SdkClient) | ||
.call { | ||
// allow access to client's Config | ||
writer.dokka("${serviceSymbol.name}'s configuration") | ||
|
@@ -198,7 +199,7 @@ 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("$visibility fun #1T.withConfig(block: #1T.Config.Builder.() -> Unit): #1T {", "}", serviceSymbol) { | ||
write("val newConfig = config.toBuilder().apply(block).build()") | ||
write("return Default#L(newConfig)", serviceSymbol.name) | ||
} | ||
|
@@ -219,7 +220,7 @@ 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())", | ||
"$visibility suspend inline fun #T.#L(crossinline block: #T.Builder.() -> Unit): #T = #L(#T.Builder().apply(block).build())", | ||
serviceSymbol, | ||
operationName, | ||
inputSymbol, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't really "build" settings since this type doc comments explicitly talk about gradle build settings. I think maybe we want a new type like "API" settings?