Skip to content

Commit

Permalink
fix APIGW exports nullability exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
Steven Yuan authored and sbiscigl committed Apr 7, 2023
1 parent 30383d5 commit 27360c1
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 8 deletions.
8 changes: 8 additions & 0 deletions .changelog/1e8147ce9c59435aaab11ac917d0dfb2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "1e8147ce-9c59-435a-aab1-1ac917d0dfb2",
"type": "bugfix",
"description": "Fix APIGW exports nullability exceptions strictness.",
"modules": [
"codegen"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.logging.Logger;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -147,7 +148,12 @@ private void patchDefaultsForRootLevelSnapshottedShapes(
Set<NumberShape> numberShapes,
Set<MemberShape> memberShapes) {
for (ShapeId shapeId : nullabilityExceptions) {
Shape shape = model.expectShape(shapeId);
Optional<Shape> shapeOptional = model.getShape(shapeId);
if (!shapeOptional.isPresent()) {
LOGGER.severe("ShapeId `" + shapeId.toString() + "` is not present in the model");
continue;
}
Shape shape = shapeOptional.get();
if (shape.hasTrait(DefaultTrait.class)) {
continue;
}
Expand Down Expand Up @@ -176,7 +182,12 @@ private void patchDefaultsForMemberLevelSnapshottedShapes(
Set<NumberShape> numberShapes,
Set<MemberShape> memberShapes) {
for (ShapeId shapeId : nullabilityExceptions) {
Shape shape = model.expectShape(shapeId);
Optional<Shape> shapeOptional = model.getShape(shapeId);
if (!shapeOptional.isPresent()) {
LOGGER.severe("ShapeId `" + shapeId.toString() + "` is not present in the model");
continue;
}
Shape shape = shapeOptional.get();
if (shape.hasTrait(DefaultTrait.class)) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,8 @@
"com.amazonaws.mediaconvert#Vp9Settings$MaxBitrate",
"com.amazonaws.mediaconvert#Vp9Settings$ParDenominator",
"com.amazonaws.mediaconvert#Vp9Settings$ParNumerator",
"com.amazonaws.mediaconvert#WarningGroup$Code",
"com.amazonaws.mediaconvert#WarningGroup$Count",
"com.amazonaws.mediaconvert#WavSettings$BitDepth",
"com.amazonaws.mediaconvert#WavSettings$Channels",
"com.amazonaws.mediaconvert#WavSettings$SampleRate",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
Expand All @@ -50,6 +52,8 @@
import software.amazon.smithy.utils.MapUtils;

public class ApiGatewayExportsNullabilityExceptionIntegrationTest {
private static final Logger LOGGER = Logger
.getLogger(ApiGatewayExportsNullabilityExceptionIntegrationTest.class.getName());
private static final String PATH_PREFIX = "../sdk-codegen/aws-models/";
private static final String NULLABILITY_EXCEPTIONS_FILE = "APIGW_exports_nullability_exceptions.json";

Expand All @@ -59,7 +63,12 @@ public class ApiGatewayExportsNullabilityExceptionIntegrationTest {
@ParameterizedTest
@MethodSource("apigwNullabilityExceptionServices")
public void test_APIGW_exports_nullability_exception_services(String modelFile) {
loadPreprocessedModel(modelFile);
try {
loadPreprocessedModel(modelFile);
} catch (Exception e) {
LOGGER.severe(e.getMessage());
throw e;
}
}

private static Stream<Arguments> apigwNullabilityExceptionServices() {
Expand Down Expand Up @@ -144,6 +153,7 @@ public void test_unaffected_MediaLive_APIGW_default_strings() {
bodyShape.expectTrait(DefaultTrait.class).toNode().asStringNode().get().getValue());
}

/*
@Test
public void test_missing_snapshotted_root_level_shape() {
ApiGatewayExportsNullabilityExceptionIntegration integration = new ApiGatewayExportsNullabilityExceptionIntegration();
Expand All @@ -162,7 +172,9 @@ public void test_missing_snapshotted_root_level_shape() {
ShapeId.from("com.amazonaws.pinpointsmsvoice#RenamedBoolean")));
assertThrows(ExpectationNotMetException.class, () -> integration.preprocessModel(renamedShapesModel, settings));
}
*/

/*
@Test
public void test_missing_snapshotted_member_level_shape() {
ApiGatewayExportsNullabilityExceptionIntegration integration = new ApiGatewayExportsNullabilityExceptionIntegration();
Expand All @@ -180,6 +192,7 @@ public void test_missing_snapshotted_member_level_shape() {
model.expectShape(ShapeId.from("com.amazonaws.pinpointsmsvoice#EventDestination$Enabled"))));
assertThrows(ExpectationNotMetException.class, () -> integration.preprocessModel(removedShapesModel, settings));
}
*/

@Test
public void test_identify_nonsnapshotted_member_level_shape() {
Expand Down Expand Up @@ -242,11 +255,16 @@ private Model stripDefaultsFromModel(Model model, ShapeId service) {
List<Shape> shapesToReplace = new ArrayList<>();
// Strip root shapes
for (ShapeId shapeId : shapeIdsToReplace) {
Shape shape = model.expectShape(shapeId);
if (shape.hasTrait(DefaultTrait.class)) {
shapesToReplace.add(Shape.shapeToBuilder(shape)
.removeTrait(DefaultTrait.ID)
.build());
// TODO: clean this up later
Optional<Shape> shape = model.getShape(shapeId);
if (shape.isPresent()) {
if (shape.get().hasTrait(DefaultTrait.class)) {
shapesToReplace.add(Shape.shapeToBuilder(shape.get())
.removeTrait(DefaultTrait.ID)
.build());
}
} else {
LOGGER.severe("ShapeId `" + shapeId.toString() + "` is not present in the model");
}
}
// Strip member shapes that target affected root shapes
Expand All @@ -266,6 +284,9 @@ private Model stripDefaultsFromModel(Model model, ShapeId service) {
Model strippedModel = ModelTransformer.create().replaceShapes(model, shapesToReplace);
// Assert root shape defaults are removed
for (ShapeId shapeId : shapeIdsToReplace) {
if (!strippedModel.getShape(shapeId).isPresent()) {
continue;
}
assertFalse(strippedModel.expectShape(shapeId).hasTrait(DefaultTrait.class));
}
// Assert member shape defaults are removed
Expand Down

0 comments on commit 27360c1

Please sign in to comment.