BouncyTrait@updateIndex uses BouncyTrait@documentFields for construct…

[tpalts]

…ing the indexed fields

previously, the documentFields() function was completely ignored when updating index.

[fadion]

Actually, I believe that the best approach would be to find the documentFields() that are dirty, so only those values are updated. Your approach would work, but would incur a performance penalty by updating all the documentFields().

I think the best way would be:

if ($fields) {
    $body = $fields;
}
// Find the dirty documents fields
elseif ($dirty = array_intersect_key($this->getDirty(), $this->documentFields())) {
    $body = $dirty;
}
else {
    return true
}

What do you think? If you want to update the PR, you're welcome to do so.

Thanks for your input.

[tpalts]

That does look more efficient than my solution. A problem still not addressed: if you use documentFields() to set fields that are not available on the Eloquent model (I often do), they will always be returned by array_intersect_key(..), even if their value in ES will not change. If these custom fields make up a big portion of all of the indexed fields, there is a performance loss. I don't have a good idea what to do about this, though.

Edit:
I guess a solution would be to only update/index the document, if there are any fields returned from documentFields(). That way the user could ensure inside their own documentFields() that only changed fields will be returned.

BouncyTrait@documentFields would perhaps look like this:

public function documentFields()
    {
        return (empty($this->documentFields)) ? $this->getDirty() : $this->documentFields;
    }

BouncyTrait@updateIndex

if ($fields) {
    $body = $fields;
}
// Find the dirty documents fields
elseif ($dirty = $this->documentFields()) {
    $body = $dirty;
}
else {
    return true
}
...

[fadion]

Didn't think about such a use case. Actually, the intersection will discard the custom keys that are not present on the model, as getDirty() has no information about them.

Let's give it a few days to investigate any good approach. Otherwise, I believe we'll go for your original method of updating all the documentFields().

[tpalts]

I believe my above-mentioned solution makes more sense. I would guess the average dev will use Bouncy to index the whole document. Therefor the default solution I provided in my previous comment for BouncyTrait@documentFields would be sufficient for most ppl. If more customization is sought, the dev customizing should also pay attention to the fact that the customised fields are only returned if they need to be (re)indexed. If you decide in favour of this solution, the documentation should probably also be updated.

Thoughts?