basho / riak-php-client

PHP clients for Riak
Apache License 2.0
160 stars 69 forks source link

Basho\Riak\Search\Doc does not allow to fetch all values returned by Solr [JIRA: CLIENTS-754] #117

Closed lastzero closed 8 years ago

lastzero commented 8 years ago

As a developer, I want to access all field names and values returned by Solr so that I don't have to perform individual requests to Riak for each result item.

Right now, there is no way to fetch all values returned by Solr from Basho\Riak\Search\Doc - there is just a magic getter for individual values (you need to know the field name to use it). While it is possible to work with this method if all value names are known upfront (e.g. from the schema), this seems to be slow and pretty inconvenient in some use cases - especially if the result set contains mixed results. Another option would be to only get the location from Basho\Riak\Search\Doc and perform individual Riak GET requests for each result item (which is very slow and even more inconvenient for large result sets).

I suggest adding a method getData(), which returns an array or an object (which can be converted to an array using (array)):

class Doc
{
    protected $data = null;

    public function __construct(\stdClass $data)
    {
        ...
        $this->data = $data;
    }

    ...

    public function __get($name)
    {
        return $this->data->{$name};
    }

    public function getData() // NEW
    {
        return (array)$this->data;
    }
}
christophermancini commented 8 years ago

I think this request is reasonable. If you submit a PR, I will approve it. Otherwise, I probably can't get to it till later this week.

lastzero commented 8 years ago

Sure, I can do that. Would you mind adding a method getDataAsArray()? The library is working with stdClass objects by default, but often developers need arrays to further process data. Type casting everywhere is kind of ugly, but I understand if that's too much change for now. After reading more code, I found no consistent return type for existing getData() methods (I found object, array and string). So I think it's best to return an array, because that will be most useful for PHP developers.

debo commented 8 years ago

@lastzero @christophermancini based on the comment

The library is working with stdClass objects by default, but often developers need arrays to further process data

My take, in the long run would then be to replace stdClass with an iterator interface because that's what you are actually need to have the requested behaviour; that would be the right way of doing this.

lastzero commented 8 years ago

@debo You can only choose between Array and stdClass as return type for json_decode($json, $assoc = false) - that's where $data comes from. Making more use of standard interfaces sounds like a great idea though for future releases.

debo commented 8 years ago

@lastzero oh I see, I should have looked at that, make sense.

christophermancini commented 8 years ago

Yea, I tried to keep things fairly raw with the library by not making many assumptions with how you want the data. This leaves the ability for the developer to wrap it how they see fit while maintaining performance with minimal layers of data mutation.

lastzero commented 8 years ago

Thanks for accepting the pull request! We're currently working with the develop branch, but it'd be great to see this in a stable release.

christophermancini commented 8 years ago

@lastzero Completely understand, I will cut a new release by end of the week.