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.

Sign up for GitHub

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

Jump to bottom

Implement attributesToRetrieve for getObjects method #273

Merged
merged 2 commits into from Feb 17, 2017
Merged

Implement attributesToRetrieve for getObjects method #273

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d9515c
Implement attributesToRetrieve for getObjects method
JanPetr Feb 17, 2017
a4dd529
Accept array as $attributesToRetrieve with keeping BC
JanPetr Feb 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/AlgoliaSearch/Index.php
View file
Open in desktop
Expand Up @@ -210,12 +210,14 @@ public function getObject($objectID, $attributesToRetrieve = null)
* Get several objects from this index.
*
* @param array $objectIDs the array of unique identifier of objects to retrieve
* @param $attributesToRetrieve (optional) if set, contains the list of attributes to retrieve as a string
* separated by ","
*
* @return mixed
*
* @throws \Exception
*/
public function getObjects($objectIDs)
public function getObjects($objectIDs, $attributesToRetrieve = null)
Copy link
Member

Choose a reason for hiding this comment

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

I think we should ask for an array of strings like we do for the other methods.
The documentation is bad for some of the other methods, we say we expect comma separated values where we are actually accepting arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to be consistent with getObject method where we take comma-separated string.
Definitely not something I'd approve and I'd definitely rather accept array, but for me consistency has higher priority right now.

Copy link
Member

Choose a reason for hiding this comment

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

Arg, I just tested and it doesn't work with an array in the 'getObject' method.
I would:

  • Make sure we support arrays in both getObject and getObjects
  • Change docbloc and documentation to array
  • Keep backward compat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, can you review @rayrutjes?

{
if ($objectIDs == null) {
throw new \Exception('No list of objectID provided');
Expand All @@ -224,6 +226,11 @@ public function getObjects($objectIDs)
$requests = array();
foreach ($objectIDs as $object) {
$req = array('indexName' => $this->indexName, 'objectID' => $object);

if ($attributesToRetrieve) {
Copy link
Member

Choose a reason for hiding this comment

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

This will fail if empty array is provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct behavior. Empty array = all attributes are fetched

Copy link
Member

Choose a reason for hiding this comment

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

It is not the current behaviour. Empty string returns only objectIDs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the API behavior for getObject endpoint. For getObjects API returns all attributes on empty string/array.

$req['attributesToRetrieve'] = $attributesToRetrieve;
}

array_push($requests, $req);
}

Expand Down
28 changes: 28 additions & 0 deletions tests/AlgoliaSearch/Tests/GetTest.php
View file
Open in desktop
Expand Up @@ -56,6 +56,34 @@ public function testGetObjects()
$this->assertEquals($results['hits'][1]['firstname'], $obj2['firstname']);
}

public function testGetObjectsWithAttributesToRetrieve()
{
$res = $this->index->addObjects(array(
array('firstname' => 'Robin', 'lastname' => 'Lee'),
array('firstname' => 'Robert', 'lastname' => 'Watson')
));

$this->index->waitTask($res['taskID']);
$results = $this->index->search('rob');
$this->assertEquals(2, $results['nbHits']);

$objects1 = $this->index->getObjects(array($results['hits'][0]['objectID'], $results['hits'][1]['objectID']));
$objects2 = $this->index->getObjects(array($results['hits'][0]['objectID'], $results['hits'][1]['objectID']), 'lastname');

$this->assertEquals(2, count($objects1['results']));
$this->assertEquals(2, count($objects2['results']));

$firstResult = reset($objects1['results']);
$this->assertTrue(isset($firstResult['objectID']));
$this->assertTrue(isset($firstResult['firstname']));
$this->assertTrue(isset($firstResult['lastname']));

$secondResult = reset($objects2['results']);
$this->assertTrue(isset($secondResult['objectID']));
$this->assertFalse(isset($secondResult['firstname']));
$this->assertTrue(isset($secondResult['lastname']));
}

public function testGetSaveObjects()
{
$res = $this->index->saveObjects(array(
Expand Down