Skip to content

POC of merging map traits #2936

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.

Sign up for GitHub

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

Closed
robin-aws wants to merge 1 commit into from
Closed

POC of merging map traits #2936

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 23 additions & 9 deletions smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderTraitMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static software.amazon.smithy.model.validation.Severity.ERROR;
import static software.amazon.smithy.model.validation.Validator.MODEL_ERROR;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -21,6 +22,7 @@
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.shapes.AbstractShapeBuilder;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.traits.DynamicTrait;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.traits.TraitDefinition;
Expand All @@ -35,7 +37,7 @@ final class LoaderTraitMap {
private static final String UNRESOLVED_TRAIT_SUFFIX = ".UnresolvedTrait";

private final TraitFactory traitFactory;
private final Map<ShapeId, Map<ShapeId, Node>> traits = new HashMap<>();
private final Map<ShapeId, Map<ShapeId, List<Node>>> traits = new HashMap<>();
private final List<ValidationEvent> events;
private final boolean allowUnknownTraits;
private final Map<ShapeId, Map<ShapeId, Trait>> unclaimed = new HashMap<>();
Expand All @@ -48,7 +50,7 @@ final class LoaderTraitMap {
}

void applyTraitsToNonMixinsInShapeMap(LoaderShapeMap shapeMap, List<ShapeId> undefinedTraits) {
for (Map.Entry<ShapeId, Map<ShapeId, Node>> entry : traits.entrySet()) {
for (Map.Entry<ShapeId, Map<ShapeId, List<Node>>> entry : traits.entrySet()) {
ShapeId target = entry.getKey();
ShapeId root = target.withoutMember();

Expand All @@ -60,9 +62,10 @@ void applyTraitsToNonMixinsInShapeMap(LoaderShapeMap shapeMap, List<ShapeId> und
? shapeMap.get(root)
: Collections::emptyIterator;

for (Map.Entry<ShapeId, Node> traitEntry : entry.getValue().entrySet()) {
for (Map.Entry<ShapeId, List<Node>> traitEntry : entry.getValue().entrySet()) {
ShapeId traitId = traitEntry.getKey();
Node traitNode = traitEntry.getValue();
List<Node> traitNodes = traitEntry.getValue();
Node traitNode = mergeAllTraitValues(target, traitId, shapeMap.getShapeType(traitId), traitNodes);
Trait created = createTrait(target, traitId, traitNode);
validateTraitIsKnown(target, traitId, created, traitNode.getSourceLocation(), shapeMap);
validateTraitWithTraitDefinition(traitId, undefinedTraits);
Expand Down Expand Up @@ -147,7 +150,7 @@ private void validateTraitIsKnown(
}

private void validateTraitWithTraitDefinition(ShapeId traitId, List<ShapeId> undefinedTraits) {
Map<ShapeId, Node> appliedTraits = traits.get(traitId);
Map<ShapeId, List<Node>> appliedTraits = traits.get(traitId);
if (appliedTraits == null || !appliedTraits.containsKey(TraitDefinition.ID)) {
undefinedTraits.add(traitId);
}
Expand Down Expand Up @@ -202,9 +205,8 @@ void add(LoadOperation.ApplyTrait operation) {
.message(message)
.build());
} else {
Map<ShapeId, Node> current = traits.computeIfAbsent(operation.target, id -> new LinkedHashMap<>());
Node previous = current.get(operation.trait);
current.put(operation.trait, mergeTraits(operation.target, operation.trait, previous, operation.value));
Map<ShapeId, List<Node>> current = traits.computeIfAbsent(operation.target, id -> new LinkedHashMap<>());
current.computeIfAbsent(operation.trait, id -> new ArrayList<>()).add(operation.value);
}
}
}
Expand All @@ -225,7 +227,15 @@ private boolean isAppliedToPreludeOutsidePrelude(LoadOperation.ApplyTrait operat
&& operation.target.getNamespace().equals(Prelude.NAMESPACE);
}

private Node mergeTraits(ShapeId target, ShapeId traitId, Node previous, Node updated) {
private Node mergeAllTraitValues(ShapeId target, ShapeId traitId, ShapeType traitType, List<Node> nodes) {
Node result = null;
for (Node node : nodes) {
result = mergeTraits(target, traitId, traitType, result, node);
}
return result;
}

private Node mergeTraits(ShapeId target, ShapeId traitId, ShapeType traitType, Node previous, Node updated) {
if (previous == null) {
return updated;
}
Expand All @@ -247,6 +257,10 @@ private Node mergeTraits(ShapeId target, ShapeId traitId, Node previous, Node up
if (previous.isArrayNode() && updated.isArrayNode()) {
// You can merge trait arrays.
return previous.expectArrayNode().merge(updated.expectArrayNode());
} else if (ShapeType.MAP.equals(traitType)) {
// You can merge trait maps (if the trait type is known in the first place).
// TODO: validation events for any overlapping keys
return previous.expectObjectNode().merge(updated.expectObjectNode());
} else if (previous.equals(updated)) {
LOGGER.fine(() -> String.format("Ignoring duplicate %s trait value on %s", traitId, target));
return previous;
Expand Down
Loading