Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Data as Serialized object even if is not valid json #150

Closed
wants to merge 1 commit into from

Conversation

MP0w
Copy link
Member

@MP0w MP0w commented Feb 1, 2017

This enable Kakapo to stub other kind of requests, for example we can create a XMLSerialzier that when serialized returns the object as Data, or we can implementer Serialziable for UIImage, a really powerful feature that let us use Kakapo not only for JSON responses but every kind of response.
This is a basic change that enable us to do so, some more documentation might be written in future and we might decide to move in a direction where Kakapo is less JSON centric

Example:

extension UIImage: ResponseFieldsProvider {

    public var statusCode: Int {
        return 200
    }

    public var body: Serializable {
        return UIImagePNGRepresentation(self)
    }

    public var headerFields: [String : String]? {
        return ["Content-Type": "image/png"]
    }
}

Usage:

router.get("image.png") {
   return UIImage(named: "whatever")
}

@MP0w MP0w requested a review from joanromano February 1, 2017 10:54
@MP0w MP0w changed the title Allow Data as Serialized object even if is not valid son Allow Data as Serialized object even if is not valid json Feb 1, 2017
@@ -45,16 +46,19 @@ public extension Serializable {
}

/**
Serialize a `Serializable` object and convert the serialized object to `Data`. Unless it is nil the return value is representing a JSON. Usually you don't need to use this method directly since `Router` will automatically serialize objects when needed.
Serialize a `Serializable` object and convert the serialized object to `Data`. Unless it is nil or the serialized object is `Data`, the return value is representing a JSON. Usually you don't need to use this method directly since `Router` will automatically serialize objects when needed.

Choose a reason for hiding this comment

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

Line Length Violation: Line should be 100 characters or less: currently 293 characters (line_length)

*/
func customSerialized(transformingKeys keyTransformer: KeyTransformer?) -> Any?
}

public extension Serializable {
/**
Serialize a `Serializable` object. The output, unless nil, is convertible to `Data` / JSON. Usually you don't need to use this method directly since `Router` will automatically serialize objects when needed.
Serialize a `Serializable` object. The output, unless nil, is convertible to `Data` or JSON. Usually you don't need to use this method directly since `Router` will automatically serialize objects when needed.

Choose a reason for hiding this comment

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

Line Length Violation: Line should be 100 characters or less: currently 213 characters (line_length)

This enable Kakapo to stub other kind of requests, for example we can create a XMLSerializier that when serialized returns the object as Data, or we can implementer `Serializable` for `UIImage`, a really powerful feature that let us use Kakapo not only for JSON responses but every kind of response.
This is a basic change that enable us to do so, some more documentation might be written in future and we might decide to move in a direction where Kakapo is less JSON centric
@MP0w MP0w force-pushed the feature/allowDataAsSerializedObject branch from a6a1a6a to 0129bfa Compare February 1, 2017 10:58
@joanromano
Copy link
Member

@MP0w in your example: how is the result of UIImagePNGRepresentation(self) a valid Serializable object for the body var? Isn't that a NSData return type?

@joanromano
Copy link
Member

Also toData() is not a requirement from ResponseFieldsProvider is it?

@MP0w
Copy link
Member Author

MP0w commented Feb 1, 2017

Sorry toData is not needed, forget about it.
NSdata is not serializable by itself but Optional is serializable

@joanromano
Copy link
Member

I see. Then since the serialization also supports Data now, how about making Data/NSData also CustomSerializable by default? This way we don't need to always wrap Data into an Optional, which maybe is not the best for other scenarios.

I think we really should update README to document this properly before merging, otherwise it's a bit misleading how to use it.

@joanromano
Copy link
Member

Also, adding a test which shows this usage together with a Router might be also helpful and self documentary for ourselves in the future.

@joanromano
Copy link
Member

Interesting your point regarding being less JSON centric: if we have to support different serializations, we might have to separate into different serialization types and wrap everything into a serialization process that takes all types into account maybe.

@MP0w
Copy link
Member Author

MP0w commented Feb 1, 2017

The non-JSON centric thing needs a major revision of documentation, README and the way we advertise Kakapo. That's why I think for the moment we can enable it (which is something I would expect to work) and later tackle it

@MP0w
Copy link
Member Author

MP0w commented Feb 1, 2017

"adding a test which shows this usage together with a Router might be also helpful" this is already tested I think, router will accept any Data without caring what it contains, because is the only thing needed to create a response

@joanromano
Copy link
Member

Yup, didn't say it was not tested: just that it would be self documentary for us in the future I believe.

@MP0w
Copy link
Member Author

MP0w commented Feb 25, 2017

@joanromano I wouldn't make Data Serializable because it should be since it doesn't make sense to have it inside a json but only as a replacement for json... what we have to do is to decouple Kakapo from json and requiring a DataConvertible instead of current Serializable... json wouldn't be anymore a special case, just something that can be converted to data.

@MP0w
Copy link
Member Author

MP0w commented Feb 25, 2017

I will work on something and see how it goes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants