Skip to content

Allow serialization to array, not just encoded string #169

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

Closed
lindyhopchris opened this issue Feb 13, 2017 · 5 comments
Closed

Allow serialization to array, not just encoded string #169

lindyhopchris opened this issue Feb 13, 2017 · 5 comments
Assignees

Comments

@lindyhopchris
Copy link
Collaborator

There are occasions when it is necessary to serialize a JSON API payload to an array, rather than the encoded string that Encoder currently returns.

For example, in Laravel when broadcasting data via a websocket, Laravel expects the data as an array rather than a string. This is typically because third-party PHP libraries will also expect the data as an array.
https://laravel.com/docs/5.4/broadcasting#broadcast-data

My suggestion would be for there to be an interface that has a similar pattern as the EncoderInterface, but returns arrays. E.g. if it was called SerializerInterface then:

EncoderInterface SerializerInterface
withLinks withLinks
withMeta withMeta
withJsonApiVersion withJsonApiVersion
withRelationshipSelfLink withRelationshipRelatedLink
encodeData serializeData
encodeIdentifiers serializeIdentifiers
encodeError serializeError
encodeErrors serializeErrors
encodeMeta serializeMeta

Then Encoder would take a Serializer instance in its constructor, use that to get the array that it then encodes to a string. This would allow you to create just a serializer if you didn't want to encode the JSON API document to a string. I.e. Encoder::instance() and Serializer::instance() would both exist.

I'm happy to submit a PR for this feature, but thought I'd run it past you first.

@neomerx neomerx self-assigned this Feb 13, 2017
@neomerx
Copy link
Owner

neomerx commented Feb 13, 2017

I actually foresaw that some would like to have raw array without converting it to json string 😄
Probably the easiest way is to change behavior of Encoder itself.

class Encoder
{
    // this is the final step in all encode methods
    protected function encodeToJson(array $document)
    {
        return $this->encoderOptions === null ?
            json_encode($document) :
            json_encode($document, $this->encoderOptions->getOptions(), $this->encoderOptions->getDepth());
    }
}

Option 1

class YourEncoder extends Encoder
{
    // this is the final step in all encode methods
    protected function encodeToJson(array $document)
    {
        return $document;
    }
}

YourEncoder::instance(...)->...;

Pros: fast and easy, no changes in lib required. Cons: encodeToJson actually do not encode to json (probably would be better to rename it to something neutral), return type is changed from string to array.

Option 2
Wrap final step into interface or Closure.
Pros: more obvious way that Encoder may return not only strings. Cons: really not better than option 1 and requires changes in code, doesn't look nice imho.

Option 3 (this one looks nice to me)
Refactor Encoder code in a way you can extend Encoder with new methods serializeXXX so you can have this functionality with 1-2 lines for each method. We only need to make the last step optional/configurable.
Pros: clean way to have strongly typed results, no extra instances/memory allocations. Cons: would require some coding on your side (could be mitigated by built-in trait for serializeXXX methods).

Your thoughts?

@lindyhopchris
Copy link
Collaborator Author

Cool, glad you're open to adding it.

The EncoderInterface::encodeXXX methods are (correctly) type-hinted to return strings. Having the methods return either strings or arrays is messy (imho) because you don't know for sure what you're getting if you're handling the interface (as opposed to a specific instance).

Plus encoder takes encoding options, which are not required for producing arrays, so imho it makes sense for the serializer to be a separate thing rather than something that's mixed in with encoding. (I tend to always separate concerns - here the serializer's concern is to produce an array, the encoder's concern is to encode a serialized array).

A compromise if you don't want separate units would be to have the serializeXXX and encodeXXX methods to be on the EncoderInterface - i.e. so you know if you are handling the interface whether you're getting an array or string based on which method you call.

I pretty much always handle the interface - i.e. my consuming code would never know what instance it has, so I'm keen for there to be interface methods somewhere. Either EncoderInterface with encodeXXX and serializeXXX, or separate EncoderInterface or SerializerInterface.

@neomerx
Copy link
Owner

neomerx commented Feb 14, 2017

I almost finished the changes. The only issue is with encoding empty errors (no properties at all, just nothing). They cannot be stored as empty arrays because when converted to json they become empty arrays instead of empty objects.
Still thinking what can we do with it.

@neomerx neomerx added the fixed label Feb 14, 2017
@lindyhopchris
Copy link
Collaborator Author

I'd filter them out: it makes no sense to have an empty error object.

@neomerx
Copy link
Owner

neomerx commented Feb 14, 2017

That's the only idea I came up with

Due to changes it will require new version. I'm thinking of v1 finally.

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

No branches or pull requests

2 participants