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

Use Doctrine MetadataCache if a cache is not set #2407

Merged

Conversation

franmomu
Copy link
Collaborator

@franmomu franmomu commented Jan 8, 2022

I'm a bit concern about performance after #2373 because before that, the cache was shared among the listeners and now it should be set externally.

I've created stof/StofDoctrineExtensionsBundle#436 to try to solve it for the bundle.

In the meantime, instead of creating a new ArrayAdapter as fallback, I think we can go back to doctrine metadata cache so everything should work as before.

I'll try later today to add a test and after this I'm fine releasing 3.5.0.

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2022

Codecov Report

Merging #2407 (f105358) into main (0fab9a7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2407      +/-   ##
============================================
+ Coverage     80.47%   80.49%   +0.01%     
- Complexity     3152     3154       +2     
============================================
  Files           159      159              
  Lines          8160     8167       +7     
============================================
+ Hits           6567     6574       +7     
  Misses         1593     1593              
Impacted Files Coverage Δ
src/Mapping/MappedEventSubscriber.php 90.47% <100.00%> (+0.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fab9a7...f105358. Read the comment docs.

@franmomu franmomu force-pushed the use_doctrine_cache_fallback branch from 251a8af to c0695aa Compare January 8, 2022 15:31
@franmomu franmomu requested a review from phansys January 8, 2022 15:34
@franmomu franmomu force-pushed the use_doctrine_cache_fallback branch 3 times, most recently from 05bf050 to ea05421 Compare January 8, 2022 15:42
@franmomu franmomu force-pushed the use_doctrine_cache_fallback branch 3 times, most recently from 0da83b4 to 8305832 Compare January 8, 2022 16:55
@franmomu franmomu force-pushed the use_doctrine_cache_fallback branch from 8305832 to f105358 Compare January 8, 2022 16:56

$this->expectDeprecationWithIdentifier('https://github.com/doctrine/persistence/issues/184');
} else {
$this->expectDeprecation('Doctrine\Persistence\Mapping\AbstractClassMetadataFactory::getCacheDriver is deprecated. Use getCache() instead.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use getCache instead of the deprecated API when it is available ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@franmomu franmomu merged commit fe98b15 into doctrine-extensions:main Jan 10, 2022
@franmomu franmomu deleted the use_doctrine_cache_fallback branch January 10, 2022 21:26
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.

4 participants