Don't conflict URI if an endpoint prefix is present

docs/source/1.0/spec/core/http-traits.rst

@@ -418,14 +418,14 @@ following rules are in place:
    - ``/{foo}/{bar+}`` is legal
    - ``/{foo+}/{bar}`` is illegal
 
-#. Patterns MUST NOT be equivalent.
+#. Patterns MUST NOT be equivalent if they share a host.
 
    - Pattern ``/foo/bar`` and ``/foo/bar`` conflict.
    - Pattern ``/foo/{bar}`` and ``/foo/{baz}`` conflict regardless of any
      constraint traits on the label members.
 
 #. A label and a literal SHOULD NOT both occupy the same segment in patterns
-   which are equivalent to that point.
+   which are equivalent to that point if they share a host.
 
    - ``/foo/bar/{baz}`` and ``/foo/baz/bam`` can coexist.
    - ``/foo/bar`` and ``/foo/{baz}/bam`` cannot coexist unless pattern
@@ -436,6 +436,15 @@ following rules are in place:
    empty value are considered equivalent. For example, ``/foo?baz`` and
    ``/foo?baz=`` are considered the same route.
 
+#. Patterns MAY conflict if the operations use different hosts. Different hosts
+   can be configured using the :ref:`endpoint-trait`'s ``hostPrefix`` property.
+
+   - ``/foo/bar`` and ``/foo/{baz}/bam`` can coexist if one operation has no
+     endpoint trait and the other specifies ``foo.`` as the ``hostPrefix``.
+   - ``/foo/bar`` and ``/foo/{baz}/bam`` can coexist if one operation specifies
+     ``foo.`` as the ``hostPrefix`` and the other specifies ``bar.`` as the
+     ``hostPrefix``.
+
 
 .. _httpError-trait:
 

smithy-model/src/main/java/software/amazon/smithy/model/pattern/SmithyPattern.java

