Skip to content

Commit 6563ec8

Browse files
committed
Fix trait/validator loading with discoverModels
When discoverModels(ClassLoader) is called on a bare Model.assembler() (not viaModel.assembler(ClassLoader)), the provided ClassLoader is used for model discovery but not for trait/validator factory creation, so traits from models that are only on the discoverModels' classpath fall back to DynamicTrait instead as the TraitFactory instance on the assembler uses the default classpath. This commit configures the traitFactory and validatorFactory from the provided ClassLoader when they have not already been set.
1 parent 936509a commit 6563ec8

3 files changed

Lines changed: 48 additions & 2 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "bugfix",
3+
"description": "Fix trait/validator loading with discoverModels",
4+
"pull_requests": [
5+
"[#3049](https://github.com/smithy-lang/smithy/pull/3049)"
6+
]
7+
}

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,18 +393,28 @@ public ModelAssembler putMetadata(String name, Node value) {
393393
}
394394

395395
/**
396-
* Discovers models by merging in all models returns by {@link ModelDiscovery}
396+
* Discovers models by merging in all models returned by {@link ModelDiscovery}
397397
* manifests using the provided {@code ClassLoader}.
398398
*
399+
* <p>This method also configures the assembler to use the provided
400+
* {@code ClassLoader} for discovering trait and validator service
401+
* providers if they have not already been explicitly configured.
402+
*
399403
* @param loader Class loader to use to discover models.
400404
* @return Returns the model assembler.
401405
*/
402406
public ModelAssembler discoverModels(ClassLoader loader) {
407+
if (traitFactory == null) {
408+
traitFactory = TraitFactory.createServiceFactory(loader);
409+
}
410+
if (validatorFactory == null) {
411+
validatorFactory = ValidatorFactory.createServiceFactory(loader);
412+
}
403413
return addDiscoveredModels(ModelDiscovery.findModels(loader));
404414
}
405415

406416
/**
407-
* Discovers models by merging in all models returns by {@link ModelDiscovery}
417+
* Discovers models by merging in all models returned by {@link ModelDiscovery}
408418
* manifests using the thread context {@code ClassLoader}.
409419
*
410420
* @return Returns the model assembler.

smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,35 @@ public void canLoadTraitFromJarMultipleTimes() {
641641
assertTrue(reloadedModel.getShape(ShapeId.from("smithy.test#test")).isPresent());
642642
}
643643

644+
// Regression test for https://github.com/smithy-lang/smithy/issues/2596.
645+
// When discoverModels(ClassLoader) is called without first configuring a
646+
// traitFactory, the assembler should use the provided classloader for trait
647+
// SPI discovery so that traits resolve to their concrete classes rather
648+
// than falling back to DynamicTrait.
649+
@Test
650+
public void discoverModelsWithClassLoaderConfiguresTraitFactory() {
651+
URL jar = getClass().getResource("jar-traits-import.jar");
652+
URL file = getClass().getResource("loads-jar-traits.smithy");
653+
URLClassLoader urlClassLoader = new URLClassLoader(new URL[] {jar});
654+
655+
// Use a plain Model.assembler() call (not Model.assembler(classLoader))
656+
// to reproduce the scenario where no explicit traitFactory is set.
657+
Model model = Model.assembler()
658+
.discoverModels(urlClassLoader)
659+
.addImport(file)
660+
.assemble()
661+
.unwrap();
662+
663+
ShapeId traitId = ShapeId.from("smithy.test#test");
664+
assertTrue(model.getShape(traitId).isPresent());
665+
666+
// The trait definition shape itself should be present and not a DynamicTrait
667+
// on the service that uses it.
668+
Shape weatherService = model.expectShape(ShapeId.from("ns.test#Weather"));
669+
assertTrue(weatherService.hasTrait(traitId));
670+
assertThat(weatherService.findTrait(traitId).get(), not(instanceOf(DynamicTrait.class)));
671+
}
672+
644673
@Test
645674
public void canDisableValidation() {
646675
String document = "namespace foo.baz\n"

0 commit comments

Comments
 (0)