automatically infer namespaces #1744
Conversation
tgummerer
force-pushed
the
tg/auto-infer-namespaces
branch
from
f6978a9 to
5df9dd6
Compare
| @@ -32,7 +32,8 @@ void testGenerateSchemaWithoutMetadata() { | |||
| var expected = new PackageSpec() | |||
| .setName("infer") | |||
| .setDisplayName("infer") | |||
| .setVersion(null); | |||
| .setVersion(null) | |||
| .setNamespace("pulumi"); | |||
There was a problem hiding this comment.
Before this PR there would not have been any namespace set, so this will test it.
| throw new IllegalArgumentException("At least one component class must be provided"); | ||
| } | ||
|
|
||
| String namespace = classes[0].getPackage().getName().split("\\.")[1]; |
There was a problem hiding this comment.
Not sure if we should error out if not all classes have the same namespace?
There was a problem hiding this comment.
Probably a safer way to go
There was a problem hiding this comment.
Done, added an exception.
| throw new IllegalArgumentException("At least one component class must be provided"); | ||
| } | ||
|
|
||
| String namespace = classes[0].getPackage().getName().split("\\.")[1]; |
There was a problem hiding this comment.
Probably a safer way to go
| throw new IllegalArgumentException("At least one component class must be provided"); | ||
| } | ||
|
|
||
| String namespace = classes[0].getPackage().getName().split("\\.")[1]; |
There was a problem hiding this comment.
Can there be no . at all?
There was a problem hiding this comment.
There can be. I think in that case we just leave the namespace empty. I made it so now.
| @@ -58,6 +66,10 @@ public String getName() { | |||
| return name; | |||
| } | |||
|
|
|||
| public String getNamespace() { | |||
| return namespace; | |||
There was a problem hiding this comment.
Thank you for cleaning up all whitespaces. We probably need a linter. But - nit: this line doesn't look right?
There was a problem hiding this comment.
In another view it's fine but here it's shown as
There was a problem hiding this comment.
Whoops, that's tabs vs.spaces, sorry about that, should be fixed now.
| if (split.length == 0) { | ||
| return ""; | ||
| } | ||
| return split[1]; |
There was a problem hiding this comment.
This will fail for length == 1. Unit tests, please.
sdk/java/pulumi/src/main/java/com/pulumi/provider/internal/infer/ComponentAnalyzer.java
Outdated
Show resolved
Hide resolved
| if (ComponentResource.class.isAssignableFrom(clazz) && !clazz.isInterface() && !java.lang.reflect.Modifier.isAbstract(clazz.getModifiers())) { | ||
| components.put(clazz.getSimpleName(), analyzer.analyzeComponent(clazz)); | ||
| } | ||
| } | ||
|
|
||
| return analyzer.generateSchema(metadata, components, analyzer.typeDefinitions); | ||
| // Java package names are not allowed to contain underscores, but namespaces expect dashes instead. | ||
| namespace = namespace.replaceAll("_", "-"); |
| for (Class<?> clazz : classes) { | ||
| if (!getNamespace(clazz).equals(namespace)) { | ||
| throw new IllegalArgumentException("All classes must be in the same top level package. Expected: " + namespace + " Found: " + getNamespace(clazz)); |
…er/ComponentAnalyzer.java Co-authored-by: Mikhail Shilkov <[email protected]>
| @@ -0,0 +1,13 @@ | |||
| package com; | |||
There was a problem hiding this comment.
Not sure if there's a better way to add these test classes other than checking these in? At least it's only in src/test so won't end up in the real package, so maybe it's okay?
Allow Java components to automatically infer the namespace based on the package name of the component.