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

Algolia addObjects called with empty model data calls api #304

Closed
mnightingale opened this issue Sep 6, 2018 · 4 comments
Closed

Algolia addObjects called with empty model data calls api #304

mnightingale opened this issue Sep 6, 2018 · 4 comments

Comments

@mnightingale
Copy link
Contributor

If line 55 is executed we end up calling $index->addObjects([]) which makes a request to Algolia.

$index->addObjects($models->map(function ($model) {
$array = array_merge(
$model->toSearchableArray(), $model->scoutMetadata()
);
if (empty($array)) {
return;
}
return array_merge(['objectID' => $model->getScoutKey()], $array);
})->filter()->values()->all());

I'm not sure whether this behaviour should be improved in Scout or algolia/algoliasearch-client-php?

@julienbourdeau
Copy link
Contributor

Hmm that's a good question, I would have said that it should be fixed in Scout but now that I think about it, maybe it makes sense to fix in the client so more people avoid making empty calls 🤔

@driesvints
Copy link
Member

Is it though? Doesn't the filter method filter out empty models?

@driesvints
Copy link
Member

Going to close this. Please provide a code example on how to reproduce this and I'll take another look.

@mnightingale
Copy link
Contributor Author

TL;DR: If a models toSearchableArray() returns an empty array the current behaviour in the AlgoliaEngine basically ends up calling the equivalent to collect([null])->filter()->values()->all()which returns [] therefore we call $index->addObjects([]) and a wasteful API request is made to Algolia, since the code filters out empty data I think the intended behaviour was not to call an indexing operation.

PR #318


I have a use case where 4,000 models get updated from an external feed several times per hour.
A model may have no changes but I still want to call save() so that the updated_at field gets touched.
Another situation is that not all of the fields that are updated are present in my index, therefore based on several comments in #153 I return [] in toSearchableArray.

I can't call unsearchable() or return false in shouldBeSearchable() because they'll both remove the model from the index which isn't what I want, I just don't want to reindex the model.

public function toSearchableArray()
{
    if (!$this->isDirty('name')) {
        return []; // I don't want to reindex this model but I don't want to remove it
    }

    return [
        'name' => $this->name
    ];
}

The issue is that the update method of the engine will call $index->addObjects([]) and still issue an empty update to Algolia wasting an API request without actually doing anything.

public function update($models)
{
if ($models->isEmpty()) {
return;
}
$index = $this->algolia->initIndex($models->first()->searchableAs());
if ($this->usesSoftDelete($models->first()) && config('scout.soft_delete', false)) {
$models->each->pushSoftDeleteMetadata();
}
$index->addObjects($models->map(function ($model) {
$array = array_merge(
$model->toSearchableArray(), $model->scoutMetadata()
);
if (empty($array)) {
return;
}
return array_merge(['objectID' => $model->getScoutKey()], $array);
})->filter()->values()->all());
}

I think either it should check if the $models->map(...) call returns an empty array or the Algolia client should check if it receives an empty addObjects request.

Based on these lines, I think it's supposed to handle empty data itself?

if (empty($array)) {
return;
}

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

No branches or pull requests

3 participants