Skip to content

Commit 3ec4aa6

Browse files
committed
Fix HttpBindingsMissing skipping REST protocol services
The `HttpBindingsMissingValidator` had a top-level short-circuit that returned early when no shape in the model carried `@http`. Services with a protocol like `restJson1` that requires `@http` on all operations would pass validation silently when no operations had the trait yet.
1 parent 607bea7 commit 3ec4aa6

9 files changed

Lines changed: 99 additions & 166 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "bugfix",
3+
"description": "Fixed a bug where HttpBindingMissing validation didn't fire if a service requiring `@http` traits had none at all.",
4+
"pull_requests": [
5+
"[#3122](https://github.com/smithy-lang/smithy/pull/3122)"
6+
]
7+
}

smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/error-rename.smithy

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ service MyService {
1919
}
2020
}
2121

22+
@http(method: "POST", uri: "/say-hello")
2223
operation SayHello {
2324
input: SayHelloInput,
2425
output: SayHelloOutput,

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*/
55
package software.amazon.smithy.model.validation.validators;
66

7-
import java.util.Collections;
87
import java.util.List;
98
import java.util.Set;
109
import java.util.stream.Collectors;
@@ -29,10 +28,6 @@
2928
public final class HttpBindingsMissingValidator extends AbstractValidator {
3029
@Override
3130
public List<ValidationEvent> validate(Model model) {
32-
if (!model.isTraitApplied(HttpTrait.class)) {
33-
return Collections.emptyList();
34-
}
35-
3631
TopDownIndex topDownIndex = TopDownIndex.of(model);
3732
return model.shapes(ServiceShape.class)
3833
.flatMap(shape -> validateService(topDownIndex, model, shape).stream())
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[ERROR] ns.foo#MissingBindings1: The `ns.foo#restProtocol` protocol requires all operations in the `ns.foo#MyService` service define the `http` trait, but this operation is missing the `http` trait. | HttpBindingsMissing
2+
[ERROR] ns.foo#MissingBindings2: The `ns.foo#restProtocol` protocol requires all operations in the `ns.foo#MyService` service define the `http` trait, but this operation is missing the `http` trait. | HttpBindingsMissing
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
$version: "2.0"
2+
3+
namespace ns.foo
4+
5+
@trait(selector: "service")
6+
@protocolDefinition(traits: [
7+
smithy.api#cors
8+
smithy.api#endpoint
9+
smithy.api#hostLabel
10+
smithy.api#http
11+
smithy.api#httpError
12+
smithy.api#httpHeader
13+
smithy.api#httpLabel
14+
smithy.api#httpPayload
15+
smithy.api#httpPrefixHeaders
16+
smithy.api#httpQuery
17+
smithy.api#httpQueryParams
18+
smithy.api#httpResponseCode
19+
smithy.api#jsonName
20+
smithy.api#timestampFormat
21+
])
22+
@documentation("A simple clone of AWS restJson1.")
23+
structure restProtocol {}
24+
25+
@restProtocol
26+
service MyService {
27+
version: "2017-01-17"
28+
operations: [MissingBindings1, MissingBindings2]
29+
}
30+
31+
@readonly
32+
operation MissingBindings1 {
33+
input := {}
34+
output := {}
35+
}
36+
37+
@readonly
38+
operation MissingBindings2 {
39+
input := {}
40+
output := {}
41+
}

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-bindings-missing-validator.json

Lines changed: 0 additions & 115 deletions
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
$version: "2.0"
2+
3+
namespace ns.foo
4+
5+
@trait(selector: "service")
6+
@protocolDefinition(traits: [
7+
smithy.api#cors
8+
smithy.api#endpoint
9+
smithy.api#hostLabel
10+
smithy.api#http
11+
smithy.api#httpError
12+
smithy.api#httpHeader
13+
smithy.api#httpLabel
14+
smithy.api#httpPayload
15+
smithy.api#httpPrefixHeaders
16+
smithy.api#httpQuery
17+
smithy.api#httpQueryParams
18+
smithy.api#httpResponseCode
19+
smithy.api#jsonName
20+
smithy.api#timestampFormat
21+
])
22+
@documentation("A simple clone of AWS restJson1.")
23+
structure restProtocol {}
24+
25+
@restProtocol
26+
service MyService {
27+
version: "2017-01-17"
28+
operations: [HasBindings, MissingBindings1, MissingBindings2]
29+
}
30+
31+
@readonly
32+
@http(method: "GET", uri: "/A")
33+
operation HasBindings {
34+
input := {}
35+
output := {}
36+
}
37+
38+
@readonly
39+
operation MissingBindings1 {
40+
input := {}
41+
output := {}
42+
}
43+
44+
@readonly
45+
operation MissingBindings2 {
46+
input := {}
47+
output := {}
48+
}

smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverterTest.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import software.amazon.smithy.openapi.OpenApiException;
3131
import software.amazon.smithy.openapi.OpenApiVersion;
3232
import software.amazon.smithy.openapi.model.OpenApi;
33-
import software.amazon.smithy.openapi.model.PathItem;
3433
import software.amazon.smithy.utils.IoUtils;
3534
import software.amazon.smithy.utils.ListUtils;
3635
import software.amazon.smithy.utils.MapUtils;
@@ -271,31 +270,6 @@ public void omitsUnsupportedHttpMethods() {
271270
Node.assertEquals(result, expectedNode);
272271
}
273272

274-
@Test
275-
public void protocolsCanOmitOperations() {
276-
Model model = Model.assembler()
277-
.addImport(getClass().getResource("missing-http-bindings.json"))
278-
.discoverModels()
279-
.assemble()
280-
.unwrap();
281-
OpenApiConfig config = new OpenApiConfig();
282-
config.setService(ShapeId.from("smithy.example#Service"));
283-
OpenApi result = OpenApiConverter.create()
284-
.config(config)
285-
.convert(model);
286-
287-
for (PathItem pathItem : result.getPaths().values()) {
288-
assertFalse(pathItem.getGet().isPresent());
289-
assertFalse(pathItem.getHead().isPresent());
290-
assertFalse(pathItem.getDelete().isPresent());
291-
assertFalse(pathItem.getPatch().isPresent());
292-
assertFalse(pathItem.getPost().isPresent());
293-
assertFalse(pathItem.getPut().isPresent());
294-
assertFalse(pathItem.getTrace().isPresent());
295-
assertFalse(pathItem.getOptions().isPresent());
296-
}
297-
}
298-
299273
@Test
300274
public void addsEmptyResponseByDefault() {
301275
Model model = Model.assembler()

smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/missing-http-bindings.json

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)