Skip to content

Use partitions.json to generate Region class #6120

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
6 changes: 6 additions & 0 deletions .changes/next-release/feature-AWSSDKforJavav2-7e045dc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "feature",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "Updated Region class generation to use Partitions.json instead of the Endpoints.json and removed the hardcoded global regions."
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,24 @@ public class RegionGenerationMojo extends AbstractMojo {
"${basedir}/src/main/resources/software/amazon/awssdk/regions/internal/region/endpoints.json")
private File endpoints;

@Parameter(property = "partitionsjson", defaultValue =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the j is lower case? (ie, why not partitionsJson?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my bad. I'll update it.

"${basedir}/src/main/resources/software/amazon/awssdk/regions/internal/region/partitions.json.resource")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the same file in codegen? Otherwise, we'd need to update our release system to keep it updated and if it's not possible, I'd prefer to keep the file in codegen-lite instead of core since we don't want anyone to use it or load it at runtime.

private File partitionsjson;

@Override
public void execute() throws MojoExecutionException {
Path baseSourcesDirectory = Paths.get(outputDirectory).resolve("generated-sources").resolve("sdk");
Path testsDirectory = Paths.get(outputDirectory).resolve("generated-test-sources").resolve("sdk-tests");

Partitions partitions = RegionMetadataLoader.build(endpoints);
Partitions regionpartitions = RegionMetadataLoader.build(partitionsjson);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here - is there a reason not to name this regionPartitions?

Alternative naming thought - rename partitions above to legacyPartitions and the new one to just partitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's a good idea. I'll rename the partitions above to legacyPartitions and the new one to just partitions.


generatePartitionMetadataClass(baseSourcesDirectory, partitions);
generateRegionClass(baseSourcesDirectory, partitions);
generateRegionClass(baseSourcesDirectory, regionpartitions);
generateServiceMetadata(baseSourcesDirectory, partitions);
generateRegions(baseSourcesDirectory, partitions);
generateRegions(baseSourcesDirectory, regionpartitions);
generatePartitionProvider(baseSourcesDirectory, partitions);
generateRegionProvider(baseSourcesDirectory, partitions);
generateRegionProvider(baseSourcesDirectory, regionpartitions);
generateServiceProvider(baseSourcesDirectory, partitions);
generateEndpointTags(baseSourcesDirectory, partitions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,21 +100,15 @@ private void regions(TypeSpec.Builder builder) {
.add("$T.unmodifiableList($T.asList(", Collections.class, Arrays.class);

String regionsCodeBlock = regions.stream().map(r -> {
boolean isGlobal = r.contains("global");
builder.addField(FieldSpec.builder(className(), regionName(r))
.addModifiers(PUBLIC, STATIC, FINAL)
.initializer("$T.of($S)", className(), r)
.initializer(isGlobal ? "$T.of($S, true)" : "$T.of($S)", className(), r)
.build());
return regionName(r);
}).collect(Collectors.joining(", "));

addGlobalRegions(builder);

regionsArray.add(regionsCodeBlock + ", ")
.add("AWS_GLOBAL, ")
.add("AWS_CN_GLOBAL, ")
.add("AWS_US_GOV_GLOBAL, ")
.add("AWS_ISO_GLOBAL, ")
.add("AWS_ISO_B_GLOBAL");
regionsArray.add(regionsCodeBlock);
regionsArray.add("))");

TypeName listOfRegions = ParameterizedTypeName.get(ClassName.get(List.class), className());
Expand All @@ -123,29 +117,6 @@ private void regions(TypeSpec.Builder builder) {
.initializer(regionsArray.build()).build());
}

private void addGlobalRegions(TypeSpec.Builder builder) {
builder.addField(FieldSpec.builder(className(), "AWS_GLOBAL")
.addModifiers(PUBLIC, STATIC, FINAL)
.initializer("$T.of($S, true)", className(), "aws-global")
.build())
.addField(FieldSpec.builder(className(), "AWS_CN_GLOBAL")
.addModifiers(PUBLIC, STATIC, FINAL)
.initializer("$T.of($S, true)", className(), "aws-cn-global")
.build())
.addField(FieldSpec.builder(className(), "AWS_US_GOV_GLOBAL")
.addModifiers(PUBLIC, STATIC, FINAL)
.initializer("$T.of($S, true)", className(), "aws-us-gov-global")
.build())
.addField(FieldSpec.builder(className(), "AWS_ISO_GLOBAL")
.addModifiers(PUBLIC, STATIC, FINAL)
.initializer("$T.of($S, true)", className(), "aws-iso-global")
.build())
.addField(FieldSpec.builder(className(), "AWS_ISO_B_GLOBAL")
.addModifiers(PUBLIC, STATIC, FINAL)
.initializer("$T.of($S, true)", className(), "aws-iso-b-global")
.build());
}

private String regionName(String region) {
return region.replace("-", "_").toUpperCase(Locale.US);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ public TypeSpec poetClass() {
.addModifiers(FINAL)
.addSuperinterface(ClassName.get(regionBasePackage, "RegionMetadata"))
.addField(staticFinalField("ID", region))
.addField(staticFinalField("DOMAIN", partition.getDnsSuffix()))
.addField(staticFinalField("DOMAIN", partition.getOutputs().getDnsSuffix()))
.addField(staticFinalField("DESCRIPTION", regionDescription))
.addField(staticFinalField("PARTITION_ID", partition.getPartition()))
.addField(staticFinalField("PARTITION_ID", partition.getId()))
.addMethod(getter("id", "ID"))
.addMethod(getter("domain", "DOMAIN"))
.addMethod(getter("description", "DESCRIPTION"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,61 @@ public final class Partition {
*/
private Endpoint defaults;

/**
* The partition id.
*/
private String id;

/**
* Configuration outputs for the partition.
*/
private PartitionOutputs outputs;

public Partition() {
}

public Partition(@JsonProperty(value = "partition") String partition,
@JsonProperty(value = "regions") Map<String, PartitionRegion>
regions,
@JsonProperty(value = "services") Map<String,
Service> services) {
Service> services,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like its a little weird to squish both the legacy endponts.json partitions and the new partitions metadata (partitions.json) model together - How much of a refactor would be required to add a new PartitionsMetadata model that models only the new partitions.json?

I think that would make the code in RegionGenerationMojo more clear as well (since its not clear to me which of the generation functions require which version of partitions).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, creating a separate PartitionsMetadata model would be more clear. However, the main issue is that some generation functions like generateServiceProvider and generateEndpointTags still require data from endpoints.json, as services and endpoints aren't present in partitions.json. So for now, using partitions.json only for region generation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense - I'm not suggesting that we completly replace the existing Partition model since its still required. I'm just suggesting that we create a new PartitionsMetadata model and use it in the places where we can. That way we don't have Partition objects that have missing/incomplete data (eg, in the current code, regionpartitions will have empty service maps, since partitions.json doesn't have that information).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, got it. I'll make that change.

@JsonProperty(value = "id") String id,
@JsonProperty(value = "outputs") PartitionOutputs outputs,
@JsonProperty(value = "regionRegex") String regionRegex) {
this.partition = Validate.paramNotNull(partition, "Partition");
this.regions = regions;
this.services = services;
this.id = id;
this.outputs = outputs;
this.regionRegex = regionRegex;
}

/**
* Returns the partition ID.
*/
public String getId() {
return id;
}

/**
* Sets the partition id.
*/
public void setId(String id) {
this.id = id;
}

/**
* Returns the configuration outputs for the partition.
*/
public PartitionOutputs getOutputs() {
return outputs;
}

/**
* Sets the configuration outputs for the partition.
*/
public void setOutputs(PartitionOutputs outputs) {
this.outputs = outputs;
}

/**
Expand Down Expand Up @@ -177,6 +221,82 @@ private boolean matchesRegionRegex(String region) {
return p.matcher(region).matches();
}

public static class PartitionOutputs {
private String dnsSuffix;
private String dualStackDnsSuffix;
private String implicitGlobalRegion;
private String name;
private boolean supportsDualStack;
private boolean supportsFIPS;

@JsonProperty("dnsSuffix")
public String getDnsSuffix() {
return dnsSuffix;
}

public void setDnsSuffix(String dnsSuffix) {
this.dnsSuffix = dnsSuffix;
}

@JsonProperty("dualStackDnsSuffix")
public String getDualStackDnsSuffix() {
return dualStackDnsSuffix;
}

public void setDualStackDnsSuffix(String dualStackDnsSuffix) {
this.dualStackDnsSuffix = dualStackDnsSuffix;
}

@JsonProperty("implicitGlobalRegion")
public String getImplicitGlobalRegion() {
return implicitGlobalRegion;
}

public void setImplicitGlobalRegion(String implicitGlobalRegion) {
this.implicitGlobalRegion = implicitGlobalRegion;
}

@JsonProperty("name")
public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

@JsonProperty("supportsDualStack")
public boolean getSupportsDualStack() {
return supportsDualStack;
}

public void setSupportsDualStack(boolean supportsDualStack) {
this.supportsDualStack = supportsDualStack;
}

@JsonProperty("supportsFIPS")
public boolean getSupportsFIPS() {
return supportsFIPS;
}

public void setSupportsFIPS(boolean supportsFIPS) {
this.supportsFIPS = supportsFIPS;
}
}

public static class Region {
private String description;

@JsonProperty("description")
public String getDescription() {
return description;
}

public void setDescription(String description) {
this.description = description;
}
}

@Deprecated
private boolean hasServiceEndpoint(String endpoint) {
for (Service s : services.values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,35 @@
public class RegionGenerationTest {

private static final String ENDPOINTS = "/software/amazon/awssdk/codegen/lite/test-endpoints.json";
private static final String PARTITIONS = "/software/amazon/awssdk/codegen/lite/test-partitions.json.resource";
private static final String SERVICE_METADATA_BASE = "software.amazon.awssdk.regions.servicemetadata";
private static final String REGION_METADATA_BASE = "software.amazon.awssdk.regions.regionmetadata";
private static final String PARTITION_METADATA_BASE = "software.amazon.awssdk.regions.partitionmetadata";
private static final String REGION_BASE = "software.amazon.awssdk.regions";

private File endpoints;
private File partitionsFile;
private Partitions partitions;
private Partitions partitionsRegions;


@BeforeEach
public void before() throws Exception {
this.endpoints = Paths.get(getClass().getResource(ENDPOINTS).toURI()).toFile();
this.partitionsFile = Paths.get(getClass().getResource(PARTITIONS).toURI()).toFile();
this.partitions = RegionMetadataLoader.build(endpoints);
this.partitionsRegions = RegionMetadataLoader.build(partitionsFile);
}

@Test
public void regionClass() {
RegionGenerator regions = new RegionGenerator(partitions, REGION_BASE);
RegionGenerator regions = new RegionGenerator(partitionsRegions, REGION_BASE);
assertThat(regions, generatesTo("regions.java"));
}

@Test
public void regionMetadataClass() {
Partition partition = partitions.getPartitions().get(0);
Partition partition = partitionsRegions.getPartitions().get(0);
RegionMetadataGenerator metadataGenerator = new RegionMetadataGenerator(partition,
"us-east-1",
"US East (N. Virginia)",
Expand Down
Loading
Loading