Add builtin validations for querystring parameter location (#488)

* Add querystring parameter builtin validations

Author: ayushshrivastv

Sources/OpenAPIKit/Validator/Validation+Builtins.swift

@@ -394,6 +394,108 @@ extension Validation {
         )
     }
 
+    /// Validate that `querystring` parameters are unique and do not coexist
+    /// with `query` parameters within a Path Item's effective operation
+    /// parameters.
+    ///
+    /// OpenAPI 3.2.0 requires that a `querystring` parameter
+    /// [must not appear more than once and must not appear in the same operation
+    /// as any `query` parameters](https://spec.openapis.org/oas/v3.2.0.html#parameter-locations).
+    ///
+    /// - Important: This is included in validation by default.
+    public static var querystringParametersAreCompatible: Validation<OpenAPI.PathItem> {
+        .init(
+            description: "Querystring parameters are unique and do not coexist with query parameters",
+            check: { context in
+                let pathParameters = resolvedParameters(context.subject.parameters, components: context.document.components)
+                let pathSummary = ParameterLocationSummary(pathParameters)
+                let pathParametersPath = context.codingPath + [Validator.CodingKey.init(stringValue: "parameters")]
+                var errors = [ValidationError]()
+                let pathHasMultipleQuerystringParameters = pathSummary.querystringCount > 1
+                let pathMixesQueryLocations = pathSummary.querystringCount > 0 && pathSummary.queryCount > 0
+
+                if pathHasMultipleQuerystringParameters {
+                    errors.append(
+                        ValidationError(
+                            reason: "Path Item parameters must not contain more than one `querystring` parameter",
+                            at: pathParametersPath
+                        )
+                    )
+                }
+
+                if pathMixesQueryLocations {
+                    errors.append(
+                        ValidationError(
+                            reason: "Path Item parameters must not mix `querystring` and `query` parameter locations",
+                            at: pathParametersPath
+                        )
+                    )
+                }
+
+                for endpoint in context.subject.endpoints {
+                    let operationParameters = resolvedParameters(endpoint.operation.parameters, components: context.document.components)
+                    let operationSummary = ParameterLocationSummary(operationParameters)
+                    let operationParametersPath = context.codingPath + [
+                        Validator.CodingKey.init(stringValue: codingPathKey(for: endpoint.method)),
+                        Validator.CodingKey.init(stringValue: "parameters")
+                    ]
+                    let operationHasMultipleQuerystringParameters = operationSummary.querystringCount > 1
+                    let operationMixesQueryLocations = operationSummary.querystringCount > 0 &&
+                        operationSummary.queryCount > 0
+                    let effectiveQuerystringCount = pathSummary.querystringCount + operationSummary.querystringCount
+                    let effectiveQueryCount = pathSummary.queryCount + operationSummary.queryCount
+                    let inheritedHasMultipleQuerystringParameters =
+                        !pathHasMultipleQuerystringParameters &&
+                        !operationHasMultipleQuerystringParameters &&
+                        effectiveQuerystringCount > 1
+                    let inheritedMixesQueryLocations =
+                        !pathMixesQueryLocations &&
+                        !operationMixesQueryLocations &&
+                        effectiveQuerystringCount > 0 &&
+                        effectiveQueryCount > 0
+
+                    if operationHasMultipleQuerystringParameters {
+                        errors.append(
+                            ValidationError(
+                                reason: "Operation parameters must not contain more than one `querystring` parameter",
+                                at: operationParametersPath
+                            )
+                        )
+                    }
+
+                    if operationMixesQueryLocations {
+                        errors.append(
+                            ValidationError(
+                                reason: "Operation parameters must not mix `querystring` and `query` parameter locations",
+                                at: operationParametersPath
+                            )
+                        )
+                    }
+
+                    if inheritedHasMultipleQuerystringParameters {
+                        errors.append(
+                            ValidationError(
+                                reason: "Operation parameters must not contain more than one `querystring` parameter, including inherited Path Item parameters",
+                                at: operationParametersPath
+                            )
+                        )
+                    }
+
+                    if inheritedMixesQueryLocations {
+                        errors.append(
+                            ValidationError(
+                                reason: "Operation parameters must not mix `querystring` and `query` parameter locations, including inherited Path Item parameters",
+                                at: operationParametersPath
+                            )
+                        )
+                    }
+                }
+
+                return errors
+            }
+        )
+    }
+
     /// Validate that all OpenAPI Operation Ids are unique across the whole Document.
     ///
     /// The OpenAPI Specification requires that Operation Ids [are unique](https://spec.openapis.org/oas/v3.2.0.html#operation-object).
