Skip to content

Commit fd64063

Browse files
authored
Refactor ContentType + improve content type matching (#173)
Refactor ContentType + improve content type matching ### Motivation The `ContentType` type had two issues: - the enum where all cases had the same associated string value was not a good representation of the data - the parsing was too strict and didn't match e.g. `foo/bar+json` as a JSON content type ### Modifications Refactored `ContentType` to split the raw value storage from the `Category` (new term, ideas of a better name are welcome) of content types (`json`, `text`, or `binary`). Also, improved the mapping to match https://json-schema.org/draft/2020-12/json-schema-core.html#section-4.2 ### Result Now the type is easier to work with, and parsing correctly recognizes `foo/bar+json` as JSON. ### Test Plan Added unit tests. Reviewed by: gjcairo, glbrntt Builds: ✔︎ pull request validation (5.8) - Build finished. ✔︎ pull request validation (5.9) - Build finished. ✔︎ pull request validation (docc test) - Build finished. ✔︎ pull request validation (integration test) - Build finished. ✔︎ pull request validation (nightly) - Build finished. ✔︎ pull request validation (soundness) - Build finished. #173
1 parent d22ddfe commit fd64063

File tree

4 files changed

+297
-91
lines changed

4 files changed

+297
-91
lines changed

Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -162,30 +162,26 @@ extension FileTranslator {
162162
)
163163
}
164164
let chosenContent: (SchemaContent, OpenAPI.Content)?
165-
if let (contentKey, contentValue) = map.first(where: { $0.key.isJSON }),
165+
if let (contentKey, contentValue) = map.first(where: { $0.key.isJSON }) {
166166
let contentType = ContentType(contentKey.typeAndSubtype)
167-
{
168167
chosenContent = (
169168
.init(
170169
contentType: contentType,
171170
schema: contentValue.schema
172171
),
173172
contentValue
174173
)
175-
} else if let (contentKey, contentValue) = map.first(where: { $0.key.isText }),
174+
} else if let (contentKey, contentValue) = map.first(where: { $0.key.isText }) {
176175
let contentType = ContentType(contentKey.typeAndSubtype)
177-
{
178176
chosenContent = (
179177
.init(
180178
contentType: contentType,
181179
schema: .b(.string)
182180
),
183181
contentValue
184182
)
185-
} else if !excludeBinary,
186-
let (contentKey, contentValue) = map.first(where: { $0.key.isBinary }),
183+
} else if !excludeBinary, let (contentKey, contentValue) = map.first(where: { $0.key.isBinary }) {
187184
let contentType = ContentType(contentKey.typeAndSubtype)
188-
{
189185
chosenContent = (
190186
.init(
191187
contentType: contentType,
@@ -201,12 +197,14 @@ extension FileTranslator {
201197
chosenContent = nil
202198
}
203199
if let chosenContent {
204-
let rawMIMEType = chosenContent.0.contentType.rawMIMEType
205-
if rawMIMEType.hasPrefix("multipart/") || rawMIMEType.contains("application/x-www-form-urlencoded") {
200+
let contentType = chosenContent.0.contentType
201+
if contentType.lowercasedType == "multipart"
202+
|| contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded")
203+
{
206204
diagnostics.emitUnsupportedIfNotNil(
207205
chosenContent.1.encoding,
208206
"Custom encoding for JSON content",
209-
foundIn: "\(foundIn), content \(rawMIMEType)"
207+
foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)"
210208
)
211209
}
212210
}
@@ -234,9 +232,8 @@ extension FileTranslator {
234232
excludeBinary: Bool = false,
235233
foundIn: String
236234
) -> SchemaContent? {
237-
if contentKey.isJSON,
235+
if contentKey.isJSON {
238236
let contentType = ContentType(contentKey.typeAndSubtype)
239-
{
240237
diagnostics.emitUnsupportedIfNotNil(
241238
contentValue.encoding,
242239
"Custom encoding for JSON content",
@@ -247,18 +244,15 @@ extension FileTranslator {
247244
schema: contentValue.schema
248245
)
249246
}
250-
if contentKey.isText,
247+
if contentKey.isText {
251248
let contentType = ContentType(contentKey.typeAndSubtype)
252-
{
253249
return .init(
254250
contentType: contentType,
255251
schema: .b(.string)
256252
)
257253
}
258-
if !excludeBinary,
259-
contentKey.isBinary,
254+
if !excludeBinary, contentKey.isBinary {
260255
let contentType = ContentType(contentKey.typeAndSubtype)
261-
{
262256
return .init(
263257
contentType: contentType,
264258
schema: .b(.string(format: .binary))

Sources/_OpenAPIGeneratorCore/Translator/Content/ContentSwiftName.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ extension FileTranslator {
2121
/// - Parameter contentType: The content type for which to compute the name.
2222
func contentSwiftName(_ contentType: ContentType) -> String {
2323
if config.featureFlags.contains(.multipleContentTypes) {
24-
let rawMIMEType = contentType.rawMIMEType
24+
let rawMIMEType = contentType.lowercasedTypeAndSubtype
2525
switch rawMIMEType {
2626
case "application/json":
2727
return "json"
@@ -41,7 +41,7 @@ extension FileTranslator {
4141
return swiftSafeName(for: rawMIMEType)
4242
}
4343
} else {
44-
switch contentType {
44+
switch contentType.category {
4545
case .json:
4646
return "json"
4747
case .text:

Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift

Lines changed: 141 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -17,103 +17,175 @@ import OpenAPIKit30
1717
///
1818
/// Represents the serialization method of the payload and affects
1919
/// the generated serialization code.
20-
enum ContentType: Hashable {
20+
struct ContentType: Hashable {
2121

22-
/// A content type for JSON.
23-
case json(String)
22+
/// The category of a content type.
23+
///
24+
/// This categorization helps the generator decide how to handle
25+
/// the content's raw body data.
26+
enum Category: Hashable {
27+
28+
/// A content type for JSON.
29+
///
30+
/// The bytes are provided to a JSON encoder or decoder.
31+
case json
32+
33+
/// A content type for any plain text.
34+
///
35+
/// The bytes are encoded or decoded as a UTF-8 string.
36+
case text
37+
38+
/// A content type for raw binary data.
39+
///
40+
/// This case covers both explicit binary data content types, such
41+
/// as `application/octet-stream`, and content types that no further
42+
/// introspection is performed on, such as `image/png`.
43+
///
44+
/// The bytes are not further processed, they are instead passed along
45+
/// either to the network (requests) or to the caller (responses).
46+
case binary
47+
48+
/// Creates a category from the provided type and subtype.
49+
///
50+
/// First checks if the provided content type is a JSON, then text,
51+
/// and uses binary if none of the two match.
52+
/// - Parameters:
53+
/// - type: The first component of the MIME type.
54+
/// - subtype: The second component of the MIME type.
55+
init(type: String, subtype: String) {
56+
let lowercasedType = type.lowercased()
57+
let lowercasedSubtype = subtype.lowercased()
58+
59+
// https://json-schema.org/draft/2020-12/json-schema-core.html#section-4.2
60+
if (lowercasedType == "application" && lowercasedSubtype == "json") || lowercasedSubtype.hasSuffix("+json")
61+
{
62+
self = .json
63+
} else if lowercasedType == "text" {
64+
self = .text
65+
} else {
66+
self = .binary
67+
}
68+
}
2469

25-
/// A content type for any plain text.
26-
case text(String)
70+
/// The coding strategy appropriate for this content type.
71+
var codingStrategy: CodingStrategy {
72+
switch self {
73+
case .json:
74+
return .json
75+
case .text:
76+
return .text
77+
case .binary:
78+
return .binary
79+
}
80+
}
81+
}
2782

28-
/// A content type for raw binary data.
29-
case binary(String)
83+
/// The mapped content type category.
84+
var category: Category {
85+
Category(type: type, subtype: subtype)
86+
}
87+
88+
/// The first component of the MIME type.
89+
private let type: String
90+
91+
/// The first component of the MIME type, as a lowercase string.
92+
///
93+
/// The raw value in its original casing is only provided by `rawTypeAndSubtype`.
94+
var lowercasedType: String {
95+
type.lowercased()
96+
}
97+
98+
/// The second component of the MIME type.
99+
private let subtype: String
100+
101+
/// The second component of the MIME type, as a lowercase string.
102+
///
103+
/// The raw value in its original casing is only provided by `originallyCasedTypeAndSubtype`.
104+
var lowercasedSubtype: String {
105+
subtype.lowercased()
106+
}
30107

31108
/// Creates a new content type by parsing the specified MIME type.
32-
/// - Parameter rawValue: A MIME type, for example "application/json".
33-
init?(_ rawValue: String) {
34-
if rawValue.hasPrefix("application/") && rawValue.hasSuffix("json") {
35-
self = .json(rawValue)
36-
return
37-
}
38-
if rawValue.hasPrefix("text/") {
39-
self = .text(rawValue)
40-
return
41-
}
42-
self = .binary(rawValue)
109+
/// - Parameter rawValue: A MIME type, for example "application/json". Must
110+
/// not be empty.
111+
init(_ rawValue: String) {
112+
precondition(!rawValue.isEmpty, "rawValue of a ContentType cannot be empty.")
113+
let rawTypeAndSubtype =
114+
rawValue
115+
.split(separator: ";")[0]
116+
.trimmingCharacters(in: .whitespaces)
117+
let typeAndSubtype =
118+
rawTypeAndSubtype
119+
.split(separator: "/")
120+
.map(String.init)
121+
precondition(
122+
typeAndSubtype.count == 2,
123+
"Invalid ContentType string, must have 2 components separated by a slash."
124+
)
125+
self.type = typeAndSubtype[0]
126+
self.subtype = typeAndSubtype[1]
43127
}
44128

45-
/// Returns the original raw MIME type.
46-
var rawMIMEType: String {
47-
switch self {
48-
case .json(let string), .text(let string), .binary(let string):
49-
return string
50-
}
129+
/// Returns the type and subtype as a "<type>/<subtype>" string.
130+
///
131+
/// Respects the original casing provided as input.
132+
var originallyCasedTypeAndSubtype: String {
133+
"\(type)/\(subtype)"
134+
}
135+
136+
/// Returns the type and subtype as a "<type>/<subtype>" string.
137+
///
138+
/// Lowercased to ease case-insensitive comparisons.
139+
var lowercasedTypeAndSubtype: String {
140+
"\(lowercasedType)/\(lowercasedSubtype)"
51141
}
52142

53143
/// The header value used when sending a content-type header.
54144
var headerValueForSending: String {
55-
switch self {
56-
case .json(let string):
57-
// We always encode JSON using JSONEncoder which uses UTF-8.
58-
return string + "; charset=utf-8"
59-
case .text(let string):
60-
return string
61-
case .binary(let string):
62-
return string
145+
guard case .json = category else {
146+
return lowercasedTypeAndSubtype
63147
}
148+
// We always encode JSON using JSONEncoder which uses UTF-8.
149+
return lowercasedTypeAndSubtype + "; charset=utf-8"
64150
}
65151

66152
/// The header value used when validating a content-type header.
67153
///
68154
/// This should be less strict, e.g. not require `charset`.
69155
var headerValueForValidation: String {
70-
switch self {
71-
case .json(let string):
72-
return string
73-
case .text(let string):
74-
return string
75-
case .binary(let string):
76-
return string
77-
}
156+
lowercasedTypeAndSubtype
78157
}
79158

80159
/// The coding strategy appropriate for this content type.
81160
var codingStrategy: CodingStrategy {
82-
switch self {
83-
case .json:
84-
return .json
85-
case .text:
86-
return .text
87-
case .binary:
88-
return .binary
89-
}
161+
category.codingStrategy
90162
}
91163

92164
/// A Boolean value that indicates whether the content type
93165
/// is a type of JSON.
94166
var isJSON: Bool {
95-
if case .json = self {
96-
return true
97-
}
98-
return false
167+
category == .json
99168
}
100169

101170
/// A Boolean value that indicates whether the content type
102171
/// is a type of plain text.
103172
var isText: Bool {
104-
if case .text = self {
105-
return true
106-
}
107-
return false
173+
category == .text
108174
}
109175

110176
/// A Boolean value that indicates whether the content type
111177
/// is just binary data.
112178
var isBinary: Bool {
113-
if case .binary = self {
114-
return true
115-
}
116-
return false
179+
category == .binary
180+
}
181+
182+
static func == (lhs: Self, rhs: Self) -> Bool {
183+
// MIME type equality is case-insensitive.
184+
lhs.lowercasedTypeAndSubtype == rhs.lowercasedTypeAndSubtype
185+
}
186+
187+
func hash(into hasher: inout Hasher) {
188+
hasher.combine(lowercasedTypeAndSubtype)
117189
}
118190
}
119191

@@ -122,27 +194,24 @@ extension OpenAPI.ContentType {
122194
/// A Boolean value that indicates whether the content type
123195
/// is a type of JSON.
124196
var isJSON: Bool {
125-
guard let contentType = ContentType(typeAndSubtype) else {
126-
return false
127-
}
128-
return contentType.isJSON
197+
asGeneratorContentType.isJSON
129198
}
130199

131200
/// A Boolean value that indicates whether the content type
132201
/// is a type of plain text.
133202
var isText: Bool {
134-
guard let contentType = ContentType(typeAndSubtype) else {
135-
return false
136-
}
137-
return contentType.isText
203+
asGeneratorContentType.isText
138204
}
139205

140206
/// A Boolean value that indicates whether the content type
141207
/// is just binary data.
142208
var isBinary: Bool {
143-
guard let contentType = ContentType(typeAndSubtype) else {
144-
return false
145-
}
146-
return contentType.isBinary
209+
asGeneratorContentType.isBinary
210+
}
211+
212+
/// Returns the content type wrapped in the generator's representation
213+
/// of a content type, as opposed to the one from OpenAPIKit.
214+
var asGeneratorContentType: ContentType {
215+
ContentType(typeAndSubtype)
147216
}
148217
}

0 commit comments

Comments
 (0)