Prevent argument modification and duplicate includes processing #26
Conversation
Since arrays are passed by reference, and we modify these arrays, this actually causes the calling method's `includes` or `excludes` variable to be modified, potentially causing the next `getMemento()` call to use a wrong includes value.
jclausen
left a comment
jclausen
left a comment
There was a problem hiding this comment.
We should be able to elminate some extra processing in de-duplication and I feel that we can perform the duplication in a way that avoids mutating the original args.
Per Jon C, this makes more sense if we want/need to reference the original argument.
|
Is it ready for review now? |
I would say so, go ahead. I just fixed it for ACF 16. 😁 |
|
I'm good with this as it stands |
|
Also added in a commit to filter out an empty string include. This resolves an issue where a |
|
Updated to use native Java array streams for performance, per @lmajano 's PR review. |
|
seems things are dying. |
Ok, looks like the |
…into patch/prevent-include-array-modification
…into patch/prevent-include-array-modification
…into patch/prevent-include-array-modification
…:michaelborn/mementifier into patch/prevent-include-array-modification
3 participants
This PR makes two improvements to Mementifier:
includesandexcludesarguments. The calling code doesn't expect its arrays to be modified and (in our case) can cause sequentialgetMemento()calls on other entities to be processed with a bunch of invalid / missingincludesthat were added via the defaultIncludes on the first entity.includes = [ "item_in_defaultIncludes" ], theitem_in_defaultIncludesproperty will be processed and included twice. (Currently.)