@@ -17,6 +17,7 @@
 
 import static java.lang.String.format;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -26,6 +27,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.utils.Pair;
 
 /**
  * Represents a contained pattern.
@@ -111,6 +113,43 @@ public boolean equals(Object other) {
         return other instanceof SmithyPattern && pattern.equals(((SmithyPattern) other).pattern);
     }
 
+    /**
+     * Gets a list of explicitly conflicting label segments between this
+     * pattern and another.
+     *
+     * @param otherPattern SmithyPattern to check against.
+     * @return A list of Segment Pairs where each pair represents a conflict
+     *     and where the left side of the Pair is a segment from this pattern.
+     */
+    public List<Pair<Segment, Segment>> getConflictingLabelSegments(SmithyPattern otherPattern) {
+        List<Pair<Segment, Segment>> conflictingSegments = new ArrayList<>();
+
+        List<Segment> segments = getSegments();
+        List<Segment> otherSegments = otherPattern.getSegments();
+        int minSize = Math.min(segments.size(), otherSegments.size());
+        for (int i = 0; i < minSize; i++) {
+            Segment thisSegment = segments.get(i);
+            Segment otherSegment = otherSegments.get(i);
+            if (thisSegment.isLabel() != otherSegment.isLabel()) {
+                // The segments conflict if one is a literal and the other
+                // is a label.
+                conflictingSegments.add(Pair.of(thisSegment, otherSegment));
+            } else if  (thisSegment.isGreedyLabel() != otherSegment.isGreedyLabel()) {
+                // The segments conflict if a greedy label is introduced at
+                // or before segments in the other pattern.
+                conflictingSegments.add(Pair.of(thisSegment, otherSegment));
+            } else if (!thisSegment.isLabel()) {
+                // Both are literals. They can only conflict if they are the
+                // same exact string.
+                if (!thisSegment.getContent().equals(otherSegment.getContent())) {
+                    return conflictingSegments;
+                }
+            }
+        }
+
+        return conflictingSegments;
+    }
+
     @Override
     public int hashCode() {
         return pattern.hashCode();

smithy-model/src/main/java/software/amazon/smithy/model/pattern/UriPattern.java

@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import software.amazon.smithy.utils.Pair;
 
 /**
  * Represents a URI pattern.
@@ -156,43 +155,6 @@ public boolean conflictsWith(UriPattern otherPattern) {
         return queryLiterals.equals(otherPattern.queryLiterals);
     }
 
-    /**
-     * Gets a list of explicitly conflicting uri label segments between this
-     * pattern and another.
-     *
-     * @param otherPattern SmithyPattern to check against.
-     * @return A list of Segment Pairs where each pair represents a conflict
-     *     and where the left side of the Pair is a segment from this pattern.
-     */
-    public List<Pair<Segment, Segment>> getConflictingLabelSegments(UriPattern otherPattern) {
-        List<Pair<Segment, Segment>> conflictingSegments = new ArrayList<>();
-
-        List<Segment> segments = getSegments();
-        List<Segment> otherSegments = otherPattern.getSegments();
-        int minSize = Math.min(segments.size(), otherSegments.size());
-        for (int i = 0; i < minSize; i++) {
-            Segment thisSegment = segments.get(i);
-            Segment otherSegment = otherSegments.get(i);
-            if (thisSegment.isLabel() != otherSegment.isLabel()) {
-                // The segments conflict if one is a literal and the other
-                // is a label.
-                conflictingSegments.add(Pair.of(thisSegment, otherSegment));
-            } else if  (thisSegment.isGreedyLabel() != otherSegment.isGreedyLabel()) {
-                // The segments conflict if a greedy label is introduced at
-                // or before segments in the other pattern.
-                conflictingSegments.add(Pair.of(thisSegment, otherSegment));
-            } else if (!thisSegment.isLabel()) {
-                // Both are literals. They can only conflict if they are the
-                // same exact string.
-                if (!thisSegment.getContent().equals(otherSegment.getContent())) {
-                    return conflictingSegments;
-                }
-            }
-        }
-
-        return conflictingSegments;
-    }
-
     @Override
     public boolean equals(Object other) {
         if (!(other instanceof UriPattern)) {

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpUriConflictValidator.java

@@ -19,17 +19,22 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiFunction;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.knowledge.HttpBinding;
 import software.amazon.smithy.model.knowledge.HttpBindingIndex;
+import software.amazon.smithy.model.knowledge.OperationIndex;
 import software.amazon.smithy.model.knowledge.TopDownIndex;
+import software.amazon.smithy.model.pattern.SmithyPattern;
 import software.amazon.smithy.model.pattern.SmithyPattern.Segment;
 import software.amazon.smithy.model.pattern.UriPattern;
 import software.amazon.smithy.model.shapes.OperationShape;
 import software.amazon.smithy.model.shapes.ServiceShape;
 import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.model.traits.EndpointTrait;
+import software.amazon.smithy.model.traits.HostLabelTrait;
 import software.amazon.smithy.model.traits.HttpTrait;
 import software.amazon.smithy.model.traits.PatternTrait;
 import software.amazon.smithy.model.traits.Trait;
@@ -83,6 +88,7 @@ private List<ValidationEvent> checkConflicts(
                 .flatMap(shape -> Trait.flatMapStream(shape, HttpTrait.class))
                 .filter(other -> other.getRight().getMethod().equals(method))
                 .filter(other -> other.getRight().getUri().conflictsWith(pattern))
+                .filter(other -> endpointConflicts(model, operation, other.getLeft()))
                 .forEach(other -> {
                     // Now that we know we have a conflict, determine whether it is allowable or not.
                     if (isAllowableConflict(model, operation, other.getLeft())) {
@@ -109,47 +115,79 @@ private List<ValidationEvent> checkConflicts(
         return events;
     }
 
+    private boolean endpointConflicts(Model model, OperationShape operation, OperationShape otherOperation) {
+        // If neither operation has the endpoint trait, then their endpoints do conflict.
+        if (!operation.hasTrait(EndpointTrait.ID) && !otherOperation.hasTrait(EndpointTrait.ID)) {
+            return true;
+        }
+
+        // If one, but not both, operations have the endpoint trait, then their endpoints can't conflict.
+        if (operation.hasTrait(EndpointTrait.ID) ^ otherOperation.hasTrait(EndpointTrait.ID)) {
+            return false;
+        }
+
+        // At this point we know both operations have the endpoint trait, so we need to check if
+        // they conflict.
+        SmithyPattern prefix = operation.getTrait(EndpointTrait.class).get().getHostPrefix();
+        SmithyPattern otherPrefix = otherOperation.getTrait(EndpointTrait.class).get().getHostPrefix();
+        boolean allowable = !isAllowableConflict(
+                model, prefix, operation, otherPrefix, otherOperation, this::getHostLabelPatterns);
+        return allowable;
+    }
+
     private boolean isAllowableConflict(Model model, OperationShape operation, OperationShape otherOperation) {
         UriPattern uriPattern = operation.getTrait(HttpTrait.class).get().getUri();
         UriPattern otherUriPattern = otherOperation.getTrait(HttpTrait.class).get().getUri();
+        return isAllowableConflict(
+                model, uriPattern, operation, otherUriPattern, otherOperation, this::getHttpLabelPatterns);
+    }
+
+    private boolean isAllowableConflict(
+            Model model,
+            SmithyPattern pattern,
+            OperationShape operation,
+            SmithyPattern otherPattern,
+            OperationShape otherOperation,
+            BiFunction<Model, OperationShape, Map<String, Pattern>> getLabelPatterns
+    ) {
 
-        List<Pair<Segment, Segment>> conflictingLabelSegments = uriPattern.getConflictingLabelSegments(otherUriPattern);
+        List<Pair<Segment, Segment>> conflictingLabelSegments = pattern.getConflictingLabelSegments(otherPattern);
 
         // If there aren't any conflicting label segments that means the uris are identical, which is not allowable.
         if (conflictingLabelSegments.isEmpty()) {
             return false;
         }
 
-        Map<String, Pattern> labelPatterns = getLabelPatterns(model, operation);
-        Map<String, Pattern> otherLabelPatterns = getLabelPatterns(model, otherOperation);
+        Map<String, Pattern> labelPatterns = getLabelPatterns.apply(model, operation);
+        Map<String, Pattern> otherLabelPatterns = getLabelPatterns.apply(model, otherOperation);
 
         return conflictingLabelSegments.stream()
                 // Only allow conflicts in cases where one of the segments is static and the other is a label.
                 .filter(conflict -> conflict.getLeft().isLabel() != conflict.getRight().isLabel())
                 // Only allow the uris to conflict if every conflicting segment is allowable.
                 .allMatch(conflict -> {
-                    Pattern pattern;
+                    Pattern p;
                     String staticSegment;
 
                     if (conflict.getLeft().isLabel()) {
-                        pattern = labelPatterns.get(conflict.getLeft().getContent());
+                        p = labelPatterns.get(conflict.getLeft().getContent());
                         staticSegment = conflict.getRight().getContent();
                     } else {
-                        pattern = otherLabelPatterns.get(conflict.getRight().getContent());
+                        p = otherLabelPatterns.get(conflict.getRight().getContent());
                         staticSegment = conflict.getLeft().getContent();
                     }
 
-                    if (pattern == null) {
+                    if (p == null) {
                         return false;
                     }
 
                     // If the pattern on the label segment does not match the static segment, then this segment's
                     // conflict is allowable.
-                    return !pattern.matcher(staticSegment).find();
+                    return !p.matcher(staticSegment).find();
                 });
     }
 
-    private Map<String, Pattern> getLabelPatterns(Model model, OperationShape operation) {
+    private Map<String, Pattern> getHttpLabelPatterns(Model model, OperationShape operation) {
         return HttpBindingIndex.of(model)
                 .getRequestBindings(operation).entrySet().stream()
                 .filter(entry -> entry.getValue().getLocation().equals(HttpBinding.Location.LABEL))
@@ -159,6 +197,16 @@ private Map<String, Pattern> getLabelPatterns(Model model, OperationShape operat
                 .collect(Collectors.toMap(Pair::getLeft, Pair::getRight));
     }
 
+    private Map<String, Pattern> getHostLabelPatterns(Model model, OperationShape operation) {
+        return OperationIndex.of(model).getInput(operation)
+                .map(structureShape -> structureShape.members().stream()
+                .filter(member -> member.hasTrait(HostLabelTrait.ID))
+                .flatMap(member -> Trait.flatMapStream(member, PatternTrait.class))
+                .collect(Collectors.toMap(
+                        pair -> pair.getLeft().getMemberName(), pair -> pair.getRight().getPattern())))
+                .orElse(Collections.emptyMap());
+    }
+
     private String formatConflicts(UriPattern pattern, List<Pair<ShapeId, UriPattern>> conflicts) {
         String conflictString = conflicts.stream()
                 .map(conflict -> String.format("`%s` (%s)", conflict.getLeft(), conflict.getRight()))

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-uri-conflict-validator.errors

@@ -15,3 +15,5 @@
 [DANGER] ns.foo#DifferingSegmentEndsConflictChecks2: Operation URI, `/differing/foo/bar/bin/{Id}`, conflicts with other operation URIs in the same service: [`ns.foo#DifferingSegmentEndsConflictChecks` (/differing/{Id}/bar/bam/{OtherId})]. Pattern traits applied to the label members prevent the label value from evaluating to a conflict, but this is still a poor design. If this is acceptable, this can be suppressed. | HttpUriConflict
 [ERROR] ns.foo#ConflictsWithoutPattern: Operation URI, `/no-pattern-conflict/foo`, conflicts with other operation URIs in the same service: [`ns.foo#ConflictsWithoutPattern2` (/no-pattern-conflict/{Id})] | HttpUriConflict
 [ERROR] ns.foo#ConflictsWithoutPattern2: Operation URI, `/no-pattern-conflict/{Id}`, conflicts with other operation URIs in the same service: [`ns.foo#ConflictsWithoutPattern` (/no-pattern-conflict/foo)] | HttpUriConflict
+[ERROR] ns.foo#LackOfPatternEnablesConflict1: Operation URI, `/double-endpoint-prefix-conflicts`, conflicts with other operation URIs in the same service: [`ns.foo#LackOfPatternEnablesConflict2` (/double-endpoint-prefix-conflicts)] | HttpUriConflict
+[ERROR] ns.foo#LackOfPatternEnablesConflict2: Operation URI, `/double-endpoint-prefix-conflicts`, conflicts with other operation URIs in the same service: [`ns.foo#LackOfPatternEnablesConflict1` (/double-endpoint-prefix-conflicts)] | HttpUriConflict