From 99bded882fb499f9ff9ff9e093a164ccc31de3ad Mon Sep 17 00:00:00 2001 From: Dmitry Demyanov Date: Mon, 8 Nov 2021 12:42:16 +0300 Subject: [PATCH] handling path parameters declaration place --- Package.resolved | 9 +++ .../PathGenerationModel.swift | 2 +- Sources/CodeGenerator/Models/PathModel.swift | 4 +- .../TreeParserStage/SwaggerCorrector.swift | 22 +++++++ .../TreeParser/TreeParser.swift | 5 +- .../Builders/AnyServiceBuilder.swift | 46 +++++++------ Sources/GASTTree/Services/PathNode.swift | 4 +- .../CodeGen/SwaggerCorrectorStage.swift | 7 +- .../SwaggerCorrectorTests.swift | 35 +++++++++- .../SwaggerCorrectorYamls.swift | 32 ++++++++++ .../ParametersTests.swift | 64 +++++++++++++++++++ .../YamlToGenerationModelsTests/Yamls.swift | 45 +++++++++++++ 12 files changed, 247 insertions(+), 28 deletions(-) create mode 100644 Tests/CodeGeneratorTests/SwaggerCorrectorYamls.swift diff --git a/Package.resolved b/Package.resolved index 7ae47a98..ac79aad9 100644 --- a/Package.resolved +++ b/Package.resolved @@ -90,6 +90,15 @@ "revision": "2e949055d9797c1a6bddcda0e58dada16cc8e970", "version": "6.0.3" } + }, + { + "package": "Yams", + "repositoryURL": "https://github.com/jpsim/Yams", + "state": { + "branch": null, + "revision": "b08dba4bcea978bf1ad37703a384097d3efce5af", + "version": "1.0.2" + } } ] }, diff --git a/Sources/CodeGenerator/Models/GenerationModels/PathGenerationModel.swift b/Sources/CodeGenerator/Models/GenerationModels/PathGenerationModel.swift index ebf97e71..a9a8ab06 100644 --- a/Sources/CodeGenerator/Models/GenerationModels/PathGenerationModel.swift +++ b/Sources/CodeGenerator/Models/GenerationModels/PathGenerationModel.swift @@ -25,7 +25,7 @@ public struct PathGenerationModel: Encodable { .map { OperationGenerationModel(operationModel: $0) } self.name = pathModel.path.pathName self.pathWithSeparatedParameters = pathModel.path.pathWithSeparatedParameters - self.parameters = operations[0].pathParameters + self.parameters = pathModel.parameters.map { $0.value } } } diff --git a/Sources/CodeGenerator/Models/PathModel.swift b/Sources/CodeGenerator/Models/PathModel.swift index 690e84cc..4c8a53be 100644 --- a/Sources/CodeGenerator/Models/PathModel.swift +++ b/Sources/CodeGenerator/Models/PathModel.swift @@ -41,10 +41,12 @@ import Foundation public struct PathModel: Encodable { /// URI template public let path: String + public let parameters: [Reference] public let operations: [OperationModel] - public init(path: String, operations: [OperationModel]) { + public init(path: String, parameters: [Reference], operations: [OperationModel]) { self.path = path + self.parameters = parameters self.operations = operations.sorted { $0.httpMethod < $1.httpMethod } } diff --git a/Sources/CodeGenerator/Stages/TreeParserStage/SwaggerCorrector.swift b/Sources/CodeGenerator/Stages/TreeParserStage/SwaggerCorrector.swift index d08eb8d2..a522889c 100644 --- a/Sources/CodeGenerator/Stages/TreeParserStage/SwaggerCorrector.swift +++ b/Sources/CodeGenerator/Stages/TreeParserStage/SwaggerCorrector.swift @@ -33,4 +33,26 @@ public class SwaggerCorrector { return path } + + /// Checks that `path` parameters are declared in `Path`, but not in `Operation` + /// If some are declared in `Operation`, gives warning and tries to fix it + /// See `ParameterTests` for details + /// See `SwaggerCorrectorTests` for example + public func correctPathParameters(for pathModel: PathModel) -> [Reference] { + let pathParameterNames = pathModel.parameters.map { $0.value.name } + + for operation in pathModel.operations { + let operationPathParameters = operation.parameters?.filter { $0.value.location == .path } ?? [] + let operationOnlyPathParameters = operationPathParameters.filter { !pathParameterNames.contains($0.value.name) } + + guard operationOnlyPathParameters.isEmpty else { + for parameter in operationOnlyPathParameters { + logger?.warning("Parameter \(parameter.value.name) for path \(pathModel.path) is declared inside Operation. Path parameters should be declared inside Path. Trying to fix...") + } + return operationPathParameters + } + } + return pathModel.parameters + } + } diff --git a/Sources/CodeGenerator/Stages/TreeParserStage/TreeParser/TreeParser.swift b/Sources/CodeGenerator/Stages/TreeParserStage/TreeParser/TreeParser.swift index 933ce34b..fd0aa39d 100644 --- a/Sources/CodeGenerator/Stages/TreeParserStage/TreeParser/TreeParser.swift +++ b/Sources/CodeGenerator/Stages/TreeParserStage/TreeParser/TreeParser.swift @@ -59,9 +59,12 @@ public struct TreeParser { ) } + let parameters = try service.parameters.map { + try parametersParser.parse(parameter: $0, current: current, other: other) + } let operations = try service.operations.map(mapper) - return .init(path: service.path, operations: operations) + return .init(path: service.path, parameters: parameters, operations: operations) } func parse(operation: OperationNode, current: DependencyWithTree, other: [DependencyWithTree]) throws -> OperationModel { diff --git a/Sources/GASTBuilder/Builders/AnyServiceBuilder.swift b/Sources/GASTBuilder/Builders/AnyServiceBuilder.swift index 375a8ae8..5bb10789 100644 --- a/Sources/GASTBuilder/Builders/AnyServiceBuilder.swift +++ b/Sources/GASTBuilder/Builders/AnyServiceBuilder.swift @@ -48,9 +48,13 @@ public struct AnyServiceBuilder: ServiceBuilder { /// Build all item which are under `paths:` public func build(paths: [Path]) throws -> [PathNode] { return try paths.map { path in + let parameters = try wrap(buildParameters(path.parameters), + message: "While parsing path paramters for \(path.path)") let operations = try wrap(self.build(operations: path.operations), message: "While parsing Path: \(path.path)") - return PathNode(path: path.path, operations: operations) + return PathNode(path: path.path, + parameters: parameters, + operations: operations) } } } @@ -59,25 +63,7 @@ extension AnyServiceBuilder { func build(operations: [Swagger.Operation]) throws -> [OperationNode] { return try operations.map { operation in - let mapper = { (param: PossibleReference) -> Referenced in - switch param { - case .reference(let ref): - return .ref(ref.rawValue) - case .value(let val): - let params = try wrap( - // TODO: - think about how to fix emty string - self.parameterBuilder.build(parameters: [.init(name: "", value: val)]), - message: "While parsing operation's parameter \(val.name)") - - guard params.count == 1 else { - throw CommonError(message: "We had sent 1 parameter, and then got \(params.count). It's very strange. Plz contact mainteiners") - } - - return .entity(params[0]) - } - } - - let params = try wrap(operation.parameters.map(mapper), + let params = try wrap(buildParameters(operation.parameters), message: "While parsing operation \(operation.method.rawValue)") let requestBody = try wrap( @@ -97,6 +83,26 @@ extension AnyServiceBuilder { } } + func buildParameters(_ parameters: [PossibleReference]) throws -> [Referenced] { + return try parameters.map { + switch $0 { + case .reference(let ref): + return .ref(ref.rawValue) + case .value(let val): + let params = try wrap( + // TODO: - think about how to fix emty string + self.parameterBuilder.build(parameters: [.init(name: "", value: val)]), + message: "While parsing operation's parameter \(val.name)") + + guard params.count == 1 else { + throw CommonError(message: "We had sent 1 parameter, and then got \(params.count). It's very strange. Plz contact mainteiners") + } + + return .entity(params[0]) + } + } + } + func buildBody(body: PossibleReference?) throws -> Referenced? { guard let body = body else { return nil } diff --git a/Sources/GASTTree/Services/PathNode.swift b/Sources/GASTTree/Services/PathNode.swift index 0c374066..aadd4847 100644 --- a/Sources/GASTTree/Services/PathNode.swift +++ b/Sources/GASTTree/Services/PathNode.swift @@ -9,10 +9,12 @@ import Foundation public struct PathNode { public var path: String + public let parameters: [Referenced] public var operations: [OperationNode] - public init(path: String, operations: [OperationNode]) { + public init(path: String, parameters: [Referenced], operations: [OperationNode]) { self.path = path + self.parameters = parameters self.operations = operations } } diff --git a/Sources/Pipelines/CodeGen/SwaggerCorrectorStage.swift b/Sources/Pipelines/CodeGen/SwaggerCorrectorStage.swift index b568adf9..bf7363f2 100644 --- a/Sources/Pipelines/CodeGen/SwaggerCorrectorStage.swift +++ b/Sources/Pipelines/CodeGen/SwaggerCorrectorStage.swift @@ -25,9 +25,10 @@ class SwaggerCorrectorStage: PipelineStage { } private func correctService(_ service: [PathModel]) -> [PathModel] { - return service.map { path in - return PathModel(path: corrector.correctPath(path.path), - operations: path.operations) + return service.map { pathModel in + return PathModel(path: corrector.correctPath(pathModel.path), + parameters: corrector.correctPathParameters(for: pathModel), + operations: pathModel.operations) } } } diff --git a/Tests/CodeGeneratorTests/SwaggerCorrectorTests.swift b/Tests/CodeGeneratorTests/SwaggerCorrectorTests.swift index 4753d0b8..7c692585 100644 --- a/Tests/CodeGeneratorTests/SwaggerCorrectorTests.swift +++ b/Tests/CodeGeneratorTests/SwaggerCorrectorTests.swift @@ -7,7 +7,10 @@ import Foundation import XCTest -import CodeGenerator +import Pipelines +import UtilsForTesting + +@testable import CodeGenerator /// Contains cases which check that `SwaggerCorrector` finds expected issues in OpenAPI elements and, if possible, fixes them class SwaggerCorrectorTests: XCTestCase { @@ -44,4 +47,34 @@ class SwaggerCorrectorTests: XCTestCase { XCTAssertEqual(expectedPath, correctedPath) } + + /// Checks that if path parameters are declared in `Operation` instead of `Path`, corrector moves them to `Path` + func testPathParametersDeclaredInOperationAreMovedToPath() throws { + // Arrange + + let pathToRoot = "/path/to/services.yaml" + let fileProvider = FileProviderStub() + fileProvider.isReadableFile = true + fileProvider.files = [pathToRoot: SwaggerCorrectorYamls.yamlWithPathParametersInOperationWontBeParsed] + + var factory = StubGASTTreeFactory(fileProvider: fileProvider) + var result = [[PathModel]]() + factory.resultClosure = { (val: [[PathModel]]) throws -> Void in + result = val + } + let pipeline = factory.build() + + let corrector = SwaggerCorrector() + + // Act + + try pipeline.run(with: URL(string: pathToRoot)!) + let fixedParameters = corrector.correctPathParameters(for: result[0][0]) + + // Assert + + XCTAssertEqual(fixedParameters.count, 1) + XCTAssertEqual(fixedParameters.first?.value.name, "id") + } + } diff --git a/Tests/CodeGeneratorTests/SwaggerCorrectorYamls.swift b/Tests/CodeGeneratorTests/SwaggerCorrectorYamls.swift new file mode 100644 index 00000000..76580ad3 --- /dev/null +++ b/Tests/CodeGeneratorTests/SwaggerCorrectorYamls.swift @@ -0,0 +1,32 @@ +// +// SwaggerCorrectorYamls.swift +// +// +// Created by Дмитрий Демьянов on 08.11.2021. +// + +import Foundation + +enum SwaggerCorrectorYamls { + /// Contains service `messages` with one operation `get` + /// Service has path parameter `id` with type `string`, it is declared in `Operation` + static var yamlWithPathParametersInOperationWontBeParsed = """ + paths: + /messages/{id}: + get: + parameters: + - name: id + required: true + in: path + schema: + type: string + responses: + default: + description: "Все ок" + content: + application/json: + schema: + type: integer + +""".data(using: .utf8)! +} diff --git a/Tests/PipelinesTests/YamlToGenerationModelsTests/ParametersTests.swift b/Tests/PipelinesTests/YamlToGenerationModelsTests/ParametersTests.swift index 19ea198f..5bac91bd 100644 --- a/Tests/PipelinesTests/YamlToGenerationModelsTests/ParametersTests.swift +++ b/Tests/PipelinesTests/YamlToGenerationModelsTests/ParametersTests.swift @@ -22,6 +22,7 @@ import UtilsForTesting /// Cases: /// /// - Params definition: +/// - Path params declared in `Path` will be parsed /// - Params with primitive type with `in place declaration` will be parsed /// - Params with ref in schema on enum will be parsed /// - Params with ref in schema on object will be parsed @@ -30,6 +31,7 @@ import UtilsForTesting /// - Params with ref in schema on object with another ref will be parsed /// - Params with ref on cycled objects will parsed /// +/// - Path params declared in `Operation` won't be parsed /// - Params with schema definition won't be parsed /// - Parameters with ref on another parameter won't be parsed /// @@ -39,6 +41,68 @@ final class ParametersTests: XCTestCase { // MARK: - Params definition + /// Path parameters declared inside `Path` will be parsed + func testPathParametersInPathWillBeParsed() throws { + // Arrange + + let pathToRoot = "/path/to/services.yaml" + let fileProvider = FileProviderStub() + fileProvider.isReadableFile = true + fileProvider.files = [pathToRoot: ParametersTests.yamlWithPathParametersInPathWillBeParsed] + + var factory = StubGASTTreeFactory(fileProvider: fileProvider) + + var result = [[PathModel]]() + + factory.resultClosure = { (val: [[PathModel]]) throws -> Void in + result = val + } + + let pipeline = factory.build() + + // Act + + try pipeline.run(with: URL(string: pathToRoot)!) + let pathParameters = result[0][0].parameters + + // Assert + + XCTAssertEqual(pathParameters.count, 1) + + let firstParam = pathParameters.first(where: { $0.name == "id" })! + + XCTAssertEqual(try firstParam.type().primitiveType(), .string) + } + + /// Path parameters declared inside `Operation` won't be parsed + func testPathParametersInOperationWontBeParsed() throws { + // Arrange + + let pathToRoot = "/path/to/services.yaml" + let fileProvider = FileProviderStub() + fileProvider.isReadableFile = true + fileProvider.files = [pathToRoot: ParametersTests.yamlWithPathParametersInOperationWontBeParsed] + + var factory = StubGASTTreeFactory(fileProvider: fileProvider) + + var result = [[PathModel]]() + + factory.resultClosure = { (val: [[PathModel]]) throws -> Void in + result = val + } + + let pipeline = factory.build() + + // Act + + try pipeline.run(with: URL(string: pathToRoot)!) + let pathParameters = result[0][0].parameters + + // Assert + + XCTAssertEqual(pathParameters.count, 0) + } + /// Params with primitive type with `in place declaration` will be parsed func testWithPrimitiveTypeWillBeParsed() throws { // Arrange diff --git a/Tests/PipelinesTests/YamlToGenerationModelsTests/Yamls.swift b/Tests/PipelinesTests/YamlToGenerationModelsTests/Yamls.swift index ec94bfd6..2e519fdc 100644 --- a/Tests/PipelinesTests/YamlToGenerationModelsTests/Yamls.swift +++ b/Tests/PipelinesTests/YamlToGenerationModelsTests/Yamls.swift @@ -8,6 +8,51 @@ import Foundation extension ParametersTests { + + /// Contains service `messages` with one operation `get` + /// Service has path parameter `id` with type `string`, it is declared in `Path` + static var yamlWithPathParametersInPathWillBeParsed = """ + paths: + /messages/{id}: + parameters: + - name: id + required: true + in: path + schema: + type: string + get: + responses: + default: + description: "Все ок" + content: + application/json: + schema: + type: integer + +""".data(using: .utf8)! + + /// Contains service `messages` with one operation `get` + /// Service has path parameter `id` with type `string`, it is declared in `Operation` + static var yamlWithPathParametersInOperationWontBeParsed = """ + paths: + /messages/{id}: + get: + parameters: + - name: id + required: true + in: path + schema: + type: string + responses: + default: + description: "Все ок" + content: + application/json: + schema: + type: integer + +""".data(using: .utf8)! + /// Contains service `messages` with one operation `get` /// And the operation contains two parameters `id2` and `id3` with `integer` and `string` types /// Each parameter is declared `in place`