Skip to content
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

Various IDL 2.0 fixes and tweaks #1313

Merged
merged 4 commits into from
Jul 25, 2022
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void detectsUnknownConditionKeys() {

assertTrue(result.isBroken());
assertThat(result.getValidationEvents(Severity.ERROR).stream()
.map(ValidationEvent::getEventId)
.map(ValidationEvent::getId)
.collect(Collectors.toSet()),
contains("ConditionKeys"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ private Optional<ValidationEvent> validateAlgorithmMember(
return Optional.empty();
}

@SuppressWarnings("deprecation")
private Optional<ValidationEvent> validateEnumMember(
Model model,
HttpChecksumTrait trait,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import software.amazon.smithy.utils.StringUtils;

@SmithyInternalApi
@SuppressWarnings("deprecation")
public final class Upgrade1to2Command extends SimpleCommand {
private static final Logger LOGGER = Logger.getLogger(Upgrade1to2Command.class.getName());
private static final Pattern VERSION_1 = Pattern.compile("(?m)^\\s*\\$\\s*version:\\s*\"1\\.0\"\\s*$");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ public void supportsStringLengthTrait() {
}

@Test
@SuppressWarnings("deprecation")
public void supportsListAndSetLengthTrait() {
StringShape string = StringShape.builder().id("smithy.api#String").build();
MemberShape member = MemberShape.builder()
Expand Down Expand Up @@ -428,6 +429,7 @@ public void supportsDocumentation() {
}

@Test
@SuppressWarnings("deprecation")
public void supportsEnum() {
StringShape string = StringShape.builder()
.id("smithy.api#String")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ default SourceLocation getSourceLocation() {
*
* @param s1 the first FromSourceLocation to compare.
* @param s2 the second FromSourceLocation to compare.
* @return the value 0 if s1 == s2; a value less than 0 if s1 < s2; and a value greater than 0 if s1 > s2.
* @return the value 0 if s1 == s2; a value less than 0 if s1 &lt; s2; and a value greater than 0 if s1 &gt; s2.
*/
static int compare(FromSourceLocation s1, FromSourceLocation s2) {
return s1.getSourceLocation().compareTo(s2.getSourceLocation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,55 @@

import java.lang.ref.WeakReference;
import java.util.Objects;
import java.util.Set;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.loader.ModelAssembler;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.shapes.ToShapeId;
import software.amazon.smithy.model.traits.BoxTrait;
import software.amazon.smithy.model.traits.ClientOptionalTrait;
import software.amazon.smithy.model.traits.DefaultTrait;
import software.amazon.smithy.model.traits.InputTrait;
import software.amazon.smithy.model.traits.RequiredTrait;
import software.amazon.smithy.model.traits.SparseTrait;
import software.amazon.smithy.utils.SetUtils;

/**
* An index that checks if a member is nullable.
*
* <p>Note: this index assumes Smithy 2.0 nullability semantics.
* There is basic support for detecting 1.0 models by detecting
* when a removed primitive prelude shape is targeted by a member.
* Beyond that, 1.0 models SHOULD be loaded through a {@link ModelAssembler}
* to upgrade them to IDL 2.0.
*/
public class NullableIndex implements KnowledgeIndex {

private static final Set<ShapeId> V1_REMOVED_PRIMITIVE_SHAPES = SetUtils.of(
ShapeId.from("smithy.api#PrimitiveBoolean"),
ShapeId.from("smithy.api#PrimitiveByte"),
ShapeId.from("smithy.api#PrimitiveShort"),
ShapeId.from("smithy.api#PrimitiveInteger"),
ShapeId.from("smithy.api#PrimitiveLong"),
ShapeId.from("smithy.api#PrimitiveFloat"),
ShapeId.from("smithy.api#PrimitiveDouble"));

private static final Set<ShapeType> V1_INHERENTLY_BOXED = SetUtils.of(
ShapeType.STRING,
ShapeType.BLOB,
ShapeType.TIMESTAMP,
ShapeType.BIG_DECIMAL,
ShapeType.BIG_INTEGER,
ShapeType.LIST,
ShapeType.SET,
mtdowling marked this conversation as resolved.
Show resolved Hide resolved
ShapeType.MAP,
ShapeType.STRUCTURE,
ShapeType.UNION,
ShapeType.DOCUMENT);

private final WeakReference<Model> model;

public NullableIndex(Model model) {
Expand Down Expand Up @@ -106,14 +140,20 @@ public boolean isMemberNullable(MemberShape member) {
}

/**
* Checks if a member is nullable.
* Checks if a member is nullable using v2 nullability rules.
*
* <p>A {@code checkMode} parameter is required to declare what kind of
* model consumer is checking if the member is optional. The authoritative
* consumers like servers do not need to honor the {@link InputTrait} or
* {@link ClientOptionalTrait}, while non-authoritative consumers like clients
* must honor these traits.
*
* <p>This method will also attempt to detect when a member targets a
* primitive prelude shape that was removed in Smithy IDL 2.0 to account
* for models that were created manually without passing through a
* ModelAssembler. If a member targets a removed primitive prelude shape,
* the member is considered non-null.
*
* @param member Member to check.
* @param checkMode The mode used when checking if the member is considered nullable.
* @return Returns true if the member is optional.
Expand All @@ -122,16 +162,23 @@ public boolean isMemberNullable(MemberShape member, CheckMode checkMode) {
Model m = Objects.requireNonNull(model.get());
Shape container = m.expectShape(member.getContainer());

// Client mode honors the nullable and input trait.
if (checkMode == CheckMode.CLIENT
&& (member.hasTrait(ClientOptionalTrait.class) || container.hasTrait(InputTrait.class))) {
return true;
}

switch (container.getType()) {
case STRUCTURE:
// Structure members are nullable by default; non-null when marked as @default / @required.
return !member.hasTrait(DefaultTrait.class) && !member.hasTrait(RequiredTrait.class);
// Client mode honors the nullable and input trait.
if (checkMode == CheckMode.CLIENT
&& (member.hasTrait(ClientOptionalTrait.class) || container.hasTrait(InputTrait.class))) {
return true;
}

// Structure members that are @required or @default are not nullable.
if (member.hasTrait(DefaultTrait.class) || member.hasTrait(RequiredTrait.class)) {
return false;
}

// Detect if the member targets a 1.0 primitive prelude shape and the shape wasn't upgraded.
// These removed prelude shapes are impossible to appear in a 2.0 model, so it's safe to
// detect them and honor 1.0 semantics here.
return !V1_REMOVED_PRIMITIVE_SHAPES.contains(member.getTarget());
case UNION:
case SET:
// Union and set members are never null.
Expand All @@ -149,4 +196,43 @@ public boolean isMemberNullable(MemberShape member, CheckMode checkMode) {
return false;
}
}

/**
* Checks if a member is nullable using v1 nullability rules.
*
* <p>This method matches the previous behavior seen in NullableIndex prior
* to Smithy 1.0. Most models are sent through a ModelAssembler which makes
* using the normal {@link #isMemberNullable(MemberShape)} the best choice.
* However, in some cases, a model might get created directly in code
* using Smithy 1.0 semantics. In those cases, this method can be used to
* detect if the member is nullable or not.
*
* <p>This method ignores the default trait, clientOptional trait,
* input trait, and required trait.
*
* @param member Member to check.
* @return Returns true if the member is nullable using 1.0 resolution rules.
*/
public boolean isMemberNullableInV1(MemberShape member) {
Model m = Objects.requireNonNull(model.get());
Shape container = m.getShape(member.getContainer()).orElse(null);
Shape target = m.getShape(member.getTarget()).orElse(null);

// Ignore broken models in this index. Other validators handle these checks.
if (container == null || target == null) {
return false;
}

// Defer to 2.0 checks for shapes that aren't structures, since the logic is the same.
if (container.getType() != ShapeType.STRUCTURE) {
return isMemberNullable(member);
}

// Check if the member or the target has the box trait.
if (member.getMemberTrait(m, BoxTrait.class).isPresent()) {
return true;
}

return V1_INHERENTLY_BOXED.contains(target.getType());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,8 @@ ValidatedResult<Model> transform() {
// Also attempt to upgrade unknown versions since they could be 1.0 and
// trying to upgrade 2.0 shapes has no effect.
// For v1 shape checks, we need to know the containing shape type to apply the appropriate transform.
model.getShape(member.getContainer()).ifPresent(container -> {
upgradeV1Member(container.getType(), member);
});
model.getShape(member.getContainer())
.ifPresent(container -> upgradeV1Member(container.getType(), member));
}
}

Expand Down Expand Up @@ -182,6 +181,7 @@ private boolean isZeroValidDefault(MemberShape member) {
return true;
}

@SuppressWarnings("deprecation")
private boolean shouldV1MemberHaveDefaultTrait(ShapeType containerType, MemberShape member, Shape target) {
return containerType == ShapeType.STRUCTURE
// Only when the targeted shape had a default value by default in v1 or if
Expand Down Expand Up @@ -210,6 +210,7 @@ private MemberShape.Builder createOrReuseBuilder(MemberShape member, MemberShape
return builder == null ? member.toBuilder() : builder;
}

@SuppressWarnings("deprecation")
private void validateV2Member(MemberShape member) {
if (REMOVED_PRIMITIVE_SHAPES.containsKey(member.getTarget())) {
emitWhenTargetingRemovedPreludeShape(Severity.ERROR, member);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Consumer;
import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.traits.Trait;
Expand All @@ -29,10 +29,18 @@
public abstract class CollectionShape extends Shape {

private final MemberShape member;
private transient Map<String, MemberShape> memberMap;

CollectionShape(Builder<?, ?> builder) {
super(builder, false);
member = builder.member != null ? builder.member : getRequiredMixinMember(builder, "member");

// Get the member from the builder or from any mixins.
if (builder.member != null) {
member = builder.member;
} else {
member = getRequiredMixinMember(builder, "member");
}

ShapeId expected = getId().withMember("member");
if (!member.getId().equals(expected)) {
throw new IllegalArgumentException(String.format(
Expand All @@ -51,18 +59,13 @@ public final MemberShape getMember() {
}

@Override
public final Optional<MemberShape> getMember(String name) {
mtdowling marked this conversation as resolved.
Show resolved Hide resolved
return name.equals("member") ? Optional.of(member) : Optional.empty();
}

@Override
public Collection<MemberShape> members() {
return Collections.singletonList(member);
}

@Override
public boolean equals(Object other) {
return super.equals(other) && getMember().equals(((CollectionShape) other).getMember());
public final Map<String, MemberShape> getAllMembers() {
Map<String, MemberShape> members = memberMap;
if (members == null) {
members = Collections.singletonMap("member", member);
memberMap = members;
}
return members;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer;
Expand All @@ -35,15 +34,13 @@
import software.amazon.smithy.model.traits.UnitTypeTrait;
import software.amazon.smithy.model.traits.synthetic.SyntheticEnumTrait;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.ListUtils;

public final class EnumShape extends StringShape implements NamedMembers {
public final class EnumShape extends StringShape {

private static final Pattern CONVERTABLE_VALUE = Pattern.compile("^[a-zA-Z-_.:/\\s]+[a-zA-Z_0-9-.:/\\s]*$");
private static final Logger LOGGER = Logger.getLogger(EnumShape.class.getName());

private final Map<String, MemberShape> members;
private volatile List<String> memberNames;
private volatile Map<String, String> enumValues;

private EnumShape(Builder builder) {
Expand Down Expand Up @@ -82,58 +79,15 @@ public Map<String, MemberShape> getAllMembers() {
return members;
}

/**
* Returns an ordered list of member names based on the order they are
* defined in the model, including mixin members.
*
* @return Returns an immutable list of member names.
*/
@Override
public List<String> getMemberNames() {
List<String> names = memberNames;
if (names == null) {
names = ListUtils.copyOf(members.keySet());
memberNames = names;
}

return names;
}

@Override
public boolean equals(Object other) {
if (!super.equals(other)) {
return false;
}

// Members are ordered, so do a test on the ordering and their values.
EnumShape b = (EnumShape) other;
return getMemberNames().equals(b.getMemberNames()) && members.equals(b.members);
}

@Override
@SuppressWarnings("deprecation")
public Optional<Trait> findTrait(ShapeId id) {
if (id.equals(EnumTrait.ID)) {
return super.findTrait(SyntheticEnumTrait.ID);
}
return super.findTrait(id);
}

/**
* Get a specific member by name.
*
* @param name Name of the member to retrieve.
* @return Returns the optional member.
*/
@Override
public Optional<MemberShape> getMember(String name) {
return Optional.ofNullable(members.get(name));
}

@Override
public Collection<MemberShape> members() {
return members.values();
}

public static Builder builder() {
return new Builder();
}
Expand Down
Loading