Skip to content

Allow disable apollo client cache#4199

Merged
nobodyiam merged 3 commits intoapolloconfig:masterfrom
hezhangjian:allow-disable-apollo-client-cache
Jan 14, 2022
Merged

Allow disable apollo client cache#4199
nobodyiam merged 3 commits intoapolloconfig:masterfrom
hezhangjian:allow-disable-apollo-client-cache

Conversation

@hezhangjian
Copy link
Copy Markdown
Member

@hezhangjian hezhangjian commented Jan 8, 2022 •
edited
Loading

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
Copy Markdown

codecov-commenter commented Jan 8, 2022 •
edited
Loading

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.

zouyx
zouyx reviewed Jan 10, 2022
View reviewed changes
Comment thread apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java
Comment thread ...t/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryDisableFileCacheTest.java Outdated
nobodyiam
nobodyiam reviewed Jan 11, 2022
View reviewed changes
Comment thread apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java Outdated
Comment thread apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java Outdated
Comment thread apollo-core/src/main/java/com/ctrip/framework/apollo/core/ApolloClientSystemConsts.java Outdated
@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
nobodyiam
nobodyiam reviewed Jan 11, 2022
View reviewed changes
Comment thread ...t/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryDisableFileCacheTest.java Outdated
Comment thread apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java Outdated
Comment thread apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java Outdated
Comment thread apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java
@hezhangjian
Copy link
Copy Markdown
Member Author

hezhangjian commented Jan 11, 2022

@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
Copy Markdown
Member Author

hezhangjian commented Jan 11, 2022

@nobodyiam PTAL again, thanks

Anilople
Anilople reviewed Jan 11, 2022
View reviewed changes
Comment thread apollo-core/src/main/java/com/ctrip/framework/apollo/core/ApolloClientSystemConsts.java Outdated
nobodyiam
nobodyiam previously approved these changes Jan 12, 2022
View reviewed changes
Copy link
Copy Markdown
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
nobodyiam
nobodyiam approved these changes Jan 14, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Reviewers

@zouyx zouyx zouyx left review comments

@Anilople Anilople Anilople left review comments

@nobodyiam nobodyiam nobodyiam approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.0.0

Development

Successfully merging this pull request may close these issues.

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

5 participants

@hezhangjian @codecov-commenter @nobodyiam @zouyx @Anilople