Skip to content

Commit

Permalink
Make sets ordered
Browse files Browse the repository at this point in the history
  • Loading branch information
JordonPhillips committed Aug 16, 2021
1 parent 28faa49 commit 6084e16
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 20 deletions.
5 changes: 5 additions & 0 deletions docs/source/1.0/spec/core/constraint-traits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,11 @@ in a response.
``uniqueItems`` trait
---------------------

.. warning:
This trait has been deprecated. It may be removed in future versions. Set
shapes should be used instead.
Summary
Indicates that the items in a :ref:`list` MUST be unique.
Trait selector
Expand Down
11 changes: 9 additions & 2 deletions docs/source/1.0/spec/core/model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ reference other shapes using :ref:`members <member>`.
* - :ref:`list`
- Ordered collection of homogeneous values
* - :ref:`set`
- Unordered collection of unique homogeneous values
- Collection of unique homogeneous values
* - :ref:`map`
- Map data structure that maps string keys to homogeneous values
* - :ref:`structure`
Expand Down Expand Up @@ -565,7 +565,7 @@ example is ``smithy.example#MyList$member``.
Set
===

The :dfn:`set` type represents an unordered collection of unique homogeneous
The :dfn:`set` type represents a collection of unique homogeneous
values. A set shape requires a single member named ``member``.
Sets are defined in the IDL using a :ref:`set_statement <idl-set>`.
The following example defines a set of strings:
Expand Down Expand Up @@ -618,6 +618,13 @@ SHOULD represent sets as a custom set data structure that can interpret value
hash codes and equality, or alternatively, store the values of a set data
structure in a list and rely on validation to ensure uniqueness.

.. rubric:: Set member ordering

Sets MUST be insertion ordered. Not all programming languages that support
sets support ordered sets, requiring them may be overly burdensome for users,
or conflict with language idioms. Such languages SHOULD store the values
of sets in a list and rely on validation to ensure uniqueness.


