OpenAPI: all `OpenApiMapper`s in scope are used, even if they don't apply to the protocol

[kubukoz]

Hi! My team has noticed that during an OpenAPI conversion, all the mappers in our ClassLoader are applied.

This seems strange to me, because I assumed mappers are protocol-specific. But the implementation takes them all:

smithy/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java

Lines 252 to 259 in 48ccceb

	private static OpenApiMapper createComposedMapper(
	List<Smithy2OpenApiExtension> extensions,
	List<OpenApiMapper> mappers
	) {
	return OpenApiMapper.compose(Stream.concat(
	extensions.stream().flatMap(extension -> extension.getOpenApiMappers().stream()),
	mappers.stream()).collect(Collectors.toList()));
	}

I'm talking about a scenario where config has a protocol set, meaning loadOrDeriveProtocolTrait was able to unambiguously derive a protocol trait.

Why this seems problematic: multiple protocols may handle the same trait differently. If they're both on the same classpath, it appears that the mappers of both protocols will be applied, which would result in unpredictable outcomes.

Or, maybe a protocol wants to ignore @jsonName, and another one has a mapper for that. You get the gist :)

---

If this was intentional: should there be a config option to only pick up mappers from the protocol that's been found / configured (rather than all protocols on the classpath)?

If this was unintentional: is that something that can be changed in a minor version? Happy to contribute.

[sugmanue]

Hi Jakub, this behavior is intentional, we rely on the auto discovery of plugins here and everywhere else in Smith under the assumption that customers can control their classpath to control which ones they want/need to be used. On top of that, as noted in the javadocs

Implementations may choose to leverage configuration options of the provided context to determine whether or not to enact the plugin.

Are any of these options not workable for your use case?

[kubukoz]

The thing with configuration is that the extensions/mappers themselves need to support it - as a user alone, I can't simply disable any of those without manipulating the classpath. And removing things from a classpath is harder than adding them 😅

It doesn't help that my particular protocol "extends" another one - that'll only make it harder, if not outright impossible, to narrow the classpath down enough.

Maybe we can have something in OpenApiConfig that'll allow one to explicitly add extensions/mappers to be used, rather than relying only on the ClassLoader for discovery? That way, I could pass an empty ClassLoader and pass just the relevant extension.