@@ -592,9 +694,32 @@ extension Validation {
 /// Used by both the Path Item parameter check and the
 /// Operation parameter check in the default validations.
 fileprivate func parametersAreUnique(_ parameters: OpenAPI.Parameter.Array, components: OpenAPI.Components) -> Bool {
-    let foundParameters = parameters.compactMap { try? components.lookup($0) }
+    let foundParameters = resolvedParameters(parameters, components: components)
 
     let identities = foundParameters.map { OpenAPI.Parameter.ParameterIdentity(name: $0.name, location: $0.location) }
 
     return Set(identities).count == foundParameters.count
 }
+
+fileprivate func resolvedParameters(_ parameters: OpenAPI.Parameter.Array, components: OpenAPI.Components) -> [OpenAPI.Parameter] {
+    parameters.compactMap { try? components.lookup($0) }
+}
+
+fileprivate struct ParameterLocationSummary {
+    let queryCount: Int
+    let querystringCount: Int
+
+    init(_ parameters: [OpenAPI.Parameter]) {
+        queryCount = parameters.filter { $0.location == .query }.count
+        querystringCount = parameters.filter { $0.location == .querystring }.count
+    }
+}
+
+fileprivate func codingPathKey(for method: OpenAPI.HttpMethod) -> String {
+    switch method {
+    case .builtin(let builtin):
+        return builtin.rawValue.lowercased()
+    case .other(let other):
+        return other
+    }
+}

Sources/OpenAPIKit/Validator/Validator.swift

@@ -78,6 +78,7 @@ extension OpenAPI.Document {
 /// - Server names are unique across the whole Document.
 /// - Parameters are unique within each Path Item.
 /// - Parameters are unique within each Operation.
+/// - Querystring parameters are unique and do not coexist with query parameters.
 /// - Operation Ids are unique across the whole Document.
 /// - All OpenAPI.References that refer to components in this
 ///     document can be found in the components dictionary.
@@ -160,6 +161,7 @@ public final class Validator {
         .init(.documentServerNamesAreUnique),
         .init(.pathItemParametersAreUnique),
         .init(.operationParametersAreUnique),
+        .init(.querystringParametersAreCompatible),
         .init(.operationIdsAreUnique),
         .init(.serverVariableEnumIsValid),
         .init(.serverVariableDefaultExistsInEnum),
@@ -205,6 +207,7 @@ public final class Validator {
     /// - Server names are unique across the whole Document.
     /// - Parameters are unique within each Path Item.
     /// - Parameters are unique within each Operation.
+    /// - Querystring parameters are unique and do not coexist with query parameters.
     /// - Operation Ids are unique across the whole Document.
     /// - All OpenAPI.References that refer to components in this document can
     ///     be found in the components dictionary.

Tests/OpenAPIKitTests/Validator/BuiltinValidationTests.swift

@@ -24,25 +24,27 @@ final class BuiltinValidationTests: XCTestCase {
         ])
 
         let withoutReferenceValidations = Validator().skippingReferenceValidations()
-        XCTAssertEqual(withoutReferenceValidations.validationDescriptions.count, 8)
+        XCTAssertEqual(withoutReferenceValidations.validationDescriptions.count, 9)
         XCTAssertEqual(withoutReferenceValidations.validationDescriptions, [
             "The names of Tags in the Document are unique",
             "The names of Servers in the Document are unique",
             "Path Item parameters are unique (identity is defined by the \'name\' and \'location\')",
             "Operation parameters are unique (identity is defined by the \'name\' and \'location\')",
+            "Querystring parameters are unique and do not coexist with query parameters",
             "All Operation Ids in Document are unique",
             "Server Variable\'s enum is either not defined or is non-empty (if defined).",
             "Server Variable\'s default must exist in enum, if enum is defined.",
             "Parameter styles are all compatible with their locations"
         ])
 
         let defaultValidations = Validator()
-        XCTAssertEqual(defaultValidations.validationDescriptions.count, 18)
+        XCTAssertEqual(defaultValidations.validationDescriptions.count, 19)
         XCTAssertEqual(defaultValidations.validationDescriptions, [
             "The names of Tags in the Document are unique",
             "The names of Servers in the Document are unique",
             "Path Item parameters are unique (identity is defined by the \'name\' and \'location\')",
             "Operation parameters are unique (identity is defined by the \'name\' and \'location\')",
+            "Querystring parameters are unique and do not coexist with query parameters",
             "All Operation Ids in Document are unique",
             "Server Variable\'s enum is either not defined or is non-empty (if defined).",
             "Server Variable\'s default must exist in enum, if enum is defined.",
@@ -60,12 +62,13 @@ final class BuiltinValidationTests: XCTestCase {
         ])
 
         let stricterReferenceValidations = Validator().validatingAllReferencesFoundInComponents()
-        XCTAssertEqual(stricterReferenceValidations.validationDescriptions.count, 18)
+        XCTAssertEqual(stricterReferenceValidations.validationDescriptions.count, 19)
         XCTAssertEqual(stricterReferenceValidations.validationDescriptions, [
             "The names of Tags in the Document are unique",
             "The names of Servers in the Document are unique",
             "Path Item parameters are unique (identity is defined by the \'name\' and \'location\')",
             "Operation parameters are unique (identity is defined by the \'name\' and \'location\')",
+            "Querystring parameters are unique and do not coexist with query parameters",
             "All Operation Ids in Document are unique",
             "Server Variable\'s enum is either not defined or is non-empty (if defined).",
             "Server Variable\'s default must exist in enum, if enum is defined.",
@@ -1483,4 +1486,135 @@ final class BuiltinValidationTests: XCTestCase {
             XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters[0]")
         }
     }
+
+    func test_duplicateQuerystringParametersOnPathItem_fails() throws {
+        let document = OpenAPI.Document(
+            info: .init(title: "test", version: "1.0"),
+            servers: [],
+            paths: [
+                "/hello": .init(
+                    parameters: [
+                        .parameter(OpenAPI.Parameter.querystring(name: "first", content: [:])),
+                        .parameter(OpenAPI.Parameter.querystring(name: "second", content: [:]))
+                    ],
+                    get: .init(responses: [:])
+                )
+            ],
+            components: .noComponents
+        )
+
+        let validator = Validator.blank.validating(.querystringParametersAreCompatible)
+
+        XCTAssertThrowsError(try document.validate(using: validator)) { error in
+            let errorCollection = error as? ValidationErrorCollection
+            XCTAssertEqual(errorCollection?.values.first?.reason, "Path Item parameters must not contain more than one `querystring` parameter")
+            XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].parameters")
+        }
+    }
+
+    func test_duplicateQuerystringParametersAcrossPathItemAndOperation_fails() throws {
+        let document = OpenAPI.Document(
+            info: .init(title: "test", version: "1.0"),
+            servers: [],
+            paths: [
+                "/hello": .init(
+                    parameters: [
+                        .parameter(OpenAPI.Parameter.querystring(name: "first", content: [:]))
+                    ],
+                    get: .init(
+                        parameters: [
+                            .parameter(OpenAPI.Parameter.querystring(name: "second", content: [:]))
+                        ],
+                        responses: [:]
+                    )
+                )
+            ],
+            components: .noComponents
+        )
+
+        let validator = Validator.blank.validating(.querystringParametersAreCompatible)
+
+        XCTAssertThrowsError(try document.validate(using: validator)) { error in
+            let errorCollection = error as? ValidationErrorCollection
+            XCTAssertEqual(errorCollection?.values.first?.reason, "Operation parameters must not contain more than one `querystring` parameter, including inherited Path Item parameters")
+            XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters")
+        }
+    }
+
+    func test_querystringAndQueryParametersOnOperation_fails() throws {
+        let document = OpenAPI.Document(
+            info: .init(title: "test", version: "1.0"),
+            servers: [],
+            paths: [
+                "/hello": .init(
+                    get: .init(
+                        parameters: [
+                            .parameter(OpenAPI.Parameter.query(name: "query", schema: .string)),
+                            .parameter(OpenAPI.Parameter.querystring(name: "querystring", content: [:]))
+                        ],
+                        responses: [:]
+                    )
+                )
+            ],
+            components: .noComponents
+        )
+
+        let validator = Validator.blank.validating(.querystringParametersAreCompatible)
+
+        XCTAssertThrowsError(try document.validate(using: validator)) { error in
+            let errorCollection = error as? ValidationErrorCollection
+            XCTAssertEqual(errorCollection?.values.first?.reason, "Operation parameters must not mix `querystring` and `query` parameter locations")
+            XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters")
+        }
+    }
+
+    func test_querystringAndQueryParametersAcrossPathItemAndOperation_fails() throws {
+        let document = OpenAPI.Document(
+            info: .init(title: "test", version: "1.0"),
+            servers: [],
+            paths: [
+                "/hello": .init(
+                    parameters: [
+                        .parameter(OpenAPI.Parameter.query(name: "query", schema: .string))
+                    ],
+                    get: .init(
+                        parameters: [
+                            .parameter(OpenAPI.Parameter.querystring(name: "querystring", content: [:]))
+                        ],
+                        responses: [:]
+                    )
+                )
+            ],
+            components: .noComponents
+        )
+
+        let validator = Validator.blank.validating(.querystringParametersAreCompatible)
+
+        XCTAssertThrowsError(try document.validate(using: validator)) { error in
+            let errorCollection = error as? ValidationErrorCollection
+            XCTAssertEqual(errorCollection?.values.first?.reason, "Operation parameters must not mix `querystring` and `query` parameter locations, including inherited Path Item parameters")
+            XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters")
+        }
+    }
+
+    func test_singleQuerystringParameter_succeeds() throws {
+        let document = OpenAPI.Document(
+            info: .init(title: "test", version: "1.0"),
+            servers: [],
+            paths: [
+                "/hello": .init(
+                    get: .init(
+                        parameters: [
+                            .parameter(OpenAPI.Parameter.querystring(name: "querystring", content: [:]))
+                        ],
+                        responses: [:]
+                    )
+                )
+            ],
+            components: .noComponents
+        )
+
+        let validator = Validator.blank.validating(.querystringParametersAreCompatible)
+        try document.validate(using: validator)
+    }
 }