Skip to content

Commit

Permalink
Refactor hierarchy of shapes in IDL 2
Browse files Browse the repository at this point in the history
Rather than expose a NamedMembers interface, I think it makes more
sense to just move all member accessors to Shape. This gets rid
of a lot of cruft when trying to deal with different kinds of shapes
(for example, if you want to write something that works with both
structures and unions).

Furthermore, structurallyExclusive only impacts structure shapes.
This fixes an issue where it was implemented to also check enums,
intEnums, and unions.
  • Loading branch information
mtdowling committed Jul 22, 2022
1 parent b278e18 commit 45c42cf
Show file tree
Hide file tree
Showing 13 changed files with 306 additions and 431 deletions.
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,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) {
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,17 @@
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;
import software.amazon.smithy.model.SourceException;
import software.amazon.smithy.model.traits.EnumValueTrait;
import software.amazon.smithy.model.traits.UnitTypeTrait;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.ListUtils;

public final class IntEnumShape extends IntegerShape implements NamedMembers {
public final class IntEnumShape extends IntegerShape {

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

private IntEnumShape(Builder builder) {
Expand Down Expand Up @@ -70,50 +67,6 @@ 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.
IntEnumShape b = (IntEnumShape) other;
return getMemberNames().equals(b.getMemberNames()) && members.equals(b.members);
}

/**
* 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
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,20 @@

package software.amazon.smithy.model.shapes;

import java.util.AbstractMap;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.Pair;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.ToSmithyBuilder;

/**
Expand All @@ -32,6 +38,7 @@ public final class MapShape extends Shape implements ToSmithyBuilder<MapShape> {

private final MemberShape key;
private final MemberShape value;
private transient Map<String, MemberShape> memberMap;

private MapShape(Builder builder) {
super(builder, false);
Expand Down Expand Up @@ -83,32 +90,60 @@ public MemberShape getKey() {
}

@Override
public Collection<MemberShape> members() {
return ListUtils.of(key, value);
}
public Map<String, MemberShape> getAllMembers() {
Map<String, MemberShape> result = memberMap;

// Create a two-entry map that only knows about "key" and "value".
if (result == null) {
result = new AbstractMap<String, MemberShape>() {
private transient Set<Entry<String, MemberShape>> entries;
private transient List<MemberShape> values;

@Override
public MemberShape get(Object keyName) {
if ("key".equals(keyName)) {
return key;
} else if ("value".equals(keyName)) {
return value;
} else {
return null;
}
}

@Override
public Optional<MemberShape> getMember(String name) {
switch (name) {
case "key":
return Optional.of(key);
case "value":
return Optional.of(value);
default:
return Optional.empty();
}
}
@Override
public boolean containsKey(Object keyName) {
return "key".equals(keyName) || "value".equals(keyName);
}

@Override
public boolean equals(Object other) {
if (!super.equals(other)) {
return false;
} else {
MapShape otherShape = (MapShape) other;
return super.equals(otherShape)
&& getKey().equals(otherShape.getKey())
&& getValue().equals(((MapShape) other).getValue());
@Override
public Set<String> keySet() {
return SetUtils.of("key", "value");
}

@Override
public Collection<MemberShape> values() {
List<MemberShape> result = values;
if (result == null) {
result = ListUtils.of(key, value);
values = result;
}
return result;
}

@Override
public Set<Entry<String, MemberShape>> entrySet() {
Set<Entry<String, MemberShape>> result = entries;
if (result == null) {
result = SetUtils.of(Pair.of("key", key), Pair.of("value", value));
entries = result;
}
return result;
}
};
memberMap = result;
}

return result;
}

/**
Expand Down

This file was deleted.

Loading

0 comments on commit 45c42cf

Please sign in to comment.