.. _map:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ private Map<ShapeId, Trait> getAuthTraitValues(Shape service, Shape subject) {

AuthTrait authTrait = subject.expectTrait(AuthTrait.class);
Map<ShapeId, Trait> result = new LinkedHashMap<>();
for (ShapeId value : authTrait.getValues()) {
for (ShapeId value : authTrait.getValueSet()) {
service.findTrait(value).ifPresent(trait -> result.put(value, trait));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -174,9 +175,11 @@ private ReflectiveSupplier<Collection<Object>> createSupplier(Class<?> targetTyp
|| targetType.equals(ArrayList.class)
|| targetType.equals(Iterable.class)) {
return ArrayList::new;
} else if (targetType.equals(Set.class) || targetType.equals(HashSet.class)) {
} else if (targetType.equals(Set.class)
|| targetType.equals(HashSet.class)
|| targetType.equals(LinkedHashSet.class)) {
// Special casing for Set or HashSet.
return HashSet::new;
return LinkedHashSet::new;
} else if (Collection.class.isAssignableFrom(targetType)) {
return createSupplierFromReflection(targetType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,17 @@

package software.amazon.smithy.model.traits;

import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import software.amazon.smithy.model.FromSourceLocation;
import software.amazon.smithy.model.SourceLocation;
import software.amazon.smithy.model.node.ArrayNode;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.StringNode;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.SetUtils;

/**
* Specifies the auth schemes supported by default for operations
Expand All @@ -33,34 +35,50 @@ public final class AuthTrait extends AbstractTrait {

public static final ShapeId ID = ShapeId.from("smithy.api#auth");

private final List<ShapeId> values;
private final Set<ShapeId> values;

@Deprecated
public AuthTrait(List<ShapeId> values, FromSourceLocation sourceLocation) {
super(ID, sourceLocation);
this.values = ListUtils.copyOf(values);
this.values = SetUtils.orderedCopyOf(values);
}

@Deprecated
public AuthTrait(List<ShapeId> values) {
this(values, SourceLocation.NONE);
}

public AuthTrait(Set<ShapeId> values, FromSourceLocation sourceLocation) {
super(ID, sourceLocation);
this.values = SetUtils.orderedCopyOf(values);
}

public AuthTrait(Set<ShapeId> values) {
this(values, SourceLocation.NONE);
}

/**
* Gets the auth scheme trait values.
*
* @return Returns the supported auth schemes.
*/
public List<ShapeId> getValues() {
public Set<ShapeId> getValueSet() {
return values;
}

@Deprecated
public List<ShapeId> getValues() {
return ListUtils.copyOf(values);
}

public static final class Provider extends AbstractTrait.Provider {
public Provider() {
super(ID);
}

@Override
public Trait createTrait(ShapeId target, Node value) {
List<ShapeId> values = new ArrayList<>();
Set<ShapeId> values = new LinkedHashSet<>();
for (StringNode node : value.expectArrayNode().getElementsAs(StringNode.class)) {
values.add(node.expectShapeId());
}
Expand All @@ -70,7 +88,7 @@ public Trait createTrait(ShapeId target, Node value) {

@Override
protected Node createNode() {
return getValues().stream().map(ShapeId::toString).map(Node::from)
return getValueSet().stream().map(ShapeId::toString).map(Node::from)
.collect(ArrayNode.collect(getSourceLocation()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

package software.amazon.smithy.model.traits;

import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -121,12 +121,12 @@ public Builder maxAge(int maxAge) {
}

public Builder additionalAllowedHeaders(Set<String> additionalAllowedHeaders) {
this.additionalAllowedHeaders = new HashSet<>(Objects.requireNonNull(additionalAllowedHeaders));
this.additionalAllowedHeaders = new LinkedHashSet<>(Objects.requireNonNull(additionalAllowedHeaders));
return this;
}

public Builder additionalExposedHeaders(Set<String> additionalExposedHeaders) {
this.additionalExposedHeaders = new HashSet<>(Objects.requireNonNull(additionalExposedHeaders));
this.additionalExposedHeaders = new LinkedHashSet<>(Objects.requireNonNull(additionalExposedHeaders));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
/**
* Indicates that the members of a list must be unique.
*/
@Deprecated
public final class UniqueItemsTrait extends AnnotationTrait {
public static final ShapeId ID = ShapeId.from("smithy.api#uniqueItems");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private void validateShape(
) {
if (shape.getTrait(AuthTrait.class).isPresent()) {
AuthTrait authTrait = shape.getTrait(AuthTrait.class).get();
Set<ShapeId> appliedAuthTraitValue = new TreeSet<>(authTrait.getValues());
Set<ShapeId> appliedAuthTraitValue = new TreeSet<>(authTrait.getValueSet());
appliedAuthTraitValue.removeAll(serviceAuth);

if (!appliedAuthTraitValue.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ map externalDocumentation {

/// Defines the list of authentication schemes supported by a service or operation.
@trait(selector: ":is(service, operation)")
@uniqueItems
list auth {
set auth {
member: AuthTraitReference
}

Expand Down Expand Up @@ -502,6 +501,7 @@ structure sparse {}

/// Indicates that the items in a list MUST be unique.
@trait(selector: "list")
@deprecated(message: "The uniqueItems trait has been deprecated in favor of using sets.", since: "2.0")
structure uniqueItems {}

/// Indicates that the shape is unstable and could change in the future.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ public void deserializesSets() {
NodeMapper mapper = new NodeMapper();

Set<String> result = mapper.deserializeCollection(value, Set.class, String.class);
assertThat(result, instanceOf(HashSet.class));
assertThat(result, instanceOf(LinkedHashSet.class));
assertThat(result, contains("a", "b"));
}

Expand All @@ -794,7 +794,7 @@ public void deserializesHashSets() {
NodeMapper mapper = new NodeMapper();

Set<String> result = mapper.deserializeCollection(value, HashSet.class, String.class);
assertThat(result, instanceOf(HashSet.class));
assertThat(result, instanceOf(LinkedHashSet.class));
assertThat(result, contains("a", "b"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ private <T extends Trait> void addOperationSecurity(
// If the operation explicitly removes authentication, ensure that "security" is set to an empty
// list as opposed to simply being unset as unset will result in the operation inheriting global
// configuration.
if (shape.getTrait(AuthTrait.class).map(trait -> trait.getValues().isEmpty()).orElse(false)) {
if (shape.getTrait(AuthTrait.class).map(trait -> trait.getValueSet().isEmpty()).orElse(false)) {
builder.security(Collections.emptyList());
return;
}
Expand Down

0 comments on commit 6084e16

Please sign in to comment.