Skip to content

Conversation

@simonLeary42
Copy link
Collaborator

@simonLeary42 simonLeary42 commented Nov 16, 2025

These functions can't agree about how they should behave if there are pending modifications:

  • attributeValueExists
    • ignores any pending modifications and use the value from $this->object
  • getAttribute
    • ignores any pending modifications and use the value from $this->object
  • getAttributes
    • ignores any pending modifications and use the value from $this->object
  • setAttribute
    • updates the attribute in $this->mods
  • setAttributes
    • discards any pending modifications and overwrites $this->mods entirely
    • setAttributes does not overwrite the entire array of attributes in $this->object, so it shouldn't do that to $this->mods
    • example where this is wrong:
      • $this->object has attributes a, b, and c
      • $this->mods has attributes c and d
      • write(); setAttributes([c => ...]); write() results in $this->object holding the d attribute
      • setAttributes([c => ...]); write() results in the d attribute being lost
  • appendAttribute
    • merges $this->object, $this->mods, and the new value
    • in the case where the specified attribute is absent from $this->mods, $this->object is necessary
    • in the case where the specified attribute is present in $this->mods and is the result of another appendAttribute, the merge is technically correct but $this->object is unnecessary and confusing
    • in the case where the specified attribute is present in $this->mods and is the result of setAttribute or removeAttributeEntryByValue, this is wrong
      • example: attribute value is [$foobar], then setAttribute([]), then appendAttribute($barfoo)
        • new value should be [$barfoo]
        • actual value is [$foobar, $barfoo]
  • removeAttributeEntryByValue
    • ignores $this->mods and overwrites it using the value from $this->object

With this patch, all the above functions should take pending modifications into consideration and behave the same way as though write() were called first.

Co-authored-by: bryank-cs <bryank@uri.edu>
@simonLeary42 simonLeary42 marked this pull request as ready for review November 17, 2025 22:23
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

Successfully merging this pull request may close these issues.

3 participants