Skip to content
This repository was archived by the owner on Feb 16, 2022. It is now read-only.

Clearing the Attributable static "cache" when creating attributes #132

Conversation

simonworkhouse
Copy link

@simonworkhouse simonworkhouse commented Oct 14, 2020

This addresses the issue raised here #127, restructures the tests slightly and adds some stress tests, attribute value tests and tests to make sure that the attributable cache is cleared correctly.

Essentially, since the entity attributes are stored in the static property static::$entityAttributes and since the getEntityAttributes() function doesn't re-fetch the attributes if they have already been set for the current entity, any additional attributes that are created in the current request weren't available to the entity.

I have also included a fix to the Attribute model setEntitiesAttribute method so that it doesn't keep adding an event handler for the saved event every time that the method is called.

Simon Geoghegan added 9 commits October 14, 2020 18:05
…eared when attributes are created/updated/deleted
- Added test migrations
- Added a test Thing model and factory
- Moved the test User model and factory
- Created a test to make sure that attribute values are saved on models
- Added a test to ensure that the attributeable cache is being cleared
…andler for every time the 'saved' event is triggered
@simonworkhouse
Copy link
Author

Feedback on this would be appreciated.

@simonworkhouse
Copy link
Author

Fixes #127

@Omranic
Copy link
Member

Omranic commented Sep 28, 2021

Thank you for the valuable contribution. I did a complete rewrite for the whole package and changed how this feature works. In the new refactor, it actually uses normal relationships, and should be easy and straightforward like default Laravel relationships. Although, that refactor is incomplete.

I'm still not happy with the overall performance (I believe we can reduce number of executed queries), if you want to check it out, see https://github.com/rinvex/laravel-attributes/tree/refactor-to-native-laravel-relationships

Currently no plans to merge that rewrite, but hopefully sometime I can get it to a stable state, improve performance and release it. Any help with that branch would be much appreciated! 🙂

@Omranic Omranic closed this Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants