Add example support. #10

n8chur · 2017-05-31T21:34:02Z

Adds example support to schema properties.
Adds support for x-example on parameter definitions.
Adds an other(String) type to StringFormat.

- Adds `example` support to schema properties. - Adds support for `x-example` on parameter definitions. - Adds an `other(String)` type to `StringFormat`.

# Conflicts: # Sources/Metadata.swift # Tests/SwaggerParserTests/SwaggerParserTests.swift

AttilaTheFun

I made some suggestions about how I think we could clean up StringFormat while adding the new functionality you suggested.

AttilaTheFun · 2017-06-02T02:45:32Z

Sources/Enums.swift

-public enum StringFormat: String {
+public enum StringFormat: RawRepresentable {
+
+    public typealias RawValue = String

    /// Base64 encoded characters
    case byte


Let's add the following options, per the JSON Schema spec:

"date-time": Date representation, as defined by RFC 3339, section 5.6.
"email": Internet email address, see RFC 5322, section 3.4.1.
"hostname": Internet host name, see RFC 1034, section 3.1.
"ipv4": IPv4 address, according to dotted-quad ABNF syntax as defined in RFC 2673, section 3.2.
"ipv6": IPv6 address, as defined in RFC 2373, section 2.2.
"uri": A universal resource identifier (URI), according to RFC3986.

As well as our added byte, binary, url and other options.

AttilaTheFun · 2017-06-02T02:51:10Z

Sources/Enums.swift

@@ -1,6 +1,8 @@
 import Foundation

-public enum StringFormat: String {
+public enum StringFormat: RawRepresentable {


How about we move StringFormat into its own file, StringFormat.swift

AttilaTheFun · 2017-06-03T22:20:00Z

Tests/SwaggerParserTests/SwaggerParserTests.swift

+        }
+
+        guard
+            let aStringFormat = aStringOptionalFormat,


How about anOptionalStringFormat

AttilaTheFun · 2017-06-12T21:45:33Z

Oh I don't know how it lost my comment, but I suggested defining a nested, private, RawStringFormat enum which is strongly-typed instead of defining a bunch of private static raw value strings @n8chur

AttilaTheFun · 2017-06-13T21:15:29Z

Sources/StringFormat.swift

+        }
+    }
+
+    private enum RawStringFormat: String {


Looks Good :)

AttilaTheFun

Looks pretty good - just added a few more comments.

AttilaTheFun · 2017-06-13T21:20:34Z

Sources/StringFormat.swift

+    /// A universal resource identifier (URI), according to RFC3986.
+    case uri
+
+    public init(rawValue: RawValue) {


Once you've typealiased RawValue = String, you can use String everywhere else in the type.

AttilaTheFun · 2017-06-13T21:40:15Z

Sources/StringFormat.swift

+
+    public var rawValue: RawValue {
+        switch self {
+        case .byte: return RawStringFormat.byte.rawValue


How about making a private function that returns the raw string format optionally? E.g.:

private func rawStringFormat() -> RawStringFormat? { switch self { case .byte: return .byte case .binary: return .binary case .date: return .date case .dateTime: return .dateTime case .email: return .email case .hostname: return .hostname case .ipv4: return .ipv4 case .ipv6: return .ipv6 case .password: return .password case .uri: return .uri case .other: return nil } } public var rawValue: String { if let rawValue = self.rawStringValue() { return rawValue } else if case .other(let rawValue) = self { return rawValue } else { assertionFailure() return "" } }

I had something similar at first, but ended up going with the approach found in this revision to avoid the fatalError() branch (which, although it should never be hit, needs to exist). I agree that it does clean up the rawValue implementation and makes the mapping easier to read, but I'm not sure that it's worth introducing the failure case for.

Would you prefer that I take the approach that you recommended or do you think avoiding the failure case is preferable? Do you have an alternate suggestion that doesn't involve a branch that results in a fatalError()?

I mean, you can see that the assertion failure can never be hit here. The raw value will only be nil if self == .other, in which case the else if case will be hit. The assertion failure is to prevent someone from adding another case and not handling it in the future, and assertions don't trigger in prod builds.

AttilaTheFun · 2017-06-13T21:41:13Z

Sources/StringFormat.swift

+            return
+        }
+
+        self = rawStringFormat.stringFormat


This function can be reduced to:

public init(rawValue: String) { self = RawStringFormat(rawValue: rawValue)?.stringFormat ?? .other(rawValue) }

AttilaTheFun · 2017-06-13T21:48:08Z

Sources/FixedParameterFields.swift


    init(map: Map) throws {
        name = try map.value("name")
        location = try map.value("in")
        description = try? map.value("description")
        required = (try? map.value("required")) ?? false
+        example = try? map.value("x-example")


Why is the field name x-example here but example in Metadata?

example is not a documented key for parameter. "body" parameters can have a schema which could have an example, but non-"body" parameters do not have an example key. Here's a link to the spec where it discusses parameters.

In order to introduce this feature on all parameter types, I added support for x-example to allow specs utilizing the feature to remain valid Swagger OpenAPI 2.0 specs.

Ok, sure. Thanks

AttilaTheFun · 2017-06-13T22:03:21Z

Also please rebase this off of master now that your other changes are merged. To prevent us from having repeated merge conflicts in our tests files, can we break up the tests into multiple files based on the subject they're testing?
E.g. AllOfTests.swift, ExampleTests.swift, BasicTests.swift ...

# Conflicts: # Tests/SwaggerParserTests/SwaggerParserTests.swift

AttilaTheFun

Thanks so much for doing this! It looks great :)

AttilaTheFun · 2017-06-14T19:59:51Z

Sources/FixedParameterFields.swift


    init(map: Map) throws {
        name = try map.value("name")
        location = try map.value("in")
        description = try? map.value("description")
        required = (try? map.value("required")) ?? false
+        example = try? map.value("x-example")


Ok, sure. Thanks

Add example support.

1809942

- Adds `example` support to schema properties. - Adds support for `x-example` on parameter definitions. - Adds an `other(String)` type to `StringFormat`.

n8chur requested a review from AttilaTheFun May 31, 2017 21:34

n8chur added 3 commits May 31, 2017 14:53

Add spaces around content inside one line closure.

b7d9e0c

Use fixture helper method to get jsonString.

374a711

Merge branch 'master' into example

59359d6

# Conflicts: # Sources/Metadata.swift # Tests/SwaggerParserTests/SwaggerParserTests.swift

AttilaTheFun requested changes Jun 3, 2017

View reviewed changes

Updates based on PR feedback.

37b4257

AttilaTheFun reviewed Jun 13, 2017

View reviewed changes

Sources/StringFormat.swift

}

}

private enum RawStringFormat: String {

Copy link

Owner

AttilaTheFun Jun 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good :)

AttilaTheFun requested changes Jun 13, 2017

View reviewed changes

n8chur added 2 commits June 14, 2017 12:17

More updates based on PR feedback.

ebdd167

Merge remote-tracking branch 'origin/master' into example

276d208

# Conflicts: # Tests/SwaggerParserTests/SwaggerParserTests.swift

AttilaTheFun approved these changes Jun 14, 2017

View reviewed changes

n8chur merged commit 8713b2d into master Jun 14, 2017

n8chur deleted the example branch June 14, 2017 20:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add example support. #10

Add example support. #10

n8chur commented May 31, 2017

AttilaTheFun left a comment

AttilaTheFun Jun 2, 2017

AttilaTheFun Jun 2, 2017

AttilaTheFun Jun 3, 2017

AttilaTheFun commented Jun 12, 2017

AttilaTheFun Jun 13, 2017

AttilaTheFun left a comment

AttilaTheFun Jun 13, 2017

AttilaTheFun Jun 13, 2017

n8chur Jun 14, 2017 •

edited

Loading

AttilaTheFun Jun 14, 2017

AttilaTheFun Jun 13, 2017

AttilaTheFun Jun 13, 2017

n8chur Jun 14, 2017

AttilaTheFun Jun 14, 2017

AttilaTheFun commented Jun 13, 2017

AttilaTheFun left a comment

AttilaTheFun Jun 14, 2017

Add example support. #10

Add example support. #10

Conversation

n8chur commented May 31, 2017

AttilaTheFun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AttilaTheFun commented Jun 12, 2017

Choose a reason for hiding this comment

AttilaTheFun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8chur Jun 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AttilaTheFun commented Jun 13, 2017

AttilaTheFun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n8chur Jun 14, 2017 •

edited

Loading