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

refactor cache decorator #3056

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wuwen5
Copy link
Contributor

@wuwen5 wuwen5 commented Jan 2, 2024

Reopen from #838 and related #2921

@coveralls
Copy link

coveralls commented Jan 2, 2024

Coverage Status

coverage: 87.144% (+0.03%) from 87.116%
when pulling 0379d03 on wuwen5:refactor-cache-decorate
into 4c9d63d on mybatis:master.

@harawata
Copy link
Member

I expressed my concern about allowing direct access to the wrapped instances.
My personal opinion is: if you want to see the cache statistics, you should use some third party cache implementation instead of MyBatis' internal cache.

@wuwen5
Copy link
Contributor Author

wuwen5 commented Feb 18, 2024

  • The access level of the getDelegate() has been modified to protected.
  • Removed the getHits() from LoggingCache

@harawata
Copy link
Member

@wuwen5 ,

I'm sorry for the belated response.
I have taken a look at this several times actually, but I still prefer keeping these cache implementations loosely coupled.

@hazendaz ,
If you like this, feel free to merge.
I won't object.

@hazendaz
Copy link
Member

hazendaz commented Apr 4, 2024

Will circle back on this in some time. I think the general idea was that users wanted it and I believe I suggested it was ok to create new PR bringing others up to date. I do not have any strong preference either way.

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