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

Full array and string access #8

Closed

Conversation

fcastilloes
Copy link

Hi there.

For my use case, I need to be able to modify a Document instance after a json string has been parsed and later get a json string output with the changes. This PR focuses on the json string output.

The PR, in case you accept it, would modify the asArray method in AccessTrait so it returns a only array structure, instead of having some objects in the response. I also modified the tests to adapt to this change.

Now, with asArray returning no objects, I added a __toString method so I can get the json string representation of the Object.

Finally, I found that the Relationship object was using an array when receiving a collection, so I changed it to have a Collection instance instead. I kept the validation to make sure every item in the collection is a Identifier. Once again, I reviewed the tests to keep them green.

I intend to prepare another PR to allow Document modifications, so please tell me if you are interested or if I better just keep it in my fork.

Thanks.

@Art4
Copy link
Owner

Art4 commented Aug 26, 2015

Hey, thank you for this PR. I'm totally interested in making this library more powerful.

The PR, in case you accept it, would modify the asArray method in AccessTrait so it returns a only array structure

I have this idea in mind too, but want to do it optional, not by default. So eg. $document->asArray(true) would return a complete array structure.

One goal for release 0.4 is a possibility to inject own classes. You can see a rough roadmap in the milestones where I've planned a factory for overriding existing classes.

<?php
namespace Art4\JsonApiClient\Utils;
class Factory
{
    /**
     * @var array
     */
    protected $specs = [
        'Document'            => 'Art4\JsonApiClient\Document',
        'Resource\Collection' => 'Art4\JsonApiClient\Resource\Collection',
        'Resource\Item'       => 'Art4\JsonApiClient\Resource\Item',
        // and so on...
    ];

    /**
     * @param array $overload Specs to be overloaded with custom classes.
     */
    public function __construct(array $overload = [])
    {
        if ($overload) {
            $this->specs = array_replace($this->specs, $overload);
        }
    }

    /**
     * Create a new instance of a class.
     * @param  string $spec
     * @param  array  $args
     * @return object
     */
    public function make($spec, array $args = [])
    {
        $class = new \ReflectionClass($this->specs[$spec])
        return $class->newInstanceArgs($args);
    }
}

So you would create Fcastillo\Document with a __toString method:

<?php
namespace Fcastillo;
class Document extends \Art4\JsonApiClient\Document
{
    public function __toString()
    {
        return json_encode($this->asArray(true));
    }
}

and put it all together like this:

$factory = new Art4\JsonApiClient\Utils\Factory([
    'Document' => 'Fcastillo\Document',
]);

$manager = new Art4\JsonApiClient\Manager($factory);

$document = $manager->parse($jsonapi_string); // $document will be Fcastillo\Document

$string = $document->__toString();

What do you think? I would be glad if I can help you with this library.

Finally, I found that the Relationship object was using an array when receiving a collection, so I changed it to have a Collection instance instead.

I've overlooked that, thank you. 👍 It would be great if you could open another PR with only the changes from 0e7f449, so I can merge this in your favor.

@fcastilloes
Copy link
Author

Hi, glad to see the library is going forward.

The class injection sounds very promising and I think it could cover most of my needs, including document modifications. Create some issues if you think you could use some help and probably I could make some pull requests.

And ok, I'll send a new pull request just with 0e7f449. And I could also send another one with asArray returning array only as an option in case you want to merge that too.

@Art4
Copy link
Owner

Art4 commented Aug 27, 2015

Create some issues if you think you could use some help and probably I could make some pull requests.

I will do that. Thank you. :-)

And I could also send another one with asArray returning array only as an option

That would be great. 👍

@Art4 Art4 closed this Aug 27, 2015
@fcastilloes fcastilloes deleted the full-array-and-string-access branch August 27, 2015 14:44
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

2 participants