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

Allow disable apollo client cache #4199

Merged

Conversation

hezhangjian
Copy link
Member

@hezhangjian hezhangjian commented Jan 8, 2022

Which issue(s) this PR fixes:

Fixes #4181

Brief changelog

  • add config apollo.property.file.cache.enable
  • if apollo.property.file.cache.enable is false, use RemoteConfigRepository
  • add corresponding unit tests

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2022

Codecov Report

Merging #4199 (50bc5b8) into master (7fb6b8b) will increase coverage by 0.03%.
The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #4199      +/-   ##
============================================
+ Coverage     52.59%   52.62%   +0.03%     
- Complexity     2620     2627       +7     
============================================
  Files           484      484              
  Lines         15192    15200       +8     
  Branches       1571     1571              
============================================
+ Hits           7990     7999       +9     
- Misses         6645     6646       +1     
+ Partials        557      555       -2     
Impacted Files Coverage Δ
...ramework/apollo/core/ApolloClientSystemConsts.java 0.00% <ø> (ø)
...va/com/ctrip/framework/apollo/util/ConfigUtil.java 82.16% <84.61%> (+0.49%) ⬆️
...rip/framework/apollo/spi/DefaultConfigFactory.java 90.69% <100.00%> (+0.69%) ⬆️
...nfigservice/service/AccessKeyServiceWithCache.java 82.65% <0.00%> (-2.05%) ⬇️
.../framework/apollo/spring/property/SpringValue.java 89.47% <0.00%> (+1.75%) ⬆️
...rk/apollo/spring/property/SpringValueRegistry.java 89.18% <0.00%> (+5.40%) ⬆️

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 7fb6b8b...50bc5b8. Read the comment docs.

@hezhangjian hezhangjian force-pushed the allow-disable-apollo-client-cache branch 2 times, most recently from e704fb5 to 8df1904 Compare January 11, 2022 02:44
@hezhangjian
Copy link
Member Author

@nobodyiam After this change, the default value is seted in

  private void initPropertyNamesCacheEnabled() {
    propertyNamesCacheEnabled = getPropertyBoolean(ApolloClientSystemConsts.APOLLO_PROPERTY_NAMES_CACHE_ENABLE,
            ApolloClientSystemConsts.APOLLO_PROPERTY_NAMES_CACHE_ENABLE_ENVIRONMENT_VARIABLES,
            "false");
  }

If we keep the original, it might be confused for there are more than one code setting the init value.

@hezhangjian
Copy link
Member Author

@nobodyiam PTAL again, thanks

nobodyiam
nobodyiam previously approved these changes Jan 12, 2022
Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

lgtm

@hezhangjian hezhangjian force-pushed the allow-disable-apollo-client-cache branch 4 times, most recently from cd483a2 to 24d10ac Compare January 14, 2022 01:07
@hezhangjian hezhangjian force-pushed the allow-disable-apollo-client-cache branch from 24d10ac to a23bca0 Compare January 14, 2022 01:11
Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit 96ee53a into apolloconfig:master Jan 14, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2022
@nobodyiam nobodyiam added this to the 2.0.0 milestone Jan 23, 2022
@hezhangjian hezhangjian deleted the allow-disable-apollo-client-cache branch February 7, 2023 15:53
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.

[security] Allow user disable apollo-client‘s local file cache
5 participants