Skip to content

Commit d154389

Browse files
mattpolziniT0nysimonjbeaumont
authored
Update OpenAPIKit to v6 (#875)
This PR builds upon #839 to update from OpenAPIKit 4.x to OpenAPIKit ~5.0~ 6.0. ### Motivation Updating to OpenAPIKit v4.x enables efforts to add external loading support to the generator (as a separate future project). This means being able to read in OpenAPI Documents spread across multiple files. Updating to OpenAPIKit v5.x allows the generator project to support OAS 3.2.0 features and take advantage of various other improvements to OpenAPIKit that required breaking changes to be made. Updating to OpenAPIKit v6.0 allows this project to turn the `ExternalLoading` trait off (on by default). Turning this trait off saves 50% of the compile time of OpenAPKit for as long as `swift-openapi-generator` is not using the external loading features of OpenAPKit anyway. ### Modifications - Update OpenAPIKit dependency to v6.x - Remove code that parsed OAS 3.2.0 documents as 3.1.x documents; OpenAPIKit v5 supports OAS 3.2.0 documents natively. - Make changes required by move to new OpenAPIKit version (following migration guide). - Introduce `assumeLookupOnce()` to temporarily maintain the existing invariant that no entries in the Components Object are themselves references; replace all `lookup()` calls with `assumeLookupOnce()`. See the **Note** on this below. - Fix code that looked for references on JSON Schemas under the assumption that they were wrapped in `OpenAPI.Reference`. This assumption does not hold in OpenAPIKit v5. **NOTE on `assumeLookupOnce()`** The new `assumeLookupOnce()` function is intended to allow for gradual migration. The generator had previously used `lookup()` which used to look at the Components Object and either return an entry or throw. Going forward, `lookup()` does recursive lookup until a non-reference is found and `lookupOnce()` acts more like `lookup()` used to act -- however, `lookupOnce()` has a different method signature owing to the fact that it may return a reference instead of the ultimately desired object. The need for these changes arise from the fact that OpenAPIKit v5 now allows entries in the Components Object to themselves be references. This has been supported by the OpenAPI Standard for some time but OpenAPIKit only introduced support for it with v5. Therefore, `assumeLookupOnce()` was added as a way to throw an error if a reference is found as an entry in the Components Object (behavior already exhibited by the generator project) but the goal should be to remove uses of `assumeLookupOnce()` over time and replace them with either `lookupOnce()` or `lookup()` depending on the needs of each call site. In this way, the generator project will move closer to supporting Component Object reference entries without the overhead of changing every call site all at once. ### Result Theoretically, nothing functionally changes. ### Test Plan Tests updated, `swift test` passing, "Compatibility test" suite passing. --------- Co-authored-by: Anton Titkov <atitkov@me.com> Co-authored-by: Anton Titkov <iT0ny@users.noreply.github.com> Co-authored-by: Si Beaumont <beaumont@apple.com>
1 parent 0d5694c commit d154389

23 files changed

Lines changed: 295 additions & 187 deletions

File tree

‎Package.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ let package = Package(
4646
.package(url: "https://github.com/apple/swift-collections", from: "1.1.4"),
4747

4848
// Read OpenAPI documents
49-
.package(url: "https://github.com/mattpolzin/OpenAPIKit", from: "3.9.0"),
49+
.package(url: "https://github.com/mattpolzin/OpenAPIKit", from: "6.0.0", traits: []),
5050
.package(url: "https://github.com/jpsim/Yams", "4.0.0"..<"7.0.0"),
5151

5252
// CLI Tool
@@ -165,4 +165,4 @@ for target in package.targets {
165165
case .macro, .plugin, .system, .binary: () // not applicable
166166
@unknown default: () // we don't know what to do here, do nothing
167167
}
168-
}// --- END: STANDARD CROSS-REPO SETTINGS DO NOT EDIT --- //
168+
} // --- END: STANDARD CROSS-REPO SETTINGS DO NOT EDIT --- //

‎Sources/_OpenAPIGeneratorCore/Extensions/OpenAPIKit.swift‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,42 @@ extension Either {
2222
/// - Throws: An error if there's an issue looking up the value in the components.
2323
func resolve(in components: OpenAPI.Components) throws -> B where A == OpenAPI.Reference<B> {
2424
switch self {
25-
case let .a(a): return try components.lookup(a)
25+
case let .a(a): return try components.assumeLookupOnce(a)
2626
case let .b(b): return b
2727
}
2828
}
2929
}
3030

31+
extension OpenAPI.Components {
32+
func assumeLookupOnce<ReferenceType: ComponentDictionaryLocatable>(_ reference: OpenAPI.Reference<ReferenceType>)
33+
throws -> ReferenceType
34+
{
35+
guard let result = try lookupOnce(reference).b else {
36+
throw JSONReferenceParsingError.componentsReferenceEntryUnsupported(reference.absoluteString)
37+
}
38+
return result
39+
}
40+
41+
func assumeLookupOnce<ReferenceType: ComponentDictionaryLocatable>(_ reference: JSONReference<ReferenceType>) throws
42+
-> ReferenceType
43+
{
44+
guard let result = try lookupOnce(reference).b else {
45+
throw JSONReferenceParsingError.componentsReferenceEntryUnsupported(reference.absoluteString)
46+
}
47+
return result
48+
}
49+
50+
func assumeLookupOnce<ReferenceType: ComponentDictionaryLocatable>(
51+
_ maybeReference: Either<OpenAPI.Reference<ReferenceType>, ReferenceType>
52+
) throws -> ReferenceType {
53+
guard let result = try lookupOnce(maybeReference).b else {
54+
throw JSONReferenceParsingError.componentsReferenceEntryUnsupported(maybeReference.a?.absoluteString)
55+
}
56+
return result
57+
}
58+
59+
}
60+
3161
extension JSONSchema.Schema {
3262

3363
/// Returns the name of the schema.

‎Sources/_OpenAPIGeneratorCore/Hooks/FilteredDocument.swift‎

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ struct FilteredDocumentBuilder {
103103
guard let methods = requiredEndpoints[path] else { continue }
104104
switch pathItem {
105105
case .a(let reference):
106-
components.pathItems[try reference.internalComponentKey] = try document.components.lookup(reference)
107-
.filteringEndpoints { methods.contains($0.method) }
106+
components.pathItems[try reference.internalComponentKey] = try document.components
107+
.assumeLookupOnce(reference).filteringEndpoints { methods.contains($0.method) }
108108
case .b(let pathItem):
109109
filteredDocument.paths[path] = .b(pathItem.filteringEndpoints { methods.contains($0.method) })
110110
}
@@ -169,7 +169,7 @@ struct FilteredDocumentBuilder {
169169
/// - Parameter name: The key in the `#/components/schemas` map in the OpenAPI document.
170170
/// - Throws: If the named schema does not exist in original OpenAPI document.
171171
mutating func includeSchema(_ name: String) throws {
172-
try includeSchema(.a(OpenAPI.Reference<JSONSchema>.component(named: name)))
172+
try includeComponentsReferencedBy(.reference(.component(named: name)))
173173
}
174174
}
175175

@@ -198,16 +198,7 @@ private extension FilteredDocumentBuilder {
198198
switch maybeReference {
199199
case .a(let reference):
200200
guard try requiredPathItemReferences.insert(reference.internalComponentKey).inserted else { return }
201-
try includeComponentsReferencedBy(try document.components.lookup(reference))
202-
case .b(let value): try includeComponentsReferencedBy(value)
203-
}
204-
}
205-
206-
mutating func includeSchema(_ maybeReference: Either<OpenAPI.Reference<JSONSchema>, JSONSchema>) throws {
207-
switch maybeReference {
208-
case .a(let reference):
209-
guard try requiredSchemaReferences.insert(reference.internalComponentKey).inserted else { return }
210-
try includeComponentsReferencedBy(try document.components.lookup(reference))
201+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
211202
case .b(let value): try includeComponentsReferencedBy(value)
212203
}
213204
}
@@ -218,7 +209,7 @@ private extension FilteredDocumentBuilder {
218209
switch maybeReference {
219210
case .a(let reference):
220211
guard try requiredParameterReferences.insert(reference.internalComponentKey).inserted else { return }
221-
try includeComponentsReferencedBy(try document.components.lookup(reference))
212+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
222213
case .b(let value): try includeComponentsReferencedBy(value)
223214
}
224215
}
@@ -229,7 +220,7 @@ private extension FilteredDocumentBuilder {
229220
switch maybeReference {
230221
case .a(let reference):
231222
guard try requiredResponseReferences.insert(reference.internalComponentKey).inserted else { return }
232-
try includeComponentsReferencedBy(try document.components.lookup(reference))
223+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
233224
case .b(let value): try includeComponentsReferencedBy(value)
234225
}
235226
}
@@ -238,7 +229,7 @@ private extension FilteredDocumentBuilder {
238229
switch maybeReference {
239230
case .a(let reference):
240231
guard try requiredHeaderReferences.insert(reference.internalComponentKey).inserted else { return }
241-
try includeComponentsReferencedBy(try document.components.lookup(reference))
232+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
242233
case .b(let value): try includeComponentsReferencedBy(value)
243234
}
244235
}
@@ -247,7 +238,7 @@ private extension FilteredDocumentBuilder {
247238
switch maybeReference {
248239
case .a(let reference):
249240
guard try requiredLinkReferences.insert(reference.internalComponentKey).inserted else { return }
250-
try includeComponentsReferencedBy(try document.components.lookup(reference))
241+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
251242
case .b(let value): try includeComponentsReferencedBy(value)
252243
}
253244
}
@@ -258,7 +249,7 @@ private extension FilteredDocumentBuilder {
258249
switch maybeReference {
259250
case .a(let reference):
260251
guard try requiredCallbacksReferences.insert(reference.internalComponentKey).inserted else { return }
261-
try includeComponentsReferencedBy(try document.components.lookup(reference))
252+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
262253
case .b(let value): try includeComponentsReferencedBy(value)
263254
}
264255
}
@@ -269,7 +260,7 @@ private extension FilteredDocumentBuilder {
269260
switch maybeReference {
270261
case .a(let reference):
271262
guard try requiredRequestReferences.insert(reference.internalComponentKey).inserted else { return }
272-
try includeComponentsReferencedBy(try document.components.lookup(reference))
263+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
273264
case .b(let value): try includeComponentsReferencedBy(value)
274265
}
275266
}
@@ -278,7 +269,7 @@ private extension FilteredDocumentBuilder {
278269
switch maybeReference {
279270
case .a(let reference):
280271
guard try requiredExampleReferences.insert(reference.internalComponentKey).inserted else { return }
281-
try includeComponentsReferencedBy(try document.components.lookup(reference))
272+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
282273
case .b(let value): try includeComponentsReferencedBy(value)
283274
}
284275
}
@@ -287,7 +278,7 @@ private extension FilteredDocumentBuilder {
287278
for (path, maybePathItemReference) in document.paths {
288279
let originalPathItem: OpenAPI.PathItem
289280
switch maybePathItemReference {
290-
case .a(let reference): originalPathItem = try document.components.lookup(reference)
281+
case .a(let reference): originalPathItem = try document.components.assumeLookupOnce(reference)
291282
case .b(let pathItem): originalPathItem = pathItem
292283
}
293284

@@ -330,7 +321,7 @@ private extension FilteredDocumentBuilder {
330321
case .reference(let reference, _):
331322
let referenceKey = try OpenAPI.ComponentKey(stringLiteral: reference.requiredName)
332323
guard requiredSchemaReferences.insert(referenceKey).inserted else { return }
333-
try includeComponentsReferencedBy(document.components.lookup(reference))
324+
try includeComponentsReferencedBy(document.components.lookupOnce(reference).flattenToJsonSchema())
334325

335326
case .object(_, let object):
336327
for schema in object.properties.values { try includeComponentsReferencedBy(schema) }
@@ -362,20 +353,22 @@ private extension FilteredDocumentBuilder {
362353
switch schemaContext.schema {
363354
case .a(let reference):
364355
guard try requiredSchemaReferences.insert(reference.internalComponentKey).inserted else { return }
365-
try includeComponentsReferencedBy(try document.components.lookup(reference))
356+
try includeComponentsReferencedBy(try document.components.assumeLookupOnce(reference))
366357
case .b(let schema): try includeComponentsReferencedBy(schema)
367358
}
368-
case .b(let contentMap):
369-
for value in contentMap.values {
370-
switch value.schema {
371-
case .a(let reference):
372-
guard try requiredSchemaReferences.insert(reference.internalComponentKey).inserted else { return }
373-
try includeComponentsReferencedBy(try document.components.lookup(reference))
374-
case .b(let schema): try includeComponentsReferencedBy(schema)
375-
case .none: continue
376-
}
377-
}
359+
case .b(let contentMap): for value in contentMap.values { try includeComponentsReferencedBy(value) }
360+
}
361+
}
362+
363+
mutating func includeComponentsReferencedBy(
364+
_ contentMapEntry: Either<OpenAPI.Reference<OpenAPI.Content>, OpenAPI.Content>
365+
) throws {
366+
let content: OpenAPI.Content
367+
switch contentMapEntry {
368+
case .a(let ref): content = try document.components.assumeLookupOnce(ref)
369+
case .b(let value): content = value
378370
}
371+
try includeComponentsReferencedBy(content)
379372
}
380373

381374
mutating func includeComponentsReferencedBy(_ response: OpenAPI.Response) throws {
@@ -385,8 +378,8 @@ private extension FilteredDocumentBuilder {
385378
}
386379

387380
mutating func includeComponentsReferencedBy(_ content: OpenAPI.Content) throws {
388-
if let schema = content.schema { try includeSchema(schema) }
389-
if let encoding = content.encoding {
381+
if let schema = content.schema { try includeComponentsReferencedBy(schema) }
382+
if let encoding = content.encodingMap {
390383
for encoding in encoding.values {
391384
if let headers = encoding.headers { for header in headers.values { try includeHeader(header) } }
392385
}

‎Sources/_OpenAPIGeneratorCore/Parser/YamsParser.swift‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,6 @@ public struct YamsParser: ParserProtocol {
5454
let decoder = YAMLDecoder()
5555
let openapiData = input.contents
5656

57-
let decodingOptions = [
58-
DocumentConfiguration.versionMapKey: [
59-
// Until we move to OpenAPIKit v5.0+ we will parse OAS 3.2.0 as if it were OAS 3.1.2
60-
"3.2.0": OpenAPI.Document.Version.v3_1_2
61-
]
62-
]
63-
6457
struct OpenAPIVersionedDocument: Decodable { var openapi: String? }
6558

6659
let versionedDocument: OpenAPIVersionedDocument
@@ -82,12 +75,7 @@ public struct YamsParser: ParserProtocol {
8275
document = openAPI30Document.convert(to: .v3_1_0)
8376
case "3.1.0", "3.1.1", "3.1.2":
8477
document = try decoder.decode(OpenAPIKit.OpenAPI.Document.self, from: input.contents)
85-
case "3.2.0":
86-
document = try decoder.decode(
87-
OpenAPIKit.OpenAPI.Document.self,
88-
from: input.contents,
89-
userInfo: decodingOptions
90-
)
78+
case "3.2.0": document = try decoder.decode(OpenAPIKit.OpenAPI.Document.self, from: input.contents)
9179
default:
9280
throw Diagnostic.openAPIVersionError(
9381
versionString: "openapi: \(openAPIVersion)",

‎Sources/_OpenAPIGeneratorCore/Parser/validateDoc.swift‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func validateContentTypes(in doc: ParsedOpenAPIRepresentation, validate: (String
6767
}
6868

6969
for (key, component) in doc.components.requestBodies {
70+
let component = try doc.components.assumeLookupOnce(component)
7071
for contentType in component.content.keys {
7172
if !validate(contentType.rawValue) {
7273
throw Diagnostic.error(
@@ -81,6 +82,7 @@ func validateContentTypes(in doc: ParsedOpenAPIRepresentation, validate: (String
8182
}
8283

8384
for (key, component) in doc.components.responses {
85+
let component = try doc.components.assumeLookupOnce(component)
8486
for contentType in component.content.keys {
8587
if !validate(contentType.rawValue) {
8688
throw Diagnostic.error(
@@ -124,21 +126,26 @@ func validateReferences(in doc: ParsedOpenAPIRepresentation) throws {
124126

125127
func validateReferencesInContentTypes(_ content: OpenAPI.Content.Map, location: String) throws {
126128
for (contentKey, contentType) in content {
127-
if let reference = contentType.schema?.reference {
128-
try validateReference(
129-
reference,
130-
in: doc.components,
131-
location: location + "/content/\(contentKey.rawValue)/schema"
132-
)
133-
}
134-
if let eitherExamples = contentType.examples?.values {
135-
for example in eitherExamples {
136-
if let reference = example.reference {
137-
try validateReference(
138-
reference,
139-
in: doc.components,
140-
location: location + "/content/\(contentKey.rawValue)/examples"
141-
)
129+
switch contentType {
130+
case .a(let ref):
131+
try validateReference(ref, in: doc.components, location: location + "/content/\(contentKey.rawValue)")
132+
case .b(let contentType):
133+
if let reference: JSONReference<JSONSchema> = contentType.schema?.reference {
134+
try validateReference(
135+
.init(reference),
136+
in: doc.components,
137+
location: location + "/content/\(contentKey.rawValue)/schema"
138+
)
139+
}
140+
if let eitherExamples = contentType.examples?.values {
141+
for example in eitherExamples {
142+
if let reference = example.reference {
143+
try validateReference(
144+
reference,
145+
in: doc.components,
146+
location: location + "/content/\(contentKey.rawValue)/examples"
147+
)
148+
}
142149
}
143150
}
144151
}

‎Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ extension FileTranslator {
100100
// In nullable enum schemas, empty strings are parsed as Void.
101101
// This is unlikely to be fixed, so handling that case here.
102102
// https://github.com/apple/swift-openapi-generator/issues/118
103-
if isNullable && anyValue is Void {
103+
// Also handle nil values in nullable schemas.
104+
let isNullValue = anyValue is Void || (anyValue as? String) == nil
105+
if isNullable && isNullValue {
104106
try addIfUnique(id: .string(""), caseName: context.safeNameGenerator.swiftMemberName(for: ""))
105107
} else {
106108
guard let rawValue = anyValue as? String else {

‎Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ extension FileTranslator {
9797
return try contents.compactMap { key, value in
9898
try parseContentIfSupported(
9999
contentKey: key,
100-
contentValue: value,
100+
contentValue: try components.assumeLookupOnce(value),
101101
excludeBinary: excludeBinary,
102102
isRequired: isRequired,
103103
foundIn: foundIn + "/\(key.rawValue)"
@@ -129,12 +129,17 @@ extension FileTranslator {
129129

130130
let chosenContent: (type: ContentType, schema: SchemaContent, content: OpenAPI.Content)?
131131
if let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isJSON }) {
132-
chosenContent = (contentType, .init(contentType: contentType, schema: contentValue.schema), contentValue)
132+
let contentValue = try components.assumeLookupOnce(contentValue)
133+
chosenContent = (
134+
contentType, .init(contentType: contentType, schema: contentValue.schema.map(Either.schema)),
135+
contentValue
136+
)
133137
} else if !excludeBinary,
134138
let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isBinary })
135139
{
140+
let contentValue = try components.assumeLookupOnce(contentValue)
136141
chosenContent = (
137-
contentType, .init(contentType: contentType, schema: .b(.string(contentEncoding: .binary))),
142+
contentType, .init(contentType: contentType, schema: .schema(.string(contentEncoding: .binary))),
138143
contentValue
139144
)
140145
} else {
@@ -188,8 +193,10 @@ extension FileTranslator {
188193
foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)"
189194
)
190195
}
191-
if contentType.isJSON { return .init(contentType: contentType, schema: contentValue.schema) }
192-
if contentType.isUrlEncodedForm { return .init(contentType: contentType, schema: contentValue.schema) }
196+
if contentType.isJSON { return .init(contentType: contentType, schema: contentValue.schema.map(Either.schema)) }
197+
if contentType.isUrlEncodedForm {
198+
return .init(contentType: contentType, schema: contentValue.schema.map(Either.schema))
199+
}
193200
if contentType.isMultipart {
194201
guard isRequired else {
195202
try diagnostics.emit(
@@ -201,10 +208,14 @@ extension FileTranslator {
201208
)
202209
return nil
203210
}
204-
return .init(contentType: contentType, schema: contentValue.schema, encoding: contentValue.encoding)
211+
return .init(
212+
contentType: contentType,
213+
schema: contentValue.schema.map(Either.schema),
214+
encoding: contentValue.encodingMap
215+
)
205216
}
206217
if !excludeBinary, contentType.isBinary {
207-
return .init(contentType: contentType, schema: .b(.string(contentEncoding: .binary)))
218+
return .init(contentType: contentType, schema: .schema(.string(contentEncoding: .binary)))
208219
}
209220
try diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
210221
return nil

0 commit comments

Comments
 (0)