add multiple appId,pull other appId config into config. #70

TerryLam2010 · 2024-07-10T07:30:25Z

What's the purpose of this PR

add multiple appId pull into config
see #66

Which issue(s) this PR fixes:

Fixes #

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

add multiple appId pull into config.see apollo-client支持多appid拉取配置的功能 #66

Summary by CodeRabbit

New Features
- Introduced appId parameter across various configuration classes, allowing configuration queries and changes to be associated with a specific application ID.
- Added new method getAccessKeySecret(String appId) to manage access keys based on application IDs.
- Introduced a new annotation MultipleConfig for managing multiple application IDs.
Changes in APIs
- Updated method signatures in multiple configuration classes and services to include appId.
- Added new methods for retrieving appId in relevant classes.
Refactor
- Refactored internal data structures and methods to support the inclusion of appId.

These changes enhance the ability to manage and query configurations on a per-application basis, improving flexibility and granularity in configuration management.

github-actions · 2024-07-10T07:30:40Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

coderabbitai · 2024-07-10T07:31:04Z

Walkthrough

The recent updates to the Apollo Client introduce an appId parameter across multiple classes and methods. This enhancement improves configuration management by allowing retrieval and handling of configurations tailored to specific applications, facilitating greater flexibility and organization in multi-application environments.

Changes

Files/Groups	Changes Summary
`PureApolloConfig.java`, `DefaultConfig.java`, `LocalFileConfigRepository.java`	Added `appId` parameter to constructors and method signatures to enhance configuration handling.
`ConfigService.java`, `ConfigManager.java`	Updated methods to include `appId`, enforcing stricter validation and improving repository management.
`AbstractConfig.java`	Modified methods to incorporate `appId`, affecting how configuration changes are processed.
`ConfigChange.java`, `ConfigChangeEvent.java`	Added `appId` field to constructors for managing changes linked to specific applications.
`ApolloConfig.java`, `MultipleConfig.java`	Introduced new annotations to support application-specific configurations and multiple app IDs.
`ConfigUtil.java`	Enhanced utility methods to manage application-specific cache directories and access keys.
`ConfigServiceTest.java`, `ConfigUtilTest.java`	Updated test cases to reflect changes in method signatures and improve clarity in testing configurations.

Sequence Diagrams

Configuration Retrieval Flow

sequenceDiagram
    participant Client as Client
    participant ConfigService as ConfigService
    participant ConfigManager as ConfigManager
    participant ConfigRepository as ConfigRepository

    Client->>ConfigService: getConfig(appId, namespace)
    ConfigService->>ConfigManager: getConfig(appId, namespace)
    ConfigManager->>ConfigRepository: fetchConfig(appId, namespace)
    ConfigRepository-->>ConfigManager: return Config
    ConfigManager-->>ConfigService: return Config
    ConfigService-->>Client: return Config

Configuration Change Notification Flow

sequenceDiagram
    participant ConfigRepository as ConfigRepository
    participant RemoteConfigLongPollService as RemoteConfigLongPollService
    participant Client as Client
    
    ConfigRepository->>RemoteConfigLongPollService: submit(appId, namespace)
    RemoteConfigLongPollService->>Client: notify(appId, namespace, changes)
    Client-->>ConfigRepository: onRepositoryChange(appId, namespace, newProperties)

🐰 In fields of green, I hop with glee,
New app IDs bloom, oh what a spree!
Configs now dance, tailored with care,
Multi-app magic fills the air!
With each little change, our dreams take flight,
A brighter tomorrow, oh what a delight! 🌼

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (4)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java (1)
82-83: Update getConfigFile method calls to include appId parameter.

The following instances of getConfigFile calls are still using the old signature and need to be updated to include the appId parameter:

ApolloClientConfigurationFactory.java

Line: 12

ConfigServiceTest.java

Line: 4

DefaultConfigManagerTest.java

Line: 12

Line: 18

Line: 24

Please update these method calls to match the new signature.

Analysis chain

Verify the function usage for the updated signature.

The getConfigFile method now includes appId as a parameter. Ensure all calls to this method are updated accordingly.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `getConfigFile` match the new signature.

# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type java -A 5 $'getConfigFile'
Length of output: 20390
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (2)
106-110: Issue Found: Update setConfig Method to Include appId Parameter

The setConfig method in ConfigService.java does not include the new appId parameter as suggested in the review comment. Please update the method to include the appId parameter.

File: apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

Lines: 106-110

Please ensure that all calls to this method are updated accordingly.

Analysis chain

Verify the function usage for the updated signature.

The setConfig method now includes appId as a parameter. Ensure all calls to this method are updated accordingly.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `setConfig` match the new signature.

# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type java -A 5 $'setConfig'
Length of output: 10705

Script:
#!/bin/bash
# Description: Verify if the setConfig method in ConfigService.java includes the new appId parameter.

# Test: Search for the setConfig method usage in ConfigService.java. Expect: Occurrences of the new signature with appId.
rg --type java -A 5 'setConfig' apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java
Length of output: 940
80-82: Update calls to ConfigService.getConfig to use the new signature.

The getConfig method now includes appId as a parameter. The following instances still use the old signature and need to be updated:

apollo-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java

apollo-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XMLConfigAnnotationTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigIntegrationTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigServiceTimeoutIntegrationTest.java

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/importer/ApolloConfigDataLoader.java

Please update these instances to use the new method signature that includes appId.

Analysis chain

Verify the function usage for the updated signature.

The getConfig method now includes appId as a parameter. Ensure all calls to this method are updated accordingly.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `getConfig` match the new signature.

# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type java -A 5 $'getConfig'
Length of output: 149777
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (1)
102-104: Update all instances of onRepositoryChange to the new signature.

The following instances of onRepositoryChange still use the old signature without appId. Please update them to include appId:

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java:

Line 1: simpleConfig.onRepositoryChange(TestApolloConfigChangeListenerWithInterestedKeyPrefixesBean1.SPECIAL_NAMESPACE, properties);

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderAutoUpdateTest.java:

Line 1: config.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);

Line 2: config.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);

Line 3: config.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);

Line 4: config.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);

Line 5: applicationConfig.onRepositoryChange(ConfigConsts.NAMESPACE_APPLICATION, newProperties);

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/XmlConfigFileTest.java:

Line 1: configFile.onRepositoryChange(someNamespace, anotherProperties);

Line 2: configFile.onRepositoryChange(someNamespace, someProperties);

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java:

Line 1: defaultConfig.onRepositoryChange(someNamespace, anotherProperties);

Line 2: defaultConfig.onRepositoryChange(someNamespace, someProperties);

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepositoryTest.java:

Line 1: verify(someListener, times(1)).onRepositoryChange(someNamespaceName, anotherProperties);

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java:

Line 1: verify(someListener, times(1)).onRepositoryChange(eq(someNamespace), captor.capture());

Line 2: verify(someListener, times(1)).onRepositoryChange(any(String.class), any(Properties.class));

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesConfigFileTest.java:

Line 1: configFile.onRepositoryChange(someNamespace, anotherProperties);

Line 2: configFile.onRepositoryChange(someNamespace, someProperties);

Please review and update these instances to ensure consistency with the new method signature.

Analysis chain

Verify the function usage for the updated signature.

The onRepositoryChange method now includes appId as a parameter. Ensure all calls to this method are updated accordingly.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `onRepositoryChange` match the new signature.

# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type java -A 5 $'onRepositoryChange'
Length of output: 63294

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6b0d4c7 and 598e03a.

Files selected for processing (57)

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (1 hunks)
apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfigFactory.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfig.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigFile.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigRepository.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (7 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEvent.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/JsonConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PlainTextConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepository.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (12 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigRepository.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RepositoryChangeListener.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/TxtConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/XmlConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YamlConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YmlConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigFileChangeEvent.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactory.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactoryManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigRegistry.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/property/AutoUpdateConfigChangeListener.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/property/SpringValue.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/AbstractConfigTest.java (4 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java (4 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java (30 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEventTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/JsonConfigFileTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepositoryTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesConfigFileTest.java (9 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (9 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/SimpleConfigTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/TxtConfigFileTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/XmlConfigFileTest.java (10 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/YamlConfigFileTest.java (10 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryFileCachePropertyTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java (3 hunks)

Files not processed due to max files limit (14)

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistryTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XMLConfigAnnotationTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderAutoUpdateTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/CachedCompositePropertySourceTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessorTest.java
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
apollo-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java

Files skipped from review due to trivial changes (1)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/property/AutoUpdateConfigChangeListener.java

Additional comments not posted (200)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1)

1-29: Review of the new MultipleConfig annotation

Annotation Purpose:
The MultipleConfig annotation is designed to supplement @EnableApolloConfig for handling multiple appId configurations.

Attributes:

appId: This attribute is correctly defined to hold a single application ID.

namespaces: This attribute is correctly defined to hold multiple namespaces.

secret: This attribute is correctly defined with a default value and an example in the documentation.

The provided code looks good and correctly defines the annotation and its attributes.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/TxtConfigFile.java (1)

23-24: Constructor modification to include appId

The constructor now includes the appId parameter, which is correctly passed to the superclass constructor.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YmlConfigFile.java (1)

25-26: Constructor modification to include appId

The constructor now includes the appId parameter, which is correctly passed to the superclass constructor.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/XmlConfigFile.java (1)

25-27: Constructor modification to include appId

The constructor now includes the appId parameter, which is correctly passed to the superclass constructor.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/JsonConfigFile.java (1)

25-27: Constructor change to include appId looks good.

The addition of appId in the constructor and passing it to the superclass is appropriate.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactoryManager.java (1)

30-37: New method to get the config factory by appId looks good.

The new method getFactory with appId parameter is consistent with the existing method and extends functionality appropriately.

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfigFactory.java (1)

30-31: Overridden method to include appId looks good.

The overridden method createRepositoryConfig now includes appId, ensuring PureApolloConfig is created correctly.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RepositoryChangeListener.java (1)

31-38: New method to handle repository changes by appId looks good.

The new method onRepositoryChange with appId parameter is consistent with the existing method and extends functionality appropriately.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PlainTextConfigFile.java (1)

27-28: Constructor change approved.

The constructor now includes appId, aligning with the PR objectives of adding appId to configurations.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigRegistry.java (2)

33-41: Method register change approved.

The method now includes appId, aligning with the PR objectives of adding appId to configurations.

50-57: Method getFactory change approved.

The method now includes appId, aligning with the PR objectives of adding appId to configurations.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactory.java (2)

35-43: Method create change approved.

The method now includes appId, aligning with the PR objectives of adding appId to configurations.

51-57: Method createConfigFile change approved.

The method now includes appId, aligning with the PR objectives of adding appId to configurations.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEvent.java (1)

44-46: Constructor change approved.

The constructor now includes appId, aligning with the PR objectives of adding appId to configurations.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/TxtConfigFileTest.java (4)

34-34: Add someAppId field.

The someAppId field is added to the test class to facilitate testing the new appId parameter in the TxtConfigFile class.

41-41: Initialize someAppId in setUp method.

The someAppId variable is initialized in the setUp method to be used in the test cases.

54-54: Update TxtConfigFile instantiation to include appId.

The instantiation of TxtConfigFile is updated to include the new appId parameter, ensuring the test reflects the updated constructor.

57-57: Add assertion for appId in TxtConfigFile.

An assertion is added to verify that the appId is correctly set in the TxtConfigFile instance.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (2)

34-34: Add getConfig method with appId.

A new method getConfig is added to the ConfigManager interface to retrieve configuration instances based on appId and namespace.

41-41: Add getConfigFile method with appId.

A new method getConfigFile is added to the ConfigManager interface to retrieve configuration file instances based on appId, namespace, and configFileFormat.

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1)

38-38: Add getAppId method.

A new method getAppId is added to the ConfigFile interface to retrieve the appId of the configuration file instance.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (5)

22-22: Import ConfigUtil.

The ConfigUtil class is imported to facilitate retrieving the appId in the getFactory method.

33-34: Add m_configUtil field.

A new field m_configUtil is added to store the ConfigUtil instance, which is used to retrieve the appId.

37-37: Initialize m_configUtil in constructor.

The m_configUtil field is initialized in the constructor to ensure it is available for use in the getFactory method.

42-43: Update getFactory method to use appId.

The getFactory method is updated to retrieve the appId using m_configUtil and delegate to the overloaded getFactory method with appId and namespace.

45-45: Add overloaded getFactory method with appId.

An overloaded getFactory method is added to handle retrieving configuration factories based on appId and namespace.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigFileChangeEvent.java (3)

26-26: Addition of appId field.

The new field appId has been introduced to store the application ID associated with the config file change event. This change aligns with the PR objective to support multiple appId configurations.

41-43: Constructor update to include appId.

The constructor now includes appId as a parameter to initialize the new field. This change ensures that each config file change event is associated with a specific application ID.

69-70: Update to toString() method to include appId.

The toString() method now includes the appId field in its output. This change ensures that the string representation of the config file change event includes the application ID.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java (3)

39-39: Usage of Tables.synchronizedTable.

The m_instances field is now initialized using Tables.synchronizedTable to ensure thread safety. This change is crucial for concurrent access to the configuration instances.

47-48: Update to register method to include appId.

The register method now includes appId as a parameter, and the existing method delegates to the new method with appId. This change ensures that configurations are registered with a specific application ID.

61-62: Update to getFactory method to include appId.

The getFactory method now includes appId as a parameter, and the existing method delegates to the new method with appId. This change ensures that the correct configuration factory is retrieved based on the application ID.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepository.java (1)

46-46: Update to fireRepositoryChange method call to include appId.

The fireRepositoryChange method call now includes appId as a parameter. This change ensures that repository changes are correctly associated with the specific application ID.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (3)

28-28: Addition of appId field.

The new field appId has been introduced to store the application ID associated with the config change event. This change aligns with the PR objective to support multiple appId configurations.

36-38: Constructor update to include appId.

The constructor now includes appId as a parameter to initialize the new field. This change ensures that each config change event is associated with a specific application ID.

83-85: Addition of getAppId method.

The new getAppId method provides access to the appId field. This change is necessary to retrieve the application ID associated with the config change event.

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (1)

37-39: Constructor updated to include appId.

The constructor now includes the appId parameter, which is passed to the superclass. Ensure that the appId is correctly utilized in the superclass and throughout the configuration management logic.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigRepository.java (1)

66-69: Method fireRepositoryChange updated to include appId.

The fireRepositoryChange method now includes the appId parameter. Ensure that all listeners handle the appId correctly and that this change is consistently propagated throughout the repository change notification logic.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1)

72-76: New parameter multipleConfig added.

The multipleConfig parameter supports additional appId and namespace configurations. Ensure this parameter is correctly utilized in the annotation processing logic and that it does not interfere with existing configurations.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (2)

27-27: New field appId added.

The appId field is now part of the ConfigChange class. Ensure this field is correctly utilized in all relevant methods and that it is properly initialized.

36-45: Constructor updated to include appId.

The constructor now includes the appId parameter. Ensure that this parameter is correctly utilized in the initialization logic and that it is consistently propagated throughout the configuration change management logic.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesConfigFile.java (1)

39-41: Constructor change: Initialization of appId looks good.

The constructor correctly passes appId to the superclass.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YamlConfigFile.java (1)

39-40: Constructor change: Initialization of appId looks good.

The constructor correctly passes appId to the superclass.

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryFileCachePropertyTest.java (3)

41-46: Test setup: Initialization of appId and namespace looks good.

The test setup correctly initializes someAppId and someNamespace.

58-62: Test method: Handling of appId looks good.

The test method correctly passes appId in the method calls and assertions.

70-74: Test method: Handling of appId looks good.

The test method correctly passes appId in the method calls and assertions.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEventTest.java (1)

74-77: Test method: Handling of appId looks good.

The test method correctly passes appId in the ConfigChange and fireConfigChange method calls.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java (3)

61-73: Concurrency handling review.

The method uses computeIfAbsent and synchronized blocks for concurrency control. This approach is correct, but ensure that the lock object is not excessively contended.

Line range hint 86-119:
Concurrency handling review.

The method uses computeIfAbsent and synchronized blocks for concurrency control. This approach is correct, but ensure that the lock object is not excessively contended.

57-58: Verify the function usage for the updated signature.

The getConfig method now includes appId as a parameter. Ensure all calls to this method are updated accordingly.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/property/SpringValue.java (1)

131-131: LGTM!

The isTargetBeanValid method correctly checks if the bean reference is still valid.

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (1)

116-119: Verify the function usage for the updated signature.

The setConfigFactory method now includes appId as a parameter. Ensure all calls to this method are updated accordingly.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (2)
Line range hint 106-126:
Concurrency handling review.

The method uses synchronized blocks for concurrency control. This approach is correct, but ensure that the lock object is not excessively contended.

57-58: Verify the constructor usage for the updated signature.

The SimpleConfig constructor now includes appId as a parameter. Ensure all calls to this constructor are updated accordingly.

Verification successful

Verified the constructor usage for the updated signature.

The SimpleConfig constructor now includes appId as a parameter, and all calls to this constructor have been updated accordingly.

All instances of SimpleConfig in the codebase correctly use the new constructor signature.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `SimpleConfig` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurances of the new signature.
rg --type java -A 5 $'SimpleConfig'
Length of output: 46034
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/JsonConfigFileTest.java (6)

Line range hint 42-51:
LGTM! Initialization of someAppId and someNamespace.

The initialization of someAppId and someNamespace in the setUp method is correct.

67-67: LGTM! Integration of someAppId in testWhenHasContent.

The integration of someAppId in the JsonConfigFile constructor and the subsequent assertions are correct.

80-80: LGTM! Integration of someAppId in testWhenHasNoContent.

The integration of someAppId in the JsonConfigFile constructor and the subsequent assertions are correct.

90-90: LGTM! Integration of someAppId in testWhenConfigRepositoryHasError.

The integration of someAppId in the JsonConfigFile constructor and the subsequent assertions are correct.

Line range hint 110-121:
LGTM! Integration of someAppId in testOnRepositoryChange.

The integration of someAppId in the JsonConfigFile constructor and the onRepositoryChange method is correct. The test verifies the behavior correctly.

139-145: LGTM! Integration of someAppId in testWhenConfigRepositoryHasErrorAndThenRecovered.

The integration of someAppId in the JsonConfigFile constructor and the onRepositoryChange method is correct. The test verifies the recovery behavior correctly.

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java (13)

101-105: LGTM! Addition of create method with appId in MockConfigRegistry.

The addition of the create method with the appId parameter in the MockConfigRegistry class is correct.

111-115: LGTM! Addition of createConfigFile method with appId in MockConfigRegistry.

The addition of the createConfigFile method with the appId parameter in the MockConfigRegistry class is correct.

123-126: LGTM! Addition of register method with appId in MockConfigRegistry.

The addition of the register method with the appId parameter in the MockConfigRegistry class is correct.

136-139: LGTM! Addition of getFactory method with appId in MockConfigRegistry.

The addition of the getFactory method with the appId parameter in the MockConfigRegistry class is correct.

148-152: LGTM! Addition of create method with appId in SomeConfigFactory.

The addition of the create method with the appId parameter in the SomeConfigFactory class is correct.

158-161: LGTM! Addition of createConfigFile method with appId in SomeConfigFactory.

The addition of the createConfigFile method with the appId parameter in the SomeConfigFactory class is correct.

170-174: LGTM! Addition of create method with appId in AnotherConfigFactory.

The addition of the create method with the appId parameter in the AnotherConfigFactory class is correct.

180-183: LGTM! Addition of createConfigFile method with appId in AnotherConfigFactory.

The addition of the createConfigFile method with the appId parameter in the AnotherConfigFactory class is correct.

116-119: LGTM! Addition of getFactory method with appId in MockConfigFactoryManager.

The addition of the getFactory method with the appId parameter in the MockConfigFactoryManager class is correct.

139-142: LGTM! Addition of create method with appId in MockConfigFactoryManager.

The addition of the create method with the appId parameter in the MockConfigFactoryManager class is correct.

Line range hint 144-146:
LGTM! Addition of createConfigFile method with appId in MockConfigFactoryManager.

The addition of the createConfigFile method with the appId parameter in the MockConfigFactoryManager class is correct.

171-173: LGTM! Addition of createConfigFile method with appId in MockConfigFactoryManager.

The addition of the createConfigFile method with the appId parameter in the MockConfigFactoryManager class is correct.

177-180: LGTM! Addition of getFactory method with appId in MockConfigFactoryManager.

The addition of the getFactory method with the appId parameter in the MockConfigFactoryManager class is correct.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigFile.java (4)

61-63: LGTM! Integration of appId in the constructor.

The integration of the appId parameter in the constructor of the AbstractConfigFile class is correct.

85-88: LGTM! Implementation of getAppId.

The implementation of the getAppId method in the AbstractConfigFile class is correct.

99-100: LGTM! Integration of appId in onRepositoryChange.

The integration of the appId parameter in the onRepositoryChange method of the AbstractConfigFile class is correct.

Line range hint 102-125:
LGTM! Integration of appId in the synchronized onRepositoryChange method.

The integration of the appId parameter in the synchronized onRepositoryChange method of the AbstractConfigFile class is correct.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java (3)

46-54: LGTM! Initialization of someAppId.

The initialization of someAppId in the setUp method is correct.

116-119: LGTM! Addition of getFactory method with appId in MockConfigFactoryManager.

The addition of the getFactory method with the appId parameter in the MockConfigFactoryManager class is correct.

139-142: LGTM! Addition of create method with appId in MockConfigFactoryManager.

The addition of the create method with the appId parameter in the MockConfigFactoryManager class is correct.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/SimpleConfigTest.java (3)

Line range hint 52-62: LGTM! Initialization and mocking are correctly done.

The setUp function correctly initializes the someAppId variable and mocks the configRepository and propertiesFactory.

91-93: LGTM! The test case correctly verifies the functionality.

The testGetProperty test case correctly verifies the getProperty and getSourceType methods of the SimpleConfig class.

Line range hint 138-166: LGTM! The test case correctly verifies the functionality.

The testOnRepositoryChange test case correctly verifies the onRepositoryChange method of the SimpleConfig class.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (2)

56-77: LGTM! The method correctly handles multiple appId configurations.

The registerBeanDefinitions method correctly handles multiple appId configurations and sets the secret property.

106-112: LGTM! The method correctly resolves the secret using the environment.

The resolveSecret method correctly resolves the secret using the environment.

apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (4)

62-66: LGTM! The test case correctly verifies the functionality.

The testHackConfig test case correctly verifies the getAppConfig method of the ConfigService class.

89-91: LGTM! The test case correctly verifies the functionality.

The testMockConfigFactory test case correctly verifies the getConfig method of the ConfigService class.

102-103: LGTM! The test case correctly verifies the functionality.

The testMockConfigFactoryForConfigFile test case correctly verifies the getConfigFile method of the ConfigService class.

197-212: LGTM! The methods correctly create Config and ConfigFile instances.

The MockConfigFactory class methods correctly create Config and ConfigFile instances with the appId parameter.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/AbstractConfigTest.java (3)

68-72: LGTM! The test case correctly verifies the functionality.

The testFireConfigChange_cannot_notify test case correctly verifies the fireConfigChange method of the AbstractConfig class.

111-112: LGTM! The test case correctly verifies the functionality.

The testFireConfigChange_event_notify_once test case correctly verifies the fireConfigChange method of the AbstractConfig class.

156-158: LGTM! The test case correctly verifies the functionality.

The testFireConfigChange_changes_notify_once test case correctly verifies the fireConfigChange method of the AbstractConfig class.
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (4)
99-100: LGTM!

The changes correctly integrate the appId parameter.

104-118: LGTM!

The changes correctly integrate the appId parameter.

124-138: LGTM!

The changes correctly integrate the appId parameter.

68-69: Verify the usage of create method with appId.

Ensure that all calls to the create method are updated to use the new method with appId where appropriate.

Verification successful

Verify the usage of create method with appId.

The create method with the appId parameter is correctly used in the codebase. The instances of the new method signature have been found and verified.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all calls to the `create` method with `appId`.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'create($_, $_)'
Length of output: 38

Script:
#!/bin/bash
# Description: Verify all calls to the `create` method with `appId`.

# Test: Search for the function usage. Expect: Occurrences of the new signature.
rg --type java -A 5 -w 'create'
Length of output: 96565
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesConfigFileTest.java (8)

60-60: LGTM!

The initialization of someAppId is correct and integrates well with existing code.

85-85: LGTM!

The changes correctly integrate the appId parameter.

97-97: LGTM!

The changes correctly integrate the appId parameter.

107-107: LGTM!

The changes correctly integrate the appId parameter.

123-123: LGTM!

The changes correctly integrate the appId parameter.

162-162: LGTM!

The changes correctly integrate the appId parameter.

182-182: LGTM!

The changes correctly integrate the appId parameter.

197-197: LGTM!

The changes correctly integrate the appId parameter.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (6)

68-70: LGTM!

The initialization of configUtil is correct and integrates well with existing code.

73-74: LGTM!

The changes correctly integrate the appId parameter.

76-79: LGTM!

The changes correctly integrate the appId parameter.

102-118: LGTM!

The changes correctly integrate the appId parameter.

176-176: LGTM!

The reset logic is correct and integrates well with existing code.

Line range hint 204-208: LGTM!

The changes correctly integrate the appId parameter.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepositoryTest.java (7)

122-122: LGTM!

The changes correctly integrate the appId parameter.

122-122: LGTM!

The changes correctly integrate the appId parameter.

138-138: LGTM!

The changes correctly integrate the appId parameter.

150-150: LGTM!

The changes correctly integrate the appId parameter.

164-171: LGTM!

The changes correctly integrate the appId parameter.

Line range hint 187-208: LGTM!

The changes correctly integrate the appId parameter.

204-204: LGTM!

The changes correctly integrate the appId parameter.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/XmlConfigFileTest.java (5)

52-52: Initialize someAppId correctly.

Ensure that the someAppId variable is correctly initialized and used in the tests.

Also applies to: 61-61

88-88: Pass appId correctly to constructor.

Ensure that the appId parameter is correctly passed to the XmlConfigFile constructor.

100-100: Pass appId correctly to constructor.

Ensure that the appId parameter is correctly passed to the XmlConfigFile constructor.

110-110: Pass appId correctly to constructor.

Ensure that the appId parameter is correctly passed to the XmlConfigFile constructor.

126-126: Pass appId correctly to constructor and method.

Ensure that the appId parameter is correctly passed to the XmlConfigFile constructor and onRepositoryChange method.

Also applies to: 143-143

apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java (1)

121-121: Initialize someAppId correctly.

Ensure that the someAppId variable is correctly initialized and used in the integration tests.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/YamlConfigFileTest.java (5)

42-42: Initialize someAppId correctly.

Ensure that the someAppId variable is correctly initialized and used in the tests.

Also applies to: 55-55

89-89: Pass appId correctly to constructor.

Ensure that the appId parameter is correctly passed to the YamlConfigFile constructor.

115-115: Pass appId correctly to constructor.

Ensure that the appId parameter is correctly passed to the YamlConfigFile constructor.

129-129: Pass appId correctly to constructor.

Ensure that the appId parameter is correctly passed to the YamlConfigFile constructor.

203-203: Pass appId correctly to constructor and method.

Ensure that the appId parameter is correctly passed to the YamlConfigFile constructor and onRepositoryChange method.

Also applies to: 215-215

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (6)

52-52: Add and initialize m_appId field correctly.

Ensure that the appId parameter is correctly passed and assigned to the m_appId field.

Also applies to: 66-71

139-150: Overload onRepositoryChange correctly.

Ensure that the appId parameter is correctly passed and utilized in the method.

Line range hint 151-193: Ensure existing logic remains intact.

The sync method does not involve changes related to appId. Ensure that the existing logic remains intact.

Line range hint 194-208: Ensure existing logic remains intact.

The trySyncFromUpstream method does not involve changes related to appId. Ensure that the existing logic remains intact.

Line range hint 209-222: Ensure existing logic remains intact.

The updateFileProperties method does not involve changes related to appId. Ensure that the existing logic remains intact.

142-150: Overload onRepositoryChange correctly.

Ensure that the appId parameter is correctly passed and utilized in the method.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (6)
49-49: Add appId initialization.

The new m_appId field is correctly initialized in the constructor.

224-225: Update onRepositoryChange method to include appId.

The onRepositoryChange method has been overloaded to include appId. Ensure it is called with the correct parameters.

Line range hint 227-245:
Add new onRepositoryChange method with appId.

The new onRepositoryChange method correctly includes the appId parameter. Ensure consistency in its usage.

Line range hint 309-321:
Load resource config using appId.

The loadFromResource method now includes appId in the resource path. Ensure the resource files follow this naming convention.

258-258: Update calcPropertyChanges call to include appId.

The calcPropertyChanges method is now called with appId. Ensure the method signature matches this change.
Verification successful

Method signature already includes appId.

The calcPropertyChanges method signature in AbstractConfig.java already includes appId as a parameter, which matches the suggested change.
calcPropertyChanges method definition:
List<ConfigChange> calcPropertyChanges(String appId, String namespace, Properties previous, Properties current)
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the method signature of `calcPropertyChanges` to include `appId`.

# Test: Search for the method definition. Expect: Signature includes `appId`.
rg --type java $'List<ConfigChange> calcPropertyChanges'
Length of output: 236
61-68: Constructor updated to include appId.

The constructor has been updated to include the appId parameter. Ensure all calls to this constructor are updated accordingly.

Verification successful

Constructor updated to include appId successfully.

All calls to the DefaultConfig constructor have been updated to include the appId parameter as required.

Verified instances in:

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistryTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryFileCachePropertyTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `DefaultConfig` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new DefaultConfig'
Length of output: 26924
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigRepository.java (5)
75-75: Add appId initialization.

The new m_appId field is correctly initialized in the constructor.

191-194: Update loadApolloConfig to use m_appId.

The loadApolloConfig method now uses m_appId. Ensure the logic correctly handles the appId.

336-336: Update submit call to include appId.

The submit method is now called with appId. Ensure consistency in its usage.

93-97: Constructor updated to include appId.

The constructor has been updated to include the appId parameter. Ensure all calls to this constructor are updated accordingly.

Verification successful

Constructor calls updated to include appId.

All instances of the RemoteConfigRepository constructor have been updated to include the appId parameter:

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `RemoteConfigRepository` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new RemoteConfigRepository'
Length of output: 8134
160-160: Update fireRepositoryChange call to include appId.

The fireRepositoryChange method is now called with appId. Ensure the method signature matches this change.
Verification successful

The method fireRepositoryChange in AbstractConfigRepository.java does include appId in its signature:
protected void fireRepositoryChange(String appId, String namespace, Properties newProperties) {
The call to fireRepositoryChange in RemoteConfigRepository.java appears to correctly pass m_appId as the first parameter:
this.fireRepositoryChange(m_appId, m_namespace, this.getConfig());
To ensure everything is correctly implemented, let's verify the definition of m_appId within RemoteConfigRepository.java.

Verified: The fireRepositoryChange method call in RemoteConfigRepository.java correctly includes appId as per the method signature in AbstractConfigRepository.java.

The m_appId is properly defined and initialized in the RemoteConfigRepository class.

The call to fireRepositoryChange correctly passes m_appId as the first parameter.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the method signature of `fireRepositoryChange` to include `appId`.

# Test: Search for the method definition. Expect: Signature includes `appId`.
rg --type java $'protected void fireRepositoryChange'
Length of output: 247

Script:
#!/bin/bash
# Description: Verify the definition and usage of `m_appId` in RemoteConfigRepository.java

# Search for the definition of `m_appId` in RemoteConfigRepository.java
rg 'm_appId' --type java -A 5

# Search for the constructor of `RemoteConfigRepository` to see how `m_appId` is initialized
rg 'public RemoteConfigRepository' --type java -A 10
Length of output: 30555
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (9)

71-73: Use ConcurrentHashMap and Table for managing configurations.

The use of ConcurrentHashMap and Table improves concurrency handling. Ensure thread safety throughout the code.

106-112: Update submit method to include appId.

The submit method is now called with appId. Ensure consistency in its usage.

Line range hint 115-141:
Update startLongPolling method to include appId.

The startLongPolling method is now called with appId. Ensure consistency in its usage.

172-172: Update URL assembly to include appId notifications.

The assembleLongPollRefreshUrl method now includes appId in the notifications. Ensure the URL format is correct.

190-193: Update notify method to include appId.

The notify method is now called with appId. Ensure consistency in its usage.

211-211: Update error logging to include appId.

The error logging now includes appId. This change improves debugging and traceability.

223-236: Update notify method to include appId.

The notify method is now called with appId. Ensure consistency in its usage.

247-260: Update updateNotifications method to include appId.

The updateNotifications method is now called with appId. Ensure consistency in its usage.

286-287: Update assembleNamespaces method to include appId.

The assembleNamespaces method now includes appId. Ensure consistency in its usage.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (8)

148-148: Add appId parameter in test setup.

The RemoteConfigRepository is now instantiated with appId in the test setup. Ensure consistency in all tests.

174-174: Update test to include appId parameter.

The test testLoadConfigWithOrderedProperties has been updated to include appId. Ensure test coverage for the new parameter.

211-211: Update test to include appId parameter.

The test testLoadConfigWithAccessKeySecret has been updated to include appId. Ensure test coverage for the new parameter.

224-224: Update test to include appId parameter.

The test testGetRemoteConfigWithServerError has been updated to include appId. Ensure test coverage for the new parameter.

237-237: Update test to include appId parameter.

The test testGetRemoteConfigWithNotFount has been updated to include appId. Ensure test coverage for the new parameter.

Line range hint 254-265:
Update test to include appId parameter.

The test testRepositoryChangeListener has been updated to include appId. Ensure test coverage for the new parameter.

290-290: Update test to include appId parameter.

The test testLongPollingRefresh has been updated to include appId. Ensure test coverage for the new parameter.

342-342: Update test to include appId parameter.

The test testAssembleQueryConfigUrl has been updated to include appId. Ensure test coverage for the new parameter.
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (1)
513-518: LGTM! Verify test coverage for the new method.

The getAppSecret method correctly retrieves the app secret based on the given appId. Ensure that there are tests covering this new functionality.

Verification successful

Indirect test coverage for getAppSecret is confirmed.

The method getAppSecret is indirectly tested through various test cases in RemoteConfigRepositoryTest.java, such as testLoadConfigWithAccessKeySecret. This ensures that the functionality is adequately covered.

RemoteConfigRepositoryTest.java: Contains tests that indirectly verify the behavior of getAppSecret.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the existence of tests for the getAppSecret method.

# Test: Search for test cases. Expect: Test cases for getAppSecret method.
rg --type java -A 5 'getAppSecret'
Length of output: 2449

Script:
#!/bin/bash
# Description: Search for test classes related to RemoteConfigRepository and RemoteConfigLongPollService to check if getAppSecret is indirectly tested.

# Find test files for RemoteConfigRepository
rg --type java -l 'RemoteConfigRepository' | grep -i 'test'

# Find test files for RemoteConfigLongPollService
rg --type java -l 'RemoteConfigLongPollService' | grep -i 'test'
Length of output: 704

Script:
#!/bin/bash
# Description: Check if getAppSecret is tested indirectly in the test files.

# Search for invocations of getAppSecret in RemoteConfigRepositoryTest.java
rg --type java 'getAppSecret' apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

# Search for invocations of getAppSecret in RemoteConfigLongPollServiceTest.java
rg --type java 'getAppSecret' apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java
Length of output: 258

Script:
#!/bin/bash
# Description: Examine the contents of the test files to confirm if getAppSecret is indirectly tested.

# Display the contents of RemoteConfigRepositoryTest.java
cat apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

# Display the contents of RemoteConfigLongPollServiceTest.java
cat apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java
Length of output: 40892
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfig.java (2)

457-465: LGTM! The method update is consistent.

The fireConfigChange method now correctly includes appId in the change event handling logic.

Line range hint 570-605:
LGTM! The method update is consistent.

The calcPropertyChanges method now correctly includes appId in the property change calculation logic.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (5)

136-136: LGTM! The test case update is consistent.

The testSubmitLongPollNamespaceWith304Response method now correctly includes appId as a parameter.

187-187: LGTM! The test case update is consistent.

The testSubmitLongPollNamespaceWith200Response method now correctly includes appId as a parameter.

249-249: LGTM! The test case update is consistent.

The testSubmitLongPollNamespaceWithAccessKeySecret method now correctly includes appId as a parameter.

320-323: LGTM! The test case update is consistent.

The testSubmitLongPollMultipleNamespaces method now correctly includes appId as a parameter.

391-392: LGTM! The test case update is consistent.

The testSubmitLongPollMultipleNamespacesWithMultipleNotificationsReturned method now correctly includes appId as a parameter.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java (30)

74-74: Initialize someAppId field

The someAppId field is introduced to support the new constructor signature of DefaultConfig.

95-95: Set someAppId in setup method

Initializing someAppId in the setup method ensures it's available for all tests.

150-150: Update constructor call to include appId

The constructor of DefaultConfig is updated to include appId. Ensure all instances are updated accordingly.

183-183: Update constructor call to include appId

Consistently updating the constructor to include appId.

202-202: Update constructor call to include appId

Ensure consistency in constructor updates.

225-225: Update constructor call to include appId

Constructor update is consistent with the new signature.

256-256: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

288-288: Update constructor call to include appId

Constructor updated to match new signature.

323-323: Update constructor call to include appId

Constructor update is consistent with the new signature.

346-346: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

369-369: Update constructor call to include appId

Constructor updated to match new signature.

392-392: Update constructor call to include appId

Constructor update is consistent with the new signature.

415-415: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

438-438: Update constructor call to include appId

Constructor updated to match new signature.

461-461: Update constructor call to include appId

Constructor update is consistent with the new signature.

485-485: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

521-521: Update constructor call to include appId

Constructor updated to match new signature.

547-547: Update constructor call to include appId

Constructor update is consistent with the new signature.

577-577: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

596-596: Update constructor call to include appId

Constructor updated to match new signature.

614-614: Update constructor call to include appId

Constructor update is consistent with the new signature.

652-652: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

719-719: Update ConfigChangeEvent constructor to include appId

Constructor update is consistent with the new signature.

745-745: Update constructor call to include appId

Constructor updated to match new signature.

805-805: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

840-840: Update constructor call to include appId

Constructor updated to match new signature.

867-867: Update constructor call to include appId

Constructor update is consistent with the new signature.

880-880: Update constructor call to include appId

Constructor update aligns with the new appId parameter.

900-900: Update constructor call to include appId

Constructor updated to match new signature.

926-926: Update constructor call to include appId

Constructor update is consistent with the new signature.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 598e03a and 39d3d86.

Files selected for processing (4)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (4 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (10 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (9 hunks)

Files skipped from review as they are similar to previous changes (3)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

Additional comments not posted (8)

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8)

30-31: Imports approved.

The new imports for HashBasedTable and Table from Guava are necessary for the changes in the data structure to support the appId parameter.

64-64: Field type change approved.

The CONFIG_REGISTRY field type change to Table<String, String, Config> is necessary to support the new appId parameter.

90-97: Method update approved.

The prepareConfig method update to include the appId parameter ensures that configurations can be prepared for specific application IDs.

118-124: Method update approved.

The prepareYamlConfigFile method update to include the appId parameter ensures that YAML configuration files can be prepared for specific application IDs.

166-167: Method update approved.

The mockConfig method update to include the appId parameter ensures that configurations can be mocked for specific application IDs.

183-188: Method update approved.

The doSetUp method update to include setup for MockConfigUtil ensures that the configuration utility can be properly mocked for testing.

Line range hint 206-225:
Class update approved.

The MockConfigManager class update to include methods with the appId parameter ensures that the mock configuration manager can handle configurations for specific application IDs.

246-248: Class update approved.

The MockConfigUtil class update to include the getAppId method ensures that the mock configuration utility can provide an application ID for testing.

nobodyiam · 2024-07-14T10:41:38Z

Thanks, it may take some time to reivew. In the meantime, could you please sign the CLA following the instructions provided by the CLA Assistant Lite bot?

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 39d3d86 and db6f733.

Files selected for processing (16)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (7 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (4 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (9 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigIntegrationTest.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java (30 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepositoryTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (10 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (29 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java (20 hunks)
apollo-core/src/main/java/com/ctrip/framework/apollo/core/utils/StringUtils.java (1 hunks)

Files skipped from review due to trivial changes (1)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java

Files skipped from review as they are similar to previous changes (6)

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java

Additional comments not posted (24)

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepositoryTest.java (3)

43-43: Addition of appId field.

The addition of the someAppId field aligns with the new requirement to handle multiple appIds. This change is necessary for testing configurations specific to an appId.

51-53: Setup method updates to handle appId.

The setUp method now initializes someAppId and mocks its retrieval. This is crucial for ensuring that the unit tests can simulate scenarios where different appIds are used to fetch configurations.

135-135: Verification of repository change handling with appId.

The testOnChange method now verifies that the onRepositoryChange method is called with the correct appId. This ensures that the repository change events are correctly propagated with the appId context, which is essential for multi-app configurations.

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (6)

29-29: Import addition for ConfigConsts.

The addition of ConfigConsts import suggests usage of constants related to configuration, possibly related to appId. This is a good practice as it promotes the use of constants instead of hard-coded values.

84-84: Modification to use appId in config repository creation.

The createConfigRepository method is modified to accept appId, which is essential for creating configurations specific to an application. This change is crucial for supporting multi-tenancy.

99-99: Local configuration repository creation with appId.

The createLocalConfigRepository method now includes appId in its parameters, ensuring that local configurations are associated with the correct application. This is a necessary change for maintaining the integrity of configurations across different applications.

117-119: Creation of PropertiesCompatibleFileConfigRepository with appId.

The method now correctly handles appId when creating a PropertiesCompatibleFileConfigRepository. This ensures that properties files are correctly associated with their respective applications, which is vital for accurate configuration management.

136-138: Multiple local repository creations with appId.

The test now covers scenarios where multiple local repositories are created with different appIds. This is important for testing the factory's ability to handle multiple applications simultaneously.

191-204: Namespace trimming with appId.

The checkNamespaceName method now includes appId in its logic. This is crucial for ensuring that namespace names are correctly processed in the context of an application, preventing potential conflicts or errors in multi-tenant environments.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java (1)

109-113: Integration of appId in ApolloConfig processing.

The processApolloConfig method now includes logic to handle appId. This is crucial for ensuring that configurations are fetched and injected based on the correct application identifier, aligning with the multi-tenancy requirements.

apollo-core/src/main/java/com/ctrip/framework/apollo/core/utils/StringUtils.java (1)

343-345: Addition of defaultIfBlank method.

The defaultIfBlank method provides a default value if the input string is blank, using appId as the fallback. This utility method is a valuable addition, especially in configurations where default values are needed based on the application context.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (5)

49-49: Addition of appId field.

The addition of the m_appId field is consistent with the PR's objective to handle configurations for multiple apps. Ensure that all usages of this field are correctly implemented across the class.

61-65: Constructor modification to include appId.

The constructor now includes an appId parameter and initializes m_appId. This change is necessary for supporting multiple app configurations. The method loadFromResource has been adapted to use appId which is a good practice to ensure configurations are loaded per app basis.

Also applies to: 68-68

224-225: Method Overloading for onRepositoryChange.

The onRepositoryChange method has been overloaded to include an appId parameter. This is a crucial change to ensure that repository changes are handled per app. The logic inside the method appears to be correct and properly checks for changes before firing a configuration change event.

Also applies to: 227-245

258-258: Calculation of property changes now includes appId.

The method calcPropertyChanges has been updated to include appId. This is important for correctly calculating changes per app. Ensure that this method is used consistently across the class.

Line range hint 309-321: Resource loading adapted for appId.

The method loadFromResource has been adapted to include appId in the resource path. This change ensures that app-specific configurations can be loaded. The error handling and resource stream closure are correctly implemented.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java (2)

60-61: Addition of appId constant for testing.

The constant someAppId has been added to use across tests, ensuring consistency when testing multi-app configurations.

96-96: Updated mockConfig calls to include appId.

All mockConfig method calls have been updated to include appId. This change is necessary to ensure that the tests are aligned with the new multi-app support in the Apollo client.

Also applies to: 140-141, 176-180, 218-218, 250-250, 284-284, 317-317, 353-353, 382-382, 414-414

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java (2)

63-63: Addition of appId constant for testing.

The constant someAppId has been added to use across tests, ensuring consistency when testing multi-app configurations.

74-74: Updated mockConfig and mockConfigFile calls to include appId.

All mockConfig and mockConfigFile method calls have been updated to include appId. This change is necessary to ensure that the tests are aligned with the new multi-app support in the Apollo client.

Also applies to: 82-82, 95-95, 111-111, 127-127, 139-139, 143-143, 161-161, 165-165, 180-180, 184-184, 199-199, 203-203, 217-217, 221-221, 235-235, 254-254, 276-276, 297-297, 315-315, 338-338, 360-360, 378-378, 409-409, 428-428, 436-436

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (4)

Line range hint 22-33: Added imports and annotations are appropriate.

The imports and the new @MultipleConfig annotation align with the changes made to support multiple appId configurations. These changes are necessary for the tests to reflect the new application structure.

79-79: Introduction of someAppId as a constant.

Defining someAppId as a constant is a good practice for reusability and maintainability in tests. This ensures that all test cases use a consistent appId, which is crucial for testing the new multi-app configuration feature.

114-115: Proper use of mockConfig with appId.

The mockConfig method is correctly used with the appId parameter to simulate configuration settings for different applications. This is key to testing the new functionality introduced in the PR.

Also applies to: 117-117

133-133: Consistent application of appId across various test methods.

Throughout the test file, the appId parameter is consistently used in various contexts, ensuring that the new functionality is thoroughly tested. This includes mocks, configuration setups, and assertions. The changes are well-integrated and cover a wide range of scenarios, which is crucial for the robustness of the feature.

Also applies to: 143-145, 159-162, 174-183, 195-196, 210-211, 264-264, 273-273, 283-284, 340-341, 372-372, 398-405, 437-448, 462-469, 493-499, 518-524, 539-544, 563-566, 578-578, 584-587, 594-598, 607-616, 625-625, 639-644, 651-675, 1093-1131

TerryLam2010 · 2024-07-16T06:24:45Z

I have read the CLA Document and I hereby sign the CLA

nobodyiam · 2024-07-20T12:32:14Z

@TerryLam2010 It seems you committed the code with [email protected], which isn't associated with your GitHub account.

...ig-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

...src/main/java/com/ctrip/framework/apollo/spring/property/AutoUpdateConfigChangeListener.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/property/SpringValue.java

apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java

Anilople · 2024-08-12T12:44:50Z

The monitor metric #74 maybe changed if support multiple appId @Rawven

Rawven · 2024-08-12T12:58:17Z

如果支持多个 appId，监控指标#74可能会改变 @Rawven

I'll adapt it after it's merged

coderabbitai

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (5)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1)
73-76: Integration of multipleConfigs() Method Not Evident

The multipleConfigs() method in the EnableApolloConfig annotation is not currently utilized in the codebase. While EnableApolloConfig is used in several places, particularly in test files, there is no evidence of multipleConfigs() being integrated or tested. Consider reviewing the intended use cases and updating the codebase to leverage this new method if necessary.

Analysis chain

LGTM! Verify the integration of multipleConfigs().

The addition of the multipleConfigs() method is well-implemented and enhances flexibility. Ensure that the integration of this method is correctly handled throughout the codebase.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the integration of `multipleConfigs()` method across the codebase.

# Test: Search for the usage of `multipleConfigs()` method. Expect: Correct integration throughout the codebase.
rg --type java 'multipleConfigs'
Length of output: 184

Script:
#!/bin/bash
# Search for references to the EnableApolloConfig class to see if it is used elsewhere.
rg --type java 'EnableApolloConfig'
Length of output: 10994
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (2)
123-125: Document the placeholder method.

The createConfigFile method currently returns null. Consider documenting its placeholder status and any future implementation plans.
+  // TODO: Implement the method or provide documentation on its intended use.
84-86: Update getConfig Method Calls to Match New Signature

The getConfig method calls in the following locations still use the old signature with a single namespace parameter. They need to be updated to include the appId parameter:

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/importer/ApolloConfigDataLoader.java

apollo-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigIntegrationTest.java

Ensure all invocations of getConfig are updated to use the new signature with both appId and namespace.

Analysis chain

LGTM! But verify the function usage in the codebase.

The addition of appId to getConfig enhances configuration specificity. Ensure that all function calls to getConfig match the new signature.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `getConfig` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'getConfig\$String\\s+appId,\\s+String\\s+namespace\$'
Length of output: 2477

Script:
#!/bin/bash
# Description: Check all invocations of `getConfig` in the codebase to ensure they match the new signature.

# Search for all `getConfig` calls and display surrounding context to verify parameters.
rg --type java -A 2 'getConfig\('
Length of output: 53637
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/DefaultApplicationProvider.java (1)
119-125: Handle potential null value for m_appId to prevent NullPointerException.

The initialization logic for m_appId does not guarantee a non-null value, which could lead to a NullPointerException in the getAccessKeySecret(String appId) method. Consider adding a null check for m_appId before calling equals.

Location: DefaultApplicationProvider.java, method getAccessKeySecret(String appId)

Analysis chain

Ensure consistent access key retrieval logic.

The new method getAccessKeySecret(String appId) enhances flexibility by allowing access key retrieval based on app ID. However, consider handling potential null values for m_appId to prevent NullPointerException.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that `m_appId` is initialized before `getAccessKeySecret(String appId)` is called.

# Test: Search for initialization and usage of `m_appId`. Expect: Proper initialization before usage.
rg --type java 'm_appId' -A 5
Length of output: 31722
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (1)

206-207: Consider implementing appId and namespace logic.

The appIdAndNamespace method currently returns the namespace unchanged. Consider implementing logic to combine appId and namespace if needed.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between db6f733 and e814053.

Files selected for processing (26)

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigFile.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (8 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (7 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (12 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigRepository.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigFileChangeEvent.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/build/MockInjector.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (7 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (29 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/DefaultApplicationProvider.java (1 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/NullProvider.java (1 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java (1 hunks)

Files skipped from review due to trivial changes (1)

apollo-client/src/test/java/com/ctrip/framework/apollo/build/MockInjector.java

Additional comments not posted (83)

apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java (1)
50-53: Addition of getAccessKeySecret(String appId) approved.

The new method enhances the interface by providing access to the access key secret for a specific app ID. Ensure that this method is correctly implemented in all classes that implement this interface.

Verify the implementation of this method across the codebase to ensure consistency.

Verification successful

Method Implemented Consistently

The getAccessKeySecret(String appId) method is implemented in the following classes that implement the ApplicationProvider interface:

DefaultApplicationProvider

NullProvider

This confirms that the method has been consistently implemented across the codebase.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the implementation of `getAccessKeySecret(String appId)` in classes implementing `ApplicationProvider`.

# Test: Search for the method implementation. Expect: Implementations in all relevant classes.
ast-grep --lang java --pattern $'class $_ implements ApplicationProvider {
  $$$
  String getAccessKeySecret(String appId) {
    $$$
  }
  $$$
}'
Length of output: 146

Script:
#!/bin/bash
# Find all classes implementing ApplicationProvider and check for the getAccessKeySecret method.
fd -e java -x grep -H 'class .* implements ApplicationProvider' {} | cut -d: -f1 | sort -u | xargs -I {} sh -c 'echo "Checking {}"; grep -q "String getAccessKeySecret(String appId)" {} && echo "Method implemented in {}" || echo "Method not implemented in {}"'
Length of output: 739
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (2)
51-58: Addition of getConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) approved.

The method extends configuration file retrieval capabilities by allowing specification of app ID and namespace. Ensure that this method is correctly implemented and tested.

Verify the implementation and usage of this method across the codebase.

Verification successful

Implementation and Usage Verified

The getConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) method is implemented in the DefaultConfigManager class and is also present in a test class, indicating it is being tested. This confirms the method's implementation and usage in the codebase. No further issues found.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the implementation and usage of `getConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat)`.

# Test: Search for the method implementation and usage. Expect: Implementations and usages in all relevant classes.
ast-grep --lang java --pattern $'class $_ implements ConfigManager {
  $$$
  ConfigFile getConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) {
    $$$
  }
  $$$
}'
Length of output: 772

Script:
#!/bin/bash
# Description: Find the implementation of `getConfigFile` method in classes implementing `ConfigManager`.

# Search for classes implementing `ConfigManager` and having `getConfigFile` method.
ast-grep --lang java --pattern $'class $_ implements ConfigManager {
  $$$
  public ConfigFile getConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat);
  $$$
}'
Length of output: 186

Script:
#!/bin/bash
# Description: Find the usage of `getConfigFile` method across the codebase.

# Search for usages of the `getConfigFile` method.
rg 'getConfigFile\s*$\s*String\s+appId\s*,\s*String\s+namespace\s*,\s*ConfigFileFormat\s+configFileFormat\s*$' -A 3
Length of output: 1711
35-41: Addition of getConfig(String appId, String namespace) approved.

The method enhances configuration management by allowing retrieval based on both app ID and namespace. Ensure that this method is correctly implemented and tested.

Verify the implementation and usage of this method across the codebase.

Verification successful

Implementation and Usage of getConfig(String appId, String namespace) Verified

The getConfig(String appId, String namespace) method is implemented and used correctly across the codebase:

DefaultConfigManager: Contains the main implementation of the method.

ConfigService: Provides a static method that delegates to the ConfigManager implementation.

AbstractSpringIntegrationTest: Used for testing purposes.

These implementations ensure the method is integrated into the configuration management workflow.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the implementation and usage of `getConfig(String appId, String namespace)`.

# Test: Search for the method implementation and usage. Expect: Implementations and usages in all relevant classes.
ast-grep --lang java --pattern $'class $_ implements ConfigManager {
  $$$
  Config getConfig(String appId, String namespace) {
    $$$
  }
  $$$
}'
Length of output: 149

Script:
#!/bin/bash
# Search for the implementation of `getConfig(String appId, String namespace)` across the codebase
rg 'Config getConfig$String appId, String namespace$' -A 5
Length of output: 2466
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1)
38-44: Addition of default method getAppId() approved.

The method provides a way to retrieve the app ID associated with the config file instance, with a default implementation returning null. Ensure that this method is overridden where necessary.

Verify the usage and overriding of this method across the codebase.

Verification successful

getAppId() Method Usage Verified

The getAppId() method in the ConfigFile interface is overridden in the AbstractConfigFile class, which is the only class implementing ConfigFile. This confirms that the method is appropriately overridden where necessary. No further action is required.

Implementations of ConfigFile:

AbstractConfigFile (overrides getAppId())
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the usage and overriding of `getAppId()` in classes implementing `ConfigFile`.

# Test: Search for the method overriding. Expect: Overriding in all relevant classes.
ast-grep --lang java --pattern $'class $_ implements ConfigFile {
  $$$
  @Override
  String getAppId() {
    $$$
  }
  $$$
}'
Length of output: 127

Script:
#!/bin/bash
# Find all classes implementing ConfigFile
ast-grep --lang java --pattern 'class $_ implements ConfigFile { $$$ }' --json | jq -r '.[].matches[].text' > implementing_classes.txt

# Check for getAppId method in these classes
while IFS= read -r class; do
  echo "Checking class: $class"
  ast-grep --lang java --pattern "class $class { $$$ String getAppId() { $$$ } $$$ }"
done < implementing_classes.txt
Length of output: 132

Script:
#!/bin/bash
# Search for any implementation of ConfigFile
rg --no-heading --line-number 'implements ConfigFile' > implementations.txt

# Search for any usage or overriding of getAppId()
rg --no-heading --line-number 'getAppId' > getAppId_usages.txt

# Output the results
echo "Implementations of ConfigFile:"
cat implementations.txt

echo "Usages or overrides of getAppId():"
cat getAppId_usages.txt
Length of output: 10138
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/NullProvider.java (1)

87-90: Addition of getAccessKeySecret(String appId) aligns with class purpose.

The new method getAccessKeySecret(String appId) returns null, which is consistent with the NullProvider class's role as a default or placeholder implementation. This ensures that the interface contract is fulfilled without altering existing behavior.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigFileChangeEvent.java (4)

26-26: Introduction of appId field enhances context.

The new appId field provides additional context for configuration file change events, aligning with the PR's objective to support multiple app IDs.

41-43: Constructor updated to include appId.

The constructor now accepts appId, ensuring that instances of ConfigFileChangeEvent are initialized with complete context.

50-52: Addition of getAppId() method.

The getAppId() method provides access to the appId field, improving encapsulation and usability.

73-73: toString() method updated to include appId.

Including appId in the toString() method enhances the string representation of the event, providing more detailed information.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (3)

28-28: Addition of m_appId field enhances context.

The m_appId field provides additional context for configuration change events, aligning with the PR's objective to support multiple app IDs.

36-38: Constructor updated to include appId.

The constructor now includes appId, ensuring that ConfigChangeEvent instances are initialized with complete context.

83-85: Addition of getAppId() method.

The getAppId() method allows retrieval of the appId, enhancing the class's utility.
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (1)
27-27: LGTM! Verify the impact of appId integration.

The introduction of the appId field and its integration into the constructor and the new getter method are well-executed. Ensure that existing code using the ConfigChange constructor is updated accordingly.

Also applies to: 43-45, 81-83

Verification successful

Integration of appId Verified in ConfigChange Constructor

The addition of the appId parameter to the ConfigChange constructor has been successfully integrated across the codebase. All instances of ConfigChange creation have been updated to include the appId parameter.

Files Updated:

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfig.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/AbstractConfigTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEventTest.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the impact of `appId` integration in the `ConfigChange` constructor across the codebase.

# Test: Search for instantiations of `ConfigChange`. Expect: Updated constructor usage with `appId`.
rg --type java 'new ConfigChange\('
Length of output: 1593
apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (1)
45-55: LGTM! Verify the usage of the new constructor.

The addition of the new constructor with appId is well-implemented and maintains backward compatibility. Ensure that the new constructor is used appropriately in the codebase.

Verification successful

Verified Usage of New Constructor

The new constructor for PureApolloConfig with parameters appId, namespace, and configRepository is correctly used in the codebase. It appears in PureApolloConfigFactory.java, and no other constructors are in use, ensuring consistency and correctness.

Location of usage: apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfigFactory.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the usage of the new `PureApolloConfig` constructor with `appId`.

# Test: Search for instantiations of `PureApolloConfig`. Expect: Usage of both constructors.
rg --type java 'new PureApolloConfig\('
Length of output: 228

Script:
#!/bin/bash
# Description: Search for all instantiations of `PureApolloConfig` to verify constructor usage.

# Test: List all usages of `PureApolloConfig` constructors.
rg --type java 'new PureApolloConfig\(' -A 3
Length of output: 475
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (1)
110-114: Verify the appId validation logic.

Ensure that the validation logic for appId is correct and that it doesn't introduce breaking changes.

Verification successful

Validation logic for appId is correctly implemented.

The validation logic in the create method uses StringUtils.equals to compare the provided appId with the expected one from m_configUtil. If they do not match, an IllegalArgumentException is thrown. This is a standard and correct approach for ensuring appId consistency.

The logic is correctly implemented and does not introduce breaking changes.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the appId validation logic in the `create` method.

# Test: Search for the appId validation logic. Expect: Correct comparison and exception handling.
rg --type java -A 5 $'if\$!StringUtils.equals\\(appId,m_configUtil.getAppId\\(\$\\)\\)'
Length of output: 699
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigFile.java (3)

61-63: Constructor changes look good.

The constructor now correctly initializes m_appId alongside other fields.

85-88: Method getAppId looks good.

The method correctly returns the initialized m_appId.

Line range hint 102-125:
Method onRepositoryChange changes look good.

The method now correctly handles configuration changes with the appId parameter.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (3)

45-46: Field initialization for configUtil looks good.

The field is correctly initialized using ApolloInjector.

56-77: Method registerBeanDefinitions changes look good.

The method now correctly handles multiple appIds and resolves secrets, enhancing configuration flexibility.

106-112: Method resolveSecret looks good.

The method correctly resolves secrets and logs warnings for unsupported Spring versions.
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (8)
68-72: LGTM! Test assertions updated for appId.

The test assertions now correctly incorporate the appId, ensuring that the configuration retrieval logic is validated for the new parameter.

83-83: LGTM! Test assertions updated for appId.

The test assertions now correctly incorporate the appId, ensuring that the configuration factory logic is validated for the new parameter.

95-95: LGTM! Test assertions updated for appId.

The test assertions now correctly incorporate the appId, ensuring that the mock configuration factory logic is validated for the new parameter.

Line range hint 113-127:
LGTM! Constructor and methods updated for appId.

The MockConfig class now correctly incorporates the appId parameter, enhancing the fidelity of mock configurations.

Line range hint 143-157:
LGTM! Constructor updated for appId.

The MockConfigFile class now correctly incorporates the appId parameter, enhancing the fidelity of mock configuration files.

203-218: LGTM! Methods updated for appId.

The MockConfigFactory class methods now correctly incorporate the appId parameter, enhancing the fidelity of mock configuration factories.

49-52: Verify the use of reflection for setting private fields.

Ensure that using reflection to access and modify private fields is necessary and correctly implemented, as it can lead to maintenance challenges.

Verification successful

Reflection usage is isolated and correctly implemented.

The use of reflection to access the m_configUtil field is limited to the ConfigServiceTest.java file, indicating it's specifically for testing purposes. This is a common practice for accessing private fields in tests and is correctly implemented in this context. No further issues found.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the use of reflection for accessing `m_configUtil` in tests.

# Test: Search for occurrences of reflection usage in test files. Expect: Consistent usage.
rg --type java --pattern 'getDeclaredField$"m_configUtil"$'
Length of output: 475

Script:
#!/bin/bash
# Description: Verify the use of reflection for accessing `m_configUtil` in tests.

# Corrected Test: Search for occurrences of reflection usage in test files. Expect: Consistent usage.
rg 'getDeclaredField$"m_configUtil"$' --type java
Length of output: 203
169-172: Verify the getAppId method returning null.

Ensure that returning null from the getAppId method is intentional and does not affect the test logic.

Verification successful

The getAppId method returning null is intentional and handled in tests.

The test suite includes assertions that explicitly check for a null return value from the getAppId() method, indicating that this behavior is expected and managed within the test logic.

DefaultApplicationProviderTest contains a test case that asserts null as the expected value.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the usage of `getAppId` method in tests.

# Test: Search for calls to `getAppId` in test files. Expect: Handled cases for null return.
rg --type java --pattern '\.getAppId'
Length of output: 455

Script:
#!/bin/bash
# Corrected: Search for calls to `getAppId` in test files to verify handling of null return.
rg '\.getAppId' --type java
Length of output: 5923
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (10)

67-68: LGTM! Method updated to use appId.

The create method now correctly delegates to the overloaded version with appId, ensuring consistent configuration creation.

Line range hint 70-91:
LGTM! Method updated to handle appId.

The create method now correctly incorporates the appId parameter, ensuring configurations are created with the correct application context.

93-95: LGTM! Method updated to use appId.

The createConfigFile method now correctly delegates to the overloaded version with appId, ensuring consistent configuration file creation.

98-99: LGTM! Method updated to handle appId.

The createRepositoryConfig method now correctly incorporates the appId parameter, ensuring repository configurations are associated with the correct application context.

103-119: LGTM! Method updated to handle appId.

The createConfigFile method now correctly incorporates the appId parameter, ensuring configuration files are created with the correct application context.

123-127: LGTM! Method updated to handle appId.

The createConfigRepository method now correctly incorporates the appId parameter, ensuring configuration repositories are created with the correct application context.

137-144: LGTM! Method updated to handle appId.

The createLocalConfigRepository method now correctly incorporates the appId parameter, ensuring local configuration repositories are created with the correct application context.

147-148: LGTM! Method updated to handle appId.

The createRemoteConfigRepository method now correctly incorporates the appId parameter, ensuring remote configuration repositories are created with the correct application context.

Line range hint 152-157:
LGTM! Method updated to handle appId.

The createPropertiesCompatibleFileConfigRepository method now correctly incorporates the appId parameter, ensuring properties-compatible file configuration repositories are created with the correct application context.

Line range hint 172-176:
LGTM! Method updated to handle appId.

The trimNamespaceFormat method now correctly incorporates the appId parameter, ensuring namespace formatting is handled with the correct application context.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (3)

59-59: LGTM! Data structure updated for appId.

The APP_NAMESPACE_NAMES field now uses a more complex structure to handle multiple namespaces with specific appIds, enhancing flexibility.

73-79: LGTM! Method updated to handle appId.

The addNamespaces method now correctly incorporates the appId parameter, ensuring namespaces are associated with the correct application context.

102-124: LGTM! Method updated for appId-based configuration retrieval.

The initializePropertySources method now correctly iterates over the new data structure, enhancing the granularity of configuration management.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8)

68-68: Transition to Table for structured configuration storage.

The use of HashBasedTable for CONFIG_REGISTRY allows for more structured storage of configurations associated with both app IDs and namespaces, enhancing flexibility.

94-101: Update method signatures to include appId.

The prepareConfig method now includes appId, improving test configuration management by associating configurations with specific app IDs.

122-130: Enhance YAML configuration handling with appId.

The prepareYamlConfigFile method now accepts appId, ensuring YAML configurations are associated with the correct application context.

170-171: Store configurations using appId and namespace.

The mockConfig method now stores configurations in the CONFIG_REGISTRY using both appId and namespace, aligning with the new table structure.

178-180: Refactor config file mocking to include appId.

The mockConfigFile method now uses appId in its key, ensuring unique identification of config files across different applications.

204-204: Clear configuration registry in teardown.

The doTearDown method clears CONFIG_REGISTRY, ensuring no residual configurations affect subsequent tests.

222-228: Refactor getConfig to use appId.

The getConfig method in MockConfigManager now requires appId, improving configuration retrieval by ensuring the correct application context is used.

Line range hint 236-242: Refactor getConfigFile to use appId.

The getConfigFile method in MockConfigManager now includes appId, aligning file retrieval with the new configuration structure.

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (6)

62-62: Initialize someAppId for test clarity.

The someAppId variable is now initialized to "someAppId", enhancing the clarity and context of the tests.

84-84: Update method calls to include appId.

The createConfigRepository method now accepts someAppId, aligning with the updated configuration management logic.

99-99: Refactor local config repository creation with appId.

The createLocalConfigRepository method now includes someAppId, ensuring the test setup reflects the new configuration structure.

117-119: Enhance properties file config repository creation with appId.

The createPropertiesCompatibleFileConfigRepository method now requires someAppId, aligning with the updated configuration logic.

136-138: Update config file creation to include appId.

The createLocalConfigRepository method calls now include someAppId, ensuring the correct application context is used in tests.

191-204: Refactor namespace name checks to include appId.

The checkNamespaceName method now incorporates appId, ensuring comprehensive validation of namespace formatting.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (7)

66-67: LGTM! Constructor changes are appropriate.

The constructor correctly initializes m_appId and delegates to the other constructor.

70-74: Constructor initialization is correct.

The constructor properly initializes all fields and sets up the local cache directory and upstream repository.

133-134: Delegation to overloaded method is correct.

The method correctly delegates to the overloaded version with m_appId.

136-144: Property update and change event handling are correct.

The method correctly updates the file properties and fires the repository change event with appId.

Line range hint 204-237:
File loading logic is correct.

The method correctly handles loading properties from a local cache file using appId and namespace, with proper exception handling.

Line range hint 239-271:
File persistence logic is correct.

The method correctly handles persisting properties to a local cache file using appId and namespace, with proper exception handling.

297-300: File path construction is correct.

The method correctly constructs the file path using appId and namespace.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (4)

67-75: Constructor initialization is correct.

The constructor properly initializes m_appId using ConfigUtil.getAppId() and sets up other fields.

Line range hint 84-92:
Constructor initialization is correct.

The constructor properly initializes all fields using the provided parameters.

243-244: Delegation to overloaded method is correct.

The method correctly delegates to the overloaded version with m_appId.

Line range hint 328-340:
Resource loading logic is correct.

The method correctly handles loading properties from a resource using appId and namespace, with proper exception handling.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigRepository.java (4)

Line range hint 96-114:
Constructor initialization is correct.

The constructor properly initializes m_appId and other fields, and sets up scheduled tasks.

157-160: Config cache update and change event handling are correct.

The method correctly updates the config cache and fires the repository change event with m_appId.

191-194: Config loading logic is correct.

The method correctly handles loading Apollo config using m_appId, with proper exception handling.

336-336: Long polling refresh scheduling is correct.

The method correctly schedules long polling refresh with m_appId.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (5)
Line range hint 115-141:
LGTM!

The inclusion of sysAppId as a parameter in startLongPolling is a necessary change to support multiple app IDs and allows long polling to be scoped by application.

223-236: LGTM!

The addition of the appId parameter in the notify method is appropriate for associating notifications with the correct application.

247-260: LGTM!

The addition of the appId parameter in the updateNotifications method is appropriate for ensuring that notification updates are correctly associated with the respective application.

286-287: LGTM!

The inclusion of the appId parameter in the assembleNamespaces method is necessary for assembling namespaces for a specific application ID.

106-112: LGTM! But verify the function usage in the codebase.

The addition of the appId parameter and the use of Table for m_longPollNamespaces and m_notifications are appropriate for supporting multiple app IDs.

However, ensure that all function calls to submit match the new signature.

Verification successful

Function submit Usage Verified

All instances of the submit function in the codebase match the new signature with appId, namespace, and remoteConfigRepository as arguments.

RemoteConfigLongPollServiceTest.java: Multiple calls using the new signature.

RemoteConfigRepository.java: Single call using the new signature.

No issues were found regarding the function usage.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `submit` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'submit'
Length of output: 17711
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java (3)

60-61: LGTM!

The introduction of the someAppId constant improves maintainability and readability by centralizing the application ID definition.

406-414: LGTM!

The addition of throws Exception to the beforeClass method enhances error handling during the setup phase of tests.

418-418: LGTM!

The addition of throws NoSuchFieldException, IllegalAccessException to the afterClass method enhances error handling during the teardown phase of tests.

apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (1)

513-515: LGTM!

The new method getAccessKeySecret appropriately extends the functionality of ConfigUtil by enabling retrieval of access key secrets for specific app IDs.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (3)

Line range hint 22-32: Imports look good.

The addition of MockInjector and MultipleConfig imports is appropriate for the new functionality.

651-675: New test method testApolloMultipleConfig looks good.

The method effectively tests multiple configurations for different app IDs and namespaces.

1094-1131: New configuration class TestApolloMultipleConfig and bean TestApolloConfigBean5 look good.

These additions support testing multiple app IDs and namespaces.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e814053 and 649b054.

Files selected for processing (5)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (9 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (14 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (29 hunks)

Additional comments not posted (28)

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (6)

94-101: LGTM!

The integration of appId into the prepareConfig method is consistent with the PR's objectives and enhances configuration handling.

122-130: LGTM!

The addition of appId to the prepareYamlConfigFile method is well-implemented and aligns with the PR's objectives.

170-171: LGTM!

The use of appId in the mockConfig method is appropriate and enhances the flexibility of configuration management.

178-180: LGTM!

The addition of appId to the mockConfigFile method and the use of a composite key enhance configuration management.

223-228: LGTM!

The integration of appId into the getConfig method in MockConfigManager is well-implemented and aligns with the PR's objectives.

Line range hint 237-242:
LGTM!

The addition of appId to the getConfigFile method in MockConfigManager enhances the retrieval process for configuration files.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java (1)

109-113: LGTM!

The integration of appId in the processApolloConfig method enhances configuration retrieval and aligns with the PR's objectives.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (7)

155-155: LGTM!

The test case testLoadConfig correctly incorporates someAppId in the RemoteConfigRepository instantiation, aligning with the updated configuration handling.

181-181: LGTM!

The test case testLoadConfigWithOrderedProperties correctly uses someAppId in the RemoteConfigRepository instantiation, ensuring proper testing of ordered properties.

218-218: LGTM!

The test case testLoadConfigWithAccessKeySecret correctly incorporates someAppId in the RemoteConfigRepository instantiation, aligning with the updated configuration handling.

231-231: LGTM!

The test case testGetRemoteConfigWithServerError correctly uses someAppId in the RemoteConfigRepository instantiation, ensuring proper error handling testing.

244-244: LGTM!

The test case testGetRemoteConfigWithNotFount correctly incorporates someAppId in the RemoteConfigRepository instantiation, aligning with the updated configuration handling.

Line range hint 261-272:
LGTM!

The test case testRepositoryChangeListener correctly verifies onRepositoryChange with someAppId, ensuring proper testing of repository change notifications.

Line range hint 295-321:
LGTM!

The test case testLongPollingRefresh correctly verifies onRepositoryChange with someAppId, ensuring proper testing of long polling refresh.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (5)
194-194: LGTM!

The addition of the appId parameter to the submit method call is consistent with the changes in other test cases.

256-256: LGTM!

The addition of the appId parameter to the submit method call is consistent with the changes in other test cases.

327-330: LGTM!

The addition of the appId parameter to the submit method calls for multiple namespaces is consistent with the changes in other test cases.

398-399: LGTM!

The addition of the appId parameter to the submit method calls for multiple namespaces with multiple notifications is consistent with the changes in other test cases.

143-143: LGTM! Verify submit method usage with appId.

The addition of the appId parameter to the submit method call is consistent with the PR objectives.

Ensure that all submit method calls in the codebase correctly include the appId parameter.

Verification successful

Let's correct the regex pattern error and re-run the script to verify the usage of the submit method with the appId parameter.

Verification Successful: submit Method Usage Consistent

The submit method calls across the codebase correctly include the appId parameter, aligning with the intended changes. No issues found.

RemoteConfigLongPollService.java: The submit method is defined with appId.

RemoteConfigLongPollServiceTest.java: Test cases consistently use appId.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all `submit` method calls include the `appId` parameter.

# Test: Search for `submit` method usage. Expect: Occurrences with the `appId` parameter.
rg --type java -A 2 $'submit('
Length of output: 96

Script:
#!/bin/bash
# Description: Verify all `submit` method calls include the `appId` parameter.

# Test: Search for `submit` method usage. Expect: Occurrences with the `appId` parameter.
rg --type java -A 2 'submit\('
Length of output: 6210
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (9)

116-119: LGTM!

The addition of someAppId to the mockConfig and prepareYamlConfigFile method calls is consistent with the PR objectives.

135-135: LGTM!

The addition of someAppId to the mockConfig method call is consistent with the changes in other test cases.

145-147: LGTM!

The addition of someAppId to the mockConfig and prepareYamlConfigFile method calls is consistent with the changes in other test cases.

161-164: LGTM!

The addition of someAppId to the mockConfig method call is consistent with the changes in other test cases.

Line range hint 176-185:
LGTM!

The addition of someAppId to the mockConfig method calls is consistent with the changes in other test cases.

197-198: LGTM!

The addition of someAppId to the mockConfig method calls is consistent with the changes in other test cases.

212-213: LGTM!

The addition of someAppId to the mockConfig method calls is consistent with the changes in other test cases.

266-266: LGTM!

The addition of someAppId to the mockConfig method call is consistent with the changes in other test cases.

660-684: LGTM!

The new test method testApolloMultipleConfig effectively tests multiple configurations for different app IDs and namespaces, aligning with the PR objectives.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 649b054 and 7e6b56a.

Files selected for processing (4)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (12 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (15 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/DefaultApplicationProvider.java (2 hunks)

Files skipped from review as they are similar to previous changes (3)

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/DefaultApplicationProvider.java

Additional comments not posted (8)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (8)

72-73: Good use of Table for structured data management.

The transition to using Table for m_longPollNamespaces and m_notifications is a suitable choice for managing multiple app IDs and namespaces. This enhances the capability to associate multiple values with a single key in a structured manner.

95-96: Thread-safe initialization of Table data structures.

The use of Tables.synchronizedTable for m_longPollNamespaces and m_notifications ensures thread safety, which is essential for the concurrent nature of this service.

106-112: Correct handling of appId in submit method.

The submit method appropriately handles the appId parameter, updating the Table data structures and initiating long polling if necessary.

Line range hint 115-141:
Efficient management of long polling state with appId.

The startLongPolling method effectively uses the sysAppId to manage the long polling state, ensuring that polling is only initiated once per appId.

Line range hint 172-211:
Proper integration of appId in long polling logic.

The doLongPollingRefresh method correctly incorporates the appId into its URL assembly and notification handling logic, ensuring operations are scoped to the correct application.

223-236: Correct use of appId in notification logic.

The notify method effectively uses the appId to retrieve the appropriate RemoteConfigRepository instances and notify them of changes.

247-260: Appropriate update of notifications with appId.

The updateNotifications method correctly updates the notification IDs for the given appId and namespace, ensuring accurate tracking of changes.

286-287: Correct assembly of namespaces with appId.

The assembleNamespaces method correctly assembles the namespaces for the specified appId, ensuring accurate representation of the data.

TerryLam2010 · 2024-08-14T10:13:39Z

I have changed the Email for submitting code to [email protected], and changed the name to TerryLam2010. Have not submitted open source request merge before,help me please....

nobodyiam

Thank you for your hard work on this significant change. Here are a few comments for your review.

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java

...o-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java

coderabbitai

Actionable comments posted: 4

Outside diff range, codebase verification and nitpick comments (3)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (2)
Update method calls to match the new signature

The onRepositoryChange method is being called with both the new and old signatures across the codebase. Ensure all calls are updated to match the new signature: onRepositoryChange(String appId, String namespace, Properties newProperties), unless there is a specific reason to maintain both versions.

Files with old signature usage:

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderAutoUpdateTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java

Analysis chain

Line range hint 116-136: LGTM! But verify the method usage in the codebase.

The method is correctly implemented. However, ensure that all method calls to onRepositoryChange match the new signature.

The code changes are approved.

Run the following script to verify the method usage:
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all method calls to `onRepositoryChange` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'onRepositoryChange'
Length of output: 63127
113-114: Inconsistent method signature usage detected

The onRepositoryChange method is being called with both the new and old signatures across the codebase. Ensure that all calls to this method are updated to use the new signature, which includes the appId parameter, to prevent potential runtime errors or unexpected behavior.

Files with old signature usage:

JsonConfigFileTest.java

YamlConfigFileTest.java

XmlConfigFileTest.java

DefaultConfigTest.java

PropertiesConfigFileTest.java

JavaConfigPlaceholderAutoUpdateTest.java

Please update these instances to use the new method signature.

Analysis chain

LGTM! But verify the method usage in the codebase.

The method is correctly implemented. However, ensure that all method calls to onRepositoryChange match the new signature.

The code changes are approved.

Run the following script to verify the method usage:
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all method calls to `onRepositoryChange` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'onRepositoryChange'
Length of output: 63127
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (1)
Line range hint 143-172: Fix the getAppId method.

The getAppId method should return the actual appId instead of null.

Apply this diff to fix the getAppId method:
-  return null;
+  return m_appId;

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7e6b56a and ed8ba9b.

Files selected for processing (17)

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (8 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (12 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (5 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (15 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (29 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessorTest.java (6 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java (1 hunks)

Additional comments not posted (58)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java (1)

52-57: LGTM!

The addition of the appId parameter enhances the flexibility of the annotation. The parameter is well-documented with a brief description and version information.

The code changes are approved.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (2)

33-37: LGTM!

The constructor correctly initializes ConfigUtil and ConfigRegistry.

The code changes are approved.

42-46: LGTM!

The changes improve the flexibility of factory retrieval by making it context-aware based on appId. The logic within getFactory(String appId, String namespace) is well-structured.

The code changes are approved.

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1)

38-46: LGTM!

The addition of the getAppId method enhances the interface by providing a standardized way to retrieve the appId. The default implementation ensures backward compatibility.

The code changes are approved.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1)

72-78: LGTM!

The addition of the multipleConfigs method enhances the configurability of the interface by allowing for more flexible application settings related to multiple configurations. The default value ensures backward compatibility.

The code changes are approved.
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (3)
100-127: LGTM! But verify the method usage in the codebase.

The method is correctly implemented. However, ensure that all method calls to setConfig match the new signature.

The code changes are approved.

Run the following script to verify the method usage:

Verification successful

Method Signature Change Verified

The setConfig method calls in the codebase match the new signature. The relevant test file confirms the correct usage. No discrepancies found.

apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java: Correct usage of setConfig.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all method calls to `setConfig` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'setConfig'
Length of output: 10715
82-84: LGTM! But verify the method usage in the codebase.

The method is correctly implemented. However, ensure that all method calls to getConfig match the new signature.

The code changes are approved.

Run the following script to verify the method usage:

111-116: LGTM! But verify the method usage in the codebase.

The method is correctly implemented. However, ensure that all method calls to create match the new signature.

The code changes are approved.

Run the following script to verify the method usage:

Verification successful

Method Implementation and Usage Verified

The create(String appId, String namespace) method is correctly implemented in ConfigService.java and used with the expected signature in the codebase.

Method Definition: Found in apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

Relevant Usage: Found in apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all method calls to `create` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'create'
Length of output: 176808

Script:
#!/bin/bash
# Search for the specific `create(String appId, String namespace)` method implementation in the `ConfigService` class.
ast-grep --lang java --pattern $'class ConfigService {
  $$$
  public Config create(String appId, String namespace) {
    $$$
  }
  $$$  
}'

# Search for the usage of `create(String appId, String namespace)` method across the codebase.
ast-grep --lang java --pattern $'$_.$create($_, $_)' --pattern-insensitive
Length of output: 420

Script:
#!/bin/bash
# Corrected search for the specific `create(String appId, String namespace)` method implementation in the `ConfigService` class.
ast-grep --lang java --pattern $'class ConfigService {
  $$$
  public Config create(String appId, String namespace) {
    $$$
  }
  $$$  
}'

# Corrected search for the usage of `create(String appId, String namespace)` method across the codebase.
ast-grep --lang java --pattern $'$_.$create($_, $_)'
Length of output: 195

Script:
#!/bin/bash
# Search for the `create(String appId, String namespace)` method definition in the `ConfigService` class.
rg --type java 'public Config create$String appId, String namespace$'

# Search for the usage of `create(String appId, String namespace)` method across the codebase.
rg --type java '\.create$[^$]*, [^\)]*\)'
Length of output: 2870
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (2)
57-58: LGTM! But verify the constructor usage in the codebase.

The constructor is correctly implemented. However, ensure that all constructor calls to SimpleConfig match the new signature.

The code changes are approved.

Run the following script to verify the constructor usage:

Verification successful

Constructor usage is consistent with the new signature.

The codebase correctly uses the new constructor signature for SimpleConfig, ensuring that all instances are created with the required parameters: appId, namespace, and configRepository. No instances of the old constructor signature were found.

Ensure that any future changes maintain this consistency.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `SimpleConfig` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'SimpleConfig'
Length of output: 46705
67-70: LGTM! But verify the constructor usage in the codebase.

The constructor is correctly implemented. However, ensure that all constructor calls to SimpleConfig match the new signature.

The code changes are approved.

Run the following script to verify the constructor usage:

Verification successful

Constructor Usage Verified: All instances align with the new signature.

The SimpleConfig constructor is correctly used across the codebase, particularly through the prepareConfig method in tests, ensuring alignment with the new signature.

AbstractSpringIntegrationTest.java: prepareConfig method correctly implements the new constructor signature.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `SimpleConfig` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'SimpleConfig'
Length of output: 46705

Script:
#!/bin/bash
# Description: Locate the implementation of the `prepareConfig` method to verify its correctness.

# Search for the implementation of the `prepareConfig` method in Java files.
rg --type java -A 5 'prepareConfig'
Length of output: 38742
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessorTest.java (4)

58-58: LGTM!

The method is correctly implemented.

The code changes are approved.

86-87: LGTM!

The method is correctly implemented.

The code changes are approved.

116-116: LGTM!

The method is correctly implemented.

The code changes are approved.

161-161: LGTM!

The method is correctly implemented.

The code changes are approved.

apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (6)

45-53: LGTM!

The setUp method correctly initializes someAppId and sets up a mock ConfigUtil.

The code changes are approved.

68-72: LGTM!

The testHackConfig method correctly tests the configuration with the new appId parameter.

The code changes are approved.

83-83: LGTM!

The testHackConfigFactory method correctly tests the configuration factory with the new appId parameter.

The code changes are approved.

95-95: LGTM!

The testMockConfigFactory method correctly tests the mock configuration factory with the new appId parameter.

The code changes are approved.

104-104: LGTM!

The testMockConfigFactoryForConfigFile method correctly tests the mock configuration factory for config files with the new appId parameter.

The code changes are approved.

Line range hint 113-127: LGTM!

The MockConfig class correctly integrates the appId parameter and updates the getProperty method to reflect this change.

The code changes are approved.

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (5)

59-59: LGTM!

The static field APP_NAMESPACE_NAMES correctly represents the namespaces associated with specific application IDs.

The code changes are approved.

69-76: LGTM!

The addNamespaces method correctly updates the namespace management to accommodate multiple application IDs.

The code changes are approved.

99-121: LGTM!

The initializePropertySources method correctly updates the property source initialization to accommodate multiple application IDs.

The code changes are approved.

173-173: LGTM!

The reset method correctly updates the reset logic to accommodate the new data structure.

The code changes are approved.

78-78: LGTM!

The postProcessBeanFactory method correctly updates the bean factory post-processing to accommodate the new data structure and configuration logic.

The code changes are approved.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (7)

68-68: LGTM!

The static field CONFIG_REGISTRY correctly represents the configurations associated with specific application IDs and namespaces.

The code changes are approved.

94-101: LGTM!

The prepareConfig method correctly updates the configuration preparation to accommodate the new appId parameter.

The code changes are approved.

122-130: LGTM!

The prepareYamlConfigFile method correctly updates the YAML configuration file preparation to accommodate the new appId parameter.

The code changes are approved.

170-171: LGTM!

The mockConfig method correctly updates the configuration mocking to accommodate the new appId parameter.

The code changes are approved.

178-180: LGTM!

The mockConfigFile method correctly updates the configuration file mocking to accommodate the new appId parameter.

The code changes are approved.

Line range hint 182-201: LGTM!

The doSetUp method correctly updates the setup logic to accommodate the new appId parameter and configuration logic.

The code changes are approved.

219-238: LGTM!

The MockConfigManager class correctly updates the configuration management to accommodate the new appId parameter.

The code changes are approved.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (6)

70-71: LGTM!

The constructor correctly calls another constructor with the appId parameter.

The code changes are approved.

80-85: LGTM!

The constructor correctly initializes the class variables with the appId parameter.

The code changes are approved.

239-240: LGTM!

The method correctly calls the overloaded method with the appId parameter.

The code changes are approved.

Line range hint 243-260: LGTM!

The method correctly handles the repository change with the appId parameter.

The code changes are approved.

273-273: LGTM!

The method correctly calculates and updates the configuration changes with the appId parameter.

The code changes are approved.

Line range hint 324-336: LGTM!

The method correctly loads properties from the resource file with the appId and namespace.

The code changes are approved.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (6)

93-96: LGTM!

The constructor correctly initializes the class variables with ConcurrentHashMap and Table.

The code changes are approved.

106-112: LGTM!

The method correctly handles the submission with the appId parameter.

The code changes are approved.

Line range hint 115-145: LGTM!

The method correctly handles the long polling start with the sysAppId parameter.

The code changes are approved.

Line range hint 176-215: LGTM!

The method correctly handles the long polling refresh with the appId parameter.

The code changes are approved.

227-240: LGTM!

The method correctly handles the notifications with the appId parameter.

The code changes are approved.

251-264: LGTM!

The method correctly handles the notification updates with the appId parameter.

The code changes are approved.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (6)

Line range hint 106-135: LGTM!

The method correctly initializes the test setup with the appId parameter.

The code changes are approved.

155-155: LGTM!

The test method correctly tests the configuration loading with the appId parameter.

The code changes are approved.

181-181: LGTM!

The test method correctly tests the configuration loading with ordered properties and the appId parameter.

The code changes are approved.

218-218: LGTM!

The test method correctly tests the configuration loading with access key secret and the appId parameter.

The code changes are approved.

231-231: LGTM!

The test method correctly tests the exception handling with the appId parameter.

The code changes are approved.

Line range hint 261-272: LGTM!

The test method correctly tests the repository change listener with the appId parameter.

The code changes are approved.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (7)

87-91: LGTM!

The setUp method correctly initializes the test environment with someAppId and sets the system property for APP_ID.

The code changes are approved.

149-149: LGTM!

The test correctly includes someAppId in the submit method call and verifies the URL contains the correct appId.

The code changes are approved.

200-200: LGTM!

The test correctly includes someAppId in the submit method call and verifies the correct processing of notifications.

The code changes are approved.

262-263: LGTM!

The test correctly includes someAppId in the submit method call and verifies the request headers contain the necessary authentication information.

The code changes are approved.

333-336: LGTM!

The test correctly includes someAppId in the submit method calls for multiple namespaces and verifies the correct processing of notifications for each namespace.

The code changes are approved.

404-405: LGTM!

The test correctly includes someAppId in the submit method calls for multiple namespaces with multiple notifications and verifies the correct processing of notifications for each namespace.

The code changes are approved.

465-465: LGTM!

The test correctly includes someAppId in the submit method call and verifies the correct processing and updating of notifications.

The code changes are approved.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (1)

660-684: LGTM!

The test correctly handles multiple configurations with multiple app IDs and verifies the correct configurations are returned for each app ID and namespace.

The code changes are approved.

apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

nobodyiam · 2024-08-31T09:04:47Z

I have changed the Email for submitting code to [email protected], and changed the name to TerryLam2010. Have not submitted open source request merge before,help me please....

emm...You may first backup the local repo and try the following:

1. git checkout main
2. git checkout -b 20240703-apollo-multiple-appid-new
3. git merge --squash 20240703-apollo-multiple-appid
4. git commit -m "some commit message"
5. git push origin 20240703-apollo-multiple-appid-new:20240703-apollo-multiple-appid

nobodyiam

This looks significantly better now. Please see some comments below:

...ig-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

.../src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java

coderabbitai

Actionable comments posted: 13

Outside diff range, codebase verification and nitpick comments (10)

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (4)

21-30: Enhance Annotation Documentation

Consider adding more details about how MultipleConfig interacts with @EnableApolloConfig to provide clearer guidance for developers.

32-36: Enhance appId Method Documentation

The documentation for the appId method should include more details about what constitutes an "appId" and how it should be formatted or used.

38-42: Enhance namespaces Method Documentation

The documentation for the namespaces method should include more details about the purpose of namespaces and examples of typical values.

44-50: Enhance secret Method Documentation

The documentation for the secret method should include more details about the significance of the secret, its typical use cases, and implications of not configuring it.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java (1)

47-53: Approved new register method with a suggestion.

The method correctly handles potential conflicts by logging a warning. Consider adding more detailed logging or metrics to track how often conflicts occur, which could be useful for monitoring and debugging.
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (1)
Line range hint 48-73: Implementation of getFactory(String appId, String namespace):

The method's implementation is robust, covering various scenarios:

It first checks for a "hacked" factory specific to the app ID and namespace.

It checks the cache implemented using HashBasedTable.

It attempts to retrieve a declared config factory.

It falls back to a default config factory if none are found.

The method ensures that the factory is never null by the end, which is crucial for avoiding null pointer exceptions. However, consider adding explicit logging or an exception throw if the factory is still null after all checks, to improve error handling and debuggability.

Consider enhancing error handling by adding logging or throwing an exception if no factory is found:
if (factory == null) {
    logger.error("No ConfigFactory found for appId: {}, namespace: {}", appId, namespace);
    throw new IllegalStateException("No ConfigFactory found for appId: " + appId + ", namespace: " + namespace);
}
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (1)

Line range hint 119-139: Approved: Handling of appId in onRepositoryChange.

The method's logic to handle configuration changes based on the specified appId is correctly implemented. However, consider adding more detailed logging for debugging purposes, especially to trace how different appIds affect the configuration changes.
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (1)
68-68: Inconsistent use of appId across test methods.

The testHackConfig method uses the someAppId parameter, but other methods like testHackConfigFactory, testMockConfigFactory, and testMockConfigFactoryForConfigFile do not. This inconsistency may lead to incomplete test coverage if the goal is to validate behavior across different app IDs. Consider reviewing these methods to ensure they align with the intended test scenarios.

testHackConfig uses someAppId.

testHackConfigFactory, testMockConfigFactory, and testMockConfigFactoryForConfigFile do not use someAppId.

Analysis chain

Ensure consistency in the use of appId across test methods.

The addition of appId in the test methods testHackConfig, testHackConfigFactory, and testMockConfigFactory is consistent with the PR's objective to handle multiple app IDs. However, ensure that all related test methods and classes are updated accordingly to maintain consistency across the entire test suite.

Also applies to: 72-72, 83-83, 95-95
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all test methods in ConfigServiceTest.java use the `appId` parameter consistently.

# Test: Search for test methods that might not be using the `appId` parameter.
rg --type java -A 5 $'public void test' apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java
Length of output: 1217
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (2)
Line range hint 128-154: Ensure robust error handling in startLongPolling.

The method startLongPolling has been adapted to handle app-specific long polling. The use of putIfAbsent for m_longPollStarted is appropriate for concurrency control. However, the error handling could be improved by ensuring that any exceptions caught are properly logged and handled to prevent the long polling service from terminating unexpectedly.

Consider adding more detailed logging and a more robust exception handling strategy:
catch (Throwable ex) {
  m_longPollStarted.remove(sysAppId);
  ApolloConfigException exception = new ApolloConfigException("Schedule long polling refresh failed", ex);
+ logger.error("Failed to schedule long polling for appId: {}", sysAppId, ex);
  Tracer.logError(exception);
  logger.warn(ExceptionUtil.getDetailMessage(exception));
}
This enhancement ensures that all exceptions are adequately logged, providing better visibility into issues that occur during the scheduling of long polling.

Line range hint 185-206: Optimize the long polling refresh logic.

The doLongPollingRefresh method handles the core functionality of long polling. The method is well-structured to handle different scenarios and responses. However, the handling of the 304 status code could be improved to ensure that the service descriptor (lastServiceDto) is reset more reliably.

Consider refining the condition for resetting lastServiceDto:
if (response.getStatusCode() == 304 && ThreadLocalRandom.current().nextBoolean()) {
- lastServiceDto = null;
+ if (shouldResetServiceDescriptor()) {
+   lastServiceDto = null;
+ }
}
Introducing a method shouldResetServiceDescriptor allows for more controlled and testable conditions for when the service descriptor should be reset, improving the reliability of the long polling mechanism.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ed8ba9b and 132e2b7.

Files selected for processing (22)

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (7 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (8 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PlainTextConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (12 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java (3 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java (3 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java (2 hunks)

Additional comments not posted (69)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PlainTextConfigFile.java (1)
27-28: Constructor Modification: Addition of appId Parameter

The constructor now includes an appId parameter, which is a significant change aligning with the project's goal to support multiple app IDs. This change is correctly implemented as the appId is passed to the superclass constructor, ensuring that the AbstractConfigFile can utilize this new parameter.

However, it's crucial to verify that all existing usages of this constructor in the codebase have been updated to include the new appId parameter to prevent runtime errors.

Verification successful

Constructor Change Verified: appId Parameter Propagation

The addition of the appId parameter to the PlainTextConfigFile constructor has been correctly propagated to all its subclasses (XmlConfigFile, TxtConfigFile, JsonConfigFile, YamlConfigFile). Each subclass constructor includes the appId parameter, ensuring consistency and preventing potential runtime errors.

XmlConfigFile: public XmlConfigFile(String appId, String namespace, ...)

TxtConfigFile: public TxtConfigFile(String appId, String namespace, ConfigRepository configRepository)

JsonConfigFile: public JsonConfigFile(String appId, String namespace, ...)

YamlConfigFile: public YamlConfigFile(String appId, String namespace, ConfigRepository configRepository)
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `PlainTextConfigFile` include the new `appId` parameter.

# Test: Search for the constructor usage. Expect: Only occurrences with three parameters.
rg --type java -A 5 $'new PlainTextConfigFile'
Length of output: 47

Script:
#!/bin/bash
# Description: Search for all usages of the `PlainTextConfigFile` class to verify constructor usage.

# Test: Search for any references to `PlainTextConfigFile` in the codebase.
rg --type java 'PlainTextConfigFile'
Length of output: 995

Script:
#!/bin/bash
# Description: Search for constructors in subclasses of `PlainTextConfigFile` to verify if `appId` is passed correctly.

# Test: Search for constructors in subclasses of `PlainTextConfigFile`.
rg --type java -A 5 $'class XmlConfigFile {\n  public XmlConfigFile'
rg --type java -A 5 $'class TxtConfigFile {\n  public TxtConfigFile'
rg --type java -A 5 $'class JsonConfigFile {\n  public JsonConfigFile'
rg --type java -A 5 $'class YamlConfigFile {\n  public YamlConfigFile'
Length of output: 1443

Script:
#!/bin/bash
# Description: Search for constructors in subclasses of `PlainTextConfigFile` to verify if `appId` is passed correctly.

# Test: Search for constructors in subclasses of `PlainTextConfigFile`.
rg --type java 'public XmlConfigFile'
rg --type java 'public TxtConfigFile'
rg --type java 'public JsonConfigFile'
rg --type java 'public YamlConfigFile'
Length of output: 795
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1)

1-17: Header and Package Declaration Approved

The file header, including copyright and licensing information, and the package declaration are correctly formatted.
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java (2)
52-58: Approved: Addition of appId attribute to ApolloConfig.

The addition of the appId attribute aligns with the PR objectives and enhances configuration flexibility. The documentation is clear and specifies the version introduction. This change is consistent with the suggestions from previous reviews.

52-58: Verify the integration of the new appId attribute across the codebase.

Ensure that the new appId attribute is correctly integrated and utilized in relevant parts of the codebase.

Run the following script to verify the usage of the appId attribute:

Verification successful

The appId attribute is correctly integrated across the codebase.

The appId attribute in the ApolloConfig annotation is utilized in the ApolloAnnotationProcessor class, which processes the annotation, indicating its integration into the application logic. Additionally, its presence in various DTOs and test files confirms its usage and testing. No further action is required.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the usage of the `appId` attribute in the ApolloConfig annotation across the codebase.

# Test: Search for the usage of the `appId` attribute. Expect: Proper integration and usage.
rg --type java -A 5 $'@ApolloConfig\$.*appId.*\$'
Length of output: 851

Script:
#!/bin/bash
# Description: Verify the usage of the `appId` attribute in the main codebase.

# Test: Search for the usage of the `appId` attribute in the main codebase. Expect: Proper integration and usage.
rg --type java -A 5 'appId\s*='
Length of output: 38709
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (2)

34-40: Review of the new getConfig method with appId support.

The addition of the getConfig(String appId, String namespace) method is well-documented and aligns with the PR's objective to support multiple app IDs. The method signature and JavaDoc are clear and concise.

Correctness: The method signature correctly takes appId and namespace as parameters, which is consistent with the PR's description.

Documentation: The JavaDoc is appropriately detailed, providing clear information on what the method does and its parameters.

50-57: Review of the new getConfigFile method with appId support.

The addition of the getConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) method extends the functionality to fetch configuration files for specific app IDs and namespaces. This method is crucial for environments where multiple applications might share configurations but need distinct setups.

Correctness: The method signature is correct and includes all necessary parameters (appId, namespace, configFileFormat).

Documentation: The JavaDoc is well-written, clearly explaining the purpose of the method and the parameters it accepts.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java (4)

19-23: Approved import changes.

The new imports are necessary for handling the new appId functionality and concurrency control with Table.

32-37: Approved class setup and instance variables.

The use of ConfigUtil to fetch the appId and a synchronized table for storing ConfigFactory instances ensures thread safety and supports the new functionality.

44-45: Approved method delegation for backward compatibility.

The register method now correctly delegates to the new method accepting appId, ensuring backward compatibility.

58-63: Approved method changes for getFactory.

The changes to getFactory methods are consistent with the new functionality and maintain backward compatibility by using the default appId.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (3)

19-20: Review of Imports:

The addition of HashBasedTable and Table from Guava and the import of ConfigUtil are appropriate given the changes to support multiple app IDs. Ensure that Maps is still used elsewhere in the project, or consider removing it if it's no longer needed.

Also applies to: 24-24

33-35: Initialization of New Members:

The introduction of m_factories as a HashBasedTable and m_configUtil is well-aligned with the new requirements for handling multiple app IDs. The instantiation of m_configUtil in the constructor ensures that the application ID can be dynamically retrieved, which is crucial for the new logic.

Also applies to: 39-39

44-45: Refactor of getFactory(String namespace):

The method now delegates to the new getFactory(String appId, String namespace) by fetching the app ID using m_configUtil. This is a clean and efficient way to integrate the new functionality without duplicating code.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (3)

28-28: Field addition approved: m_appId.

The addition of the m_appId field is correctly implemented as a private final String, ensuring immutability and thread safety.

36-38: Constructor modification approved.

The updated constructor now correctly includes the appId parameter and initializes the m_appId field along with m_namespace and m_changes. This change supports the new functionality of handling multiple app IDs effectively.

78-84: New method addition approved: getAppId().

The addition of the getAppId() method is correctly implemented to provide external access to the m_appId field, adhering to Java's encapsulation principles.

apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (3)

27-27: Field addition approved.

The addition of the appId field is crucial for supporting multiple app IDs and is correctly declared as private final for immutability.

43-45: Constructor modification approved.

The modification to include appId as the first parameter in the constructor is logical and necessary for initializing the new field correctly.

80-82: Getter method addition approved.

The addition of the getAppId method is essential for providing external access to the appId field, enhancing the class's functionality.
apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (1)
42-52: Well-implemented new constructor with appId.

The addition of the appId parameter in the constructor enhances the configuration management capabilities as intended. The constructor is well-documented and correctly initializes the superclass with the new parameter.

Please ensure that all integrations and usages of this new constructor are updated across the codebase to reflect the new parameter. This can be verified by searching for instances of PureApolloConfig instantiation and checking if they now include the appId parameter.

Verification successful

New constructor usage verified in PureApolloConfigFactory.

The instantiation of PureApolloConfig in PureApolloConfigFactory.java correctly uses the new constructor with the appId parameter. This confirms that the integration of the new constructor is consistent with its intended usage.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all instances of `PureApolloConfig` instantiation include the new `appId` parameter.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new PureApolloConfig'
Length of output: 473
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java (5)

27-31: Addition of new imports for enhanced configuration management.

The addition of ConfigUtil and Table from Guava supports the new functionality for handling multiple app IDs. This aligns with the PR's objective to enhance configuration capabilities.

39-46: Initialization of new data structures for configuration management.

The use of Table for m_configs and m_configFiles is a significant change that allows storing configurations with a compound key of appId and namespace. This is a robust approach to handle configurations in a multi-application environment.

52-52: Initialization of ConfigUtil in the constructor.

This change is crucial as ConfigUtil is used to fetch the appId dynamically in the overridden methods. It ensures that the appId can be retrieved consistently across different parts of the application.

57-58: Delegation of getConfig to the new method with appId.

This change maintains backward compatibility by delegating to the new getConfig method that includes appId. It's a good practice to ensure that existing code relying on the old method signature continues to work without modification.

82-83: Delegation of getConfigFile to the new method with appId.

Similar to getConfig, this change ensures backward compatibility by delegating to the new getConfigFile method that includes appId. This is a good practice for maintaining existing functionality while extending new features.

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (3)

82-84: New method for fetching configuration by appId and namespace.

The implementation of getConfig(String appId, String namespace) correctly extends the functionality to handle multiple app IDs, aligning with the PR's objectives.

110-116: Validation of appId in configuration creation.

The addition of appId validation in create(String appId, String namespace) ensures that configurations are correctly associated with their respective applications. This is a critical enhancement for maintaining configuration integrity.

22-22: Import of StringUtils.

The addition of StringUtils is necessary for the appId comparison logic in the new methods. This is a justified and appropriate use of the library.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (4)

42-42: Approved: Introduction of m_appId and related changes.

The addition of m_appId and its integration into the constructors and methods aligns with the PR's objective to handle multiple app IDs effectively.

Also applies to: 67-71

57-58: Approved: Constructor delegation for backward compatibility.

The updated constructor that delegates to the new constructor with appId as null ensures backward compatibility while supporting the new functionality.

116-117: Approved: Overloading of onRepositoryChange to handle appId.

The method now correctly delegates handling to the version that accepts an appId, aligning with the new functionality requirements.

68-70: Approved: Dynamic fetching of appId using ConfigUtil.

The approach to dynamically fetch the appId if not provided ensures robustness in handling configurations. Ensure that ConfigUtil is always correctly initialized and available to avoid potential null pointer exceptions.

apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java (8)

102-104: Approved: New method signature for Config create.

The addition of the appId parameter aligns with the PR's objectives to support multiple app IDs. The implementation is consistent with the interface's requirements.

112-114: Approved: New method signature for ConfigFile createConfigFile.

The addition of the appId parameter enhances the method's capability to handle configurations specific to an application ID, which is crucial for multi-application environments.

124-126: Approved: New method signature for void register.

The updated method signature allows for more granular control over configuration factories based on both application ID and namespace, supporting the PR's goal of enhanced configuration management.

137-142: Approved: New method signature for ConfigFactory getFactory.

This method now supports fetching factories based on both appId and namespace, enhancing the flexibility and specificity of configuration retrieval.

152-154: Approved: Consistent implementation of Config create across different factories.

The method's implementation is aligned with the interface's new requirements, supporting configurations specific to both an application ID and a namespace.

162-164: Approved: Consistent implementation of ConfigFile createConfigFile across different factories.

This method now allows for the creation of configuration files that are specific to both an application ID and a namespace, enhancing the system's configurability.

174-176: Approved: Uniform implementation of Config create across all factories.

The method's implementation is consistent with the new interface requirements, ensuring that configurations can be tailored to specific applications and namespaces.

184-186: Approved: Uniform implementation of ConfigFile createConfigFile across all factories.

This method enhances the configurability of the system by allowing for the creation of configuration files that are specific to both an application ID and a namespace.
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (3)
45-45: Initialization of ConfigUtil.

The direct instantiation of ConfigUtil using ApolloInjector ensures that the configuration utility is properly integrated and managed within the Apollo infrastructure. This is a good use of dependency injection principles.

74-77: Dynamic addition of namespaces to PropertySourcesProcessor.

This loop effectively adds namespaces for each app ID, ensuring that configurations are dynamically loaded based on the app ID. The use of Lists.newArrayList is appropriate here for converting arrays to lists.

Line range hint 19-28: Review of new imports and dependencies.

The addition of ConfigUtil and other imports are necessary for the new functionalities introduced. Ensure that these dependencies are properly managed and do not introduce any version conflicts or additional overhead that could affect the application's performance.

Verification successful

No Dependency Conflicts with ConfigUtil Import
The import of ConfigUtil in DefaultApolloConfigRegistrarHelper.java is consistent with its usage as an internal utility class within the Apollo project. It is managed by the ApolloInjector and does not introduce any external dependency conflicts.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for potential dependency conflicts.

# Test: Search for dependency declarations in build files. Expect: No conflicts with existing dependencies.
fd 'pom.xml' --exec cat {} \; | grep 'ConfigUtil'
Length of output: 50

Script:
#!/bin/bash
# Search for the definition or declaration of ConfigUtil in the codebase
ast-grep --lang java --pattern 'class ConfigUtil { $$$ }'

# Check if ConfigUtil is part of a library that is already included as a dependency
rg 'ConfigUtil' -A 5
Length of output: 116958
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (3)

116-117: Review the constructor and method changes in MockConfig.

The constructor of MockConfig has been correctly updated to include appId, aligning with the changes required for supporting multiple app IDs. The method getProperty now correctly formats the return value to include appId, which is crucial for ensuring that the configuration keys are correctly formed and tested.

Also applies to: 127-127

152-157: Review the constructor changes in MockConfigFile.

The updated constructor in MockConfigFile now accepts appId along with namespace and configFileFormat, which is essential for testing configurations that are specific to an application ID. This change is well-aligned with the PR's objectives and enhances the specificity of the configuration tests.

206-208: Review the method changes in MockConfigFactory.

The methods create and createConfigFile in MockConfigFactory have been updated to include an appId parameter. This modification is crucial for creating configurations that are specific to an application ID, thus supporting the new functionality introduced in the PR. The implementation appears correct and aligns with the changes described in the PR summary.

Also applies to: 216-218

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (5)

59-59: Refactor: Use of Multimap for Namespace Management

The change to use a Multimap for managing namespaces associated with multiple app IDs is a significant improvement. This structure allows for more flexible and scalable configuration management, especially in environments with multiple applications.

69-70: Refactor: Static Method Overload for Backward Compatibility

The addition of an overloaded addNamespaces method that defaults to using the current app ID is a good practice for maintaining backward compatibility. This allows existing code to function without modification while supporting new functionality.

72-75: Enhancement: Support for Multiple App IDs in Namespace Management

This method enhancement to handle multiple app IDs is aligned with the PR's objectives. It correctly creates a new multimap for each app ID and associates it with the given namespaces, which is crucial for the new functionality.

120-120: Cleanup Operation on APP_NAMESPACE_NAMES

Clearing APP_NAMESPACE_NAMES after initialization is a critical operation to ensure that the state does not persist beyond its intended scope. This is a good practice to prevent potential memory leaks or stale data issues.

172-172: Reset Method Enhancement

The update to the reset method to clear both APP_NAMESPACE_NAMES and AUTO_UPDATE_INITIALIZED_BEAN_FACTORIES is necessary to ensure that all stateful components are correctly reset during testing or reinitialization scenarios.

apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java (2)

226-226: Approved: Updated test method reflects changes correctly.

The test method testLocalCacheDirWithSystemProperty correctly verifies the behavior of getDefaultLocalCacheDir(someAppId) by ensuring the appId is appended to the cache directory path.

239-243: Approved: Test method accurately reflects OS-specific behavior with appId.

The test method testDefaultLocalCacheDir effectively verifies the behavior of getDefaultLocalCacheDir(someAppId) for both Windows and non-Windows systems, ensuring that the appId is correctly incorporated into the path.

apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java (1)

20-20: Approved: Import and system property setting.

The import of ApolloClientSystemConsts and the dynamic setting of the APP_ID system property are crucial for supporting multiple app IDs. This aligns with the PR's objectives to enhance configuration capabilities.

Ensure that the new system property APP_ID is utilized consistently across the test suite where applicable.

Also applies to: 121-121

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (7)

52-52: Initialization of m_appId

The addition of m_appId as a class member is consistent with the PR's objective to handle multiple app IDs. This change allows the class to manage configurations based on the application identifier effectively.

66-71: Constructor Overloading with appId

The constructors have been overloaded to include appId, which is essential for distinguishing configurations across different applications. This change aligns well with the PR's objectives and enhances the configurability of the system.

88-88: Dynamic Local Cache Directory Based on appId

The method findLocalCacheDir now dynamically determines the cache directory based on appId. This is a crucial enhancement for supporting environments with multiple applications, ensuring that each application's configuration is isolated and managed independently.

133-144: Handling Repository Changes with appId

The method onRepositoryChange has been updated to include appId. This ensures that changes are correctly associated with the specific application, which is vital for accurate configuration management in multi-application environments.

160-160: Loading Properties from Local Cache File

The method loadFromLocalCacheFile now includes appId in its parameters, which is used to construct the cache file name. This modification is necessary to ensure that the cache files are uniquely identified per application, preventing configuration conflicts.

201-201: Persisting Local Cache File with appId

The method persistLocalCacheFile has been updated to include appId, enhancing the specificity of cache file management. This change is crucial for maintaining the integrity and isolation of configuration data across multiple applications.

Also applies to: 239-239

297-300: Assembling Local Cache File Name with appId

The method assembleLocalCacheFile now constructs the file name using appId, which is a necessary change to support unique file naming conventions across different applications. This ensures that configurations are not only isolated by namespace but also by application identifier.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (5)

68-69: Refactor suggestion: Constructor delegation.

The constructor at lines 68-69 delegates to another constructor using null for appId. This is a good use of constructor chaining to maintain a clean and DRY codebase.

78-84: Review: New constructor with appId.

The new constructor introduced at lines 78-84 correctly handles the case where appId is null by fetching the default appId from ConfigUtil. This ensures backward compatibility and flexibility in configuration management.

240-241: Refactor suggestion: Method overloading for backward compatibility.

The method onRepositoryChange at lines 240-241 is overloaded to maintain backward compatibility while supporting the new appId parameter. This is a good practice as it allows existing code to function without modification while enabling new functionality.

Line range hint 243-261: Review: Handling of configuration changes with appId.

The method onRepositoryChange at lines 243-261 now includes handling for the appId. This change is crucial for supporting configurations specific to different applications, enhancing the flexibility of the configuration management system.

274-274: Review: Calculation of property changes.

The method updateAndCalcConfigChanges at line 274 now includes appId in the calculation of property changes. This is an important update as it ensures that changes are accurately tracked and managed per application, aligning with the PR's objectives to support multiple app IDs.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (2)

78-80: Refactor to use ConcurrentHashMap and Table for tracking and notifications.

The transition to using ConcurrentHashMap for m_longPollStarted and Table for m_notifications is a significant improvement. This change enhances the granularity of control and allows for app-specific tracking, which is crucial for the feature of handling multiple app IDs. The use of ConcurrentHashMap ensures thread safety and better performance under concurrent access scenarios.

299-300: Simplify namespace assembly for app-specific configurations.

The assembleNamespaces method has been updated to handle app-specific configurations. The method now correctly assembles namespaces based on the app ID. This change is a straightforward adaptation to support the new multi-app functionality and is implemented correctly.
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (1)
524-526: Review: getAccessKeySecret(String appId) method implementation.

The method getAccessKeySecret(String appId) retrieves the access key secret for a specific appId using the Foundation class. This is a straightforward implementation that delegates the responsibility to the Foundation.app().getAccessKeySecret(appId) method.

Given the previous comments about integrating this logic into Foundation.app(), it appears this change aligns with those suggestions. Ensure that the Foundation class has been appropriately updated to handle this new method call.

Verification successful

Clarification on getAccessKeySecret Method Implementation

The getAccessKeySecret method is not directly implemented in the Foundation class. Instead, it is part of the ApplicationProvider interface and implemented by classes like DefaultApplicationProvider. Ensure that Foundation.app() correctly delegates to an appropriate provider that implements this method.

DefaultApplicationProvider implements getAccessKeySecret(String appId).

ApplicationProvider interface declares getAccessKeySecret.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that the Foundation class has been updated to handle the new method call.

# Test: Search for the updated Foundation class implementation.
ast-grep --lang java --pattern $'class Foundation {
  $$$
  getAccessKeySecret($_) {
    $$$
  }
  $$$  
}'
Length of output: 108

Script:
#!/bin/bash
# Locate the Foundation class in the codebase
fd -e java -x grep -l 'class Foundation' {}

# Search for the getAccessKeySecret method in the codebase
rg 'getAccessKeySecret' -A 3 --glob '*.java'
Length of output: 8689

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java

apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java

.../src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java

apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java

apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java

coderabbitai

Actionable comments posted: 18

Outside diff range, codebase verification and nitpick comments (6)

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RepositoryChangeListener.java (1)
32-38: Review of the new method addition in RepositoryChangeListener.

The addition of the new method onRepositoryChange(String appId, String namespace, Properties newProperties) is well-documented and aligns with the PR's objectives to enhance configuration management by supporting multiple app IDs. This method allows for more granular control and differentiation of repository changes per application, which is crucial in multi-application environments.

However, it's important to ensure that this change does not break existing implementations. Since this is an interface, adding a new method could break classes that implement this interface unless they are updated accordingly.

Consider providing a default implementation for the new method in the interface to maintain backward compatibility. This can be done using Java 8's default methods in interfaces. Here's a suggested implementation:
default void onRepositoryChange(String appId, String namespace, Properties newProperties) {
    // Default implementation to avoid breaking existing implementations
    onRepositoryChange(namespace, newProperties);
}
This default method simply redirects to the existing onRepositoryChange(String namespace, Properties newProperties) method, ensuring that existing implementations that do not yet handle appId can continue to function without modification.
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1)

38-46: Approve the addition of the getAppId() method.

The addition of the getAppId() method as a default method in the ConfigFile interface is a good practice for interface evolution. It allows the interface to be extended without breaking existing implementations. The method is well-documented and includes a version annotation, which is helpful for maintainers.

However, it's important to ensure that implementations that require a specific appId should override this method to return the correct value instead of null.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEventTest.java (1)

74-77: Approved: Use of someAppId in method calls.

The changes to include someAppId in the ConfigChange constructor and config.fireConfigChange method call are well-implemented. This ensures consistent application ID usage across configuration changes.

Consider adding a comment explaining the purpose of someAppId for future maintainability.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (1)
Line range hint 243-261: Enhancement: Extended onRepositoryChange to support appId.

This method extension is crucial for handling configuration changes in a multi-app environment. The method correctly checks for changes before proceeding, which is efficient. However, consider adding more detailed logging to help trace the flow of changes, especially in complex environments.
+ logger.debug("Processing repository change for appId: {}, namespace: {}", appId, namespace);
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (1)
Line range hint 128-154: Review the startLongPolling method with app-specific logic.

The method has been updated to handle app-specific long polling initiation. The use of putIfAbsent to check and set the m_longPollStarted flag is efficient and prevents redundant polling initialization. However, the exception handling within the method could be improved by logging more specific details about the failure context, which would aid in debugging.

Consider enhancing the logging detail in the exception handling:
ApolloConfigException exception =
    new ApolloConfigException("Schedule long polling refresh failed for appId: " + sysAppId, ex);
This addition provides clearer context in the logs regarding which app ID encountered the scheduling issue.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (1)

106-108: Cleanup: Remove commented-out code.

The commented-out code related to PropertySourcesProcessor is unnecessary and clutters the codebase. Removing this code will improve readability and maintainability.

Consider removing the commented-out code unless there is a specific reason to keep it for future reference.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 132e2b7 and 38469c4.

Files selected for processing (82)

CHANGES.md (1 hunks)
apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java (1 hunks)
apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfigFactory.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfig.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigFile.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigRepository.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (7 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEvent.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/JsonConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java (8 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PlainTextConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepository.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (12 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigRepository.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RepositoryChangeListener.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java (4 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/TxtConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/XmlConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YamlConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YmlConfigFile.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (2 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigFileChangeEvent.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactory.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactoryManager.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigRegistry.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java (3 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java (1 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java (5 hunks)
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java (3 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/build/MockInjector.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigIntegrationTest.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/AbstractConfigTest.java (4 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java (4 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java (30 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEventTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/JsonConfigFileTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepositoryTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepositoryTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesConfigFileTest.java (9 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (15 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/SimpleConfigTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/TxtConfigFileTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/XmlConfigFileTest.java (10 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/YamlConfigFileTest.java (10 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryFileCachePropertyTest.java (2 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java (3 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (7 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistryTest.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (8 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java (11 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (29 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java (40 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java (20 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XMLConfigAnnotationTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderAutoUpdateTest.java (25 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderTest.java (5 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/CachedCompositePropertySourceTest.java (1 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessorTest.java (6 hunks)
apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java (2 hunks)
apollo-core/src/main/java/com/ctrip/framework/apollo/core/utils/StringUtils.java (1 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/DefaultApplicationProvider.java (2 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/NullProvider.java (1 hunks)
apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java (1 hunks)
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java (2 hunks)
apollo-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java (1 hunks)
changes/changes-2.3.0.md (1 hunks)
pom.xml (1 hunks)

Files skipped from review due to trivial changes (4)

CHANGES.md
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloConfig.java
apollo-client/src/test/java/com/ctrip/framework/apollo/build/MockInjector.java
pom.xml

Files skipped from review as they are similar to previous changes (16)

apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfig.java
apollo-client/src/main/java/com/ctrip/framework/apollo/ConfigService.java
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/ConfigManager.java
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfigManager.java
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepository.java
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PlainTextConfigFile.java
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChange.java
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManager.java
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistry.java
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/MultipleConfig.java
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/spi/DefaultApolloConfigRegistrarHelper.java
apollo-client/src/main/java/com/ctrip/framework/apollo/util/ConfigUtil.java
apollo-client/src/test/java/com/ctrip/framework/apollo/BaseIntegrationTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/ConfigServiceTest.java
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryManagerTest.java

Additional comments not posted (237)

changes/changes-2.3.0.md (2)

7-13: Review of Change Entries

The entries from line 7 to 13 are well-documented and each includes a direct link to the corresponding pull request, which enhances transparency and traceability. The descriptions are clear and concise, providing a quick overview of the changes made in version 2.3.0.

16-16: Review of Comprehensive Link

The link provided on line 16 to view all issues and pull requests associated with this release is correctly formatted and functional. This is a valuable resource for users looking for a comprehensive view of the development process for version 2.3.0.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/TxtConfigFile.java (2)
Line range hint 27-29: Method getConfigFileFormat remains unchanged.

The method getConfigFileFormat which returns ConfigFileFormat.TXT is unchanged and continues to function as expected. This method is not directly affected by the constructor change.

23-24: Constructor updated to include appId.

The constructor of TxtConfigFile now includes an additional parameter appId, which is passed to the superclass PlainTextConfigFile. This change aligns with the PR's objective to support multiple app IDs and enhances configuration management capabilities. However, it is crucial to ensure that all instantiations of TxtConfigFile across the codebase are updated to reflect this new parameter.

Verification successful

All instantiations of TxtConfigFile updated successfully.

The constructor change to include the appId parameter in TxtConfigFile has been successfully propagated across the codebase. Both the production and test code reflect the new constructor signature, ensuring consistency and alignment with the PR's objectives.

DefaultConfigFactory.java: Updated to use the new constructor signature.

TxtConfigFileTest.java: Updated to use the new constructor signature.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all instantiations of `TxtConfigFile` are updated to include the new `appId` parameter.

# Test: Search for instantiations of `TxtConfigFile`. Expect: Only occurrences with the new constructor signature.
rg --type java -A 5 $'new TxtConfigFile'
Length of output: 1503
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YmlConfigFile.java (2)
25-26: Constructor signature change is well-implemented.

The addition of the appId parameter to the constructor of YmlConfigFile is correctly implemented. The super call has been appropriately updated to pass this new parameter along with the existing ones. This change supports the PR's objective of enhancing configuration capabilities by allowing multiple app IDs.

25-26: Verify the impact on existing code.

Given the change in the constructor signature, it is crucial to ensure that all existing code that instantiates YmlConfigFile is updated to pass the new appId parameter. This might involve significant changes across the codebase.

Run the following script to verify the usage of YmlConfigFile constructor:

Verification successful

Verification Successful: All instantiations of YmlConfigFile use the new constructor signature.

The instantiation of YmlConfigFile in DefaultConfigFactory.java includes the appId parameter, and no other outdated usages were found in the codebase. The update appears to be correctly applied across the codebase.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all instantiations of `YmlConfigFile` to ensure they include the new `appId` parameter.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new YmlConfigFile'
Length of output: 731
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/XmlConfigFile.java (3)
25-27: Constructor changes are correctly implemented.

The updated constructor now correctly passes the appId, namespace, and configRepository parameters to the superclass constructor. This change is essential for supporting multiple app IDs and aligns with the PR's objectives.

Line range hint 29-31: Config file format method remains correct.

The getConfigFileFormat method correctly returns ConfigFileFormat.XML, which is appropriate for the XmlConfigFile class. No changes were needed here, and the method continues to function as expected.

25-27: Verify the usage of the new constructor across the codebase.

The constructor of XmlConfigFile now includes an appId parameter. It is crucial to ensure that all instantiations of XmlConfigFile across the codebase are updated to pass this new parameter.

Run the following script to verify the constructor usage:

Verification successful

Constructor usage correctly updated across the codebase.

The XmlConfigFile constructor has been updated to include an appId parameter, and all instances of its usage in both test and main code have been correctly updated to reflect this change. No further action is required.

XmlConfigFileTest.java shows multiple instances of the updated constructor.

DefaultConfigFactory.java also uses the updated constructor.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all instantiations of `XmlConfigFile` include the new `appId` parameter.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new XmlConfigFile'
Length of output: 6186
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/JsonConfigFile.java (2)
25-27: Constructor change aligns with PR objectives.

The modification to include an appId parameter in the constructor of JsonConfigFile aligns with the PR's objective to support multiple app IDs. This change enhances the configuration management capabilities by allowing configurations to be associated with specific applications.

25-27: Verify the impact of the constructor change on other parts of the system.

The addition of the appId parameter to the constructor is a significant change. It is crucial to ensure that all parts of the system that instantiate JsonConfigFile are updated to pass the new appId parameter. This change should be verified to ensure it does not break existing functionality.

Run the following script to verify the usage of JsonConfigFile constructor across the codebase:

Verification successful

Constructor change verification successful.

The addition of the appId parameter to the JsonConfigFile constructor has been successfully integrated across the codebase. All instances of the constructor usage have been updated to include the new parameter, and the changes are reflected in both test and production code. No issues were found.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all instantiations of `JsonConfigFile` include the new `appId` parameter.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new JsonConfigFile'
Length of output: 4780
apollo-client-config-data/src/main/java/com/ctrip/framework/apollo/config/data/internals/PureApolloConfigFactory.java (1)
30-31: Verify the usage of the new method signature across the codebase.

The method createRepositoryConfig now includes an additional parameter appId. It is crucial to ensure that all references to this method in the codebase have been updated to pass the new parameter. This change could potentially break existing functionality if not handled correctly.

Run the following script to verify the method usage:

Verification successful

Verification successful: Method signature update is correctly implemented.

The createRepositoryConfig method's signature change to include the appId parameter has been verified. The method is used consistently with the new signature in the codebase, particularly in the DefaultConfigFactory class. No instances of the old signature were found, indicating that the update has been correctly propagated.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all function calls to `createRepositoryConfig` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'createRepositoryConfig'
Length of output: 2301
apollo-core/src/main/java/com/ctrip/framework/foundation/spi/provider/ApplicationProvider.java (1)
55-57: Address potential NullPointerException by providing a safer return value.

The method getAccessKeySecret(String appId) currently returns null, which could lead to NullPointerException in the calling code. It's important to handle this case more robustly. Consider implementing one of the following solutions:

Return an empty string if the access key is not available, which avoids NullPointerException and allows calling code to handle the absence of a key gracefully.

Throw a specific exception if the access key cannot be retrieved, which makes the error handling explicit and informs the caller of the issue.

Here are the proposed changes:
default String getAccessKeySecret(String appId) {
-  return null;
+  return ""; // Option 1: Return an empty string
}
Or:
default String getAccessKeySecret(String appId) {
-  return null;
+  throw new UnsupportedOperationException("Access key secret not available"); // Option 2: Throw an exception
}
Likely invalid or redundant comment.
apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigRegistry.java (2)

33-40: Review of the new register method with appId.

The addition of the appId parameter to the register method is a significant enhancement that aligns with the PR's objectives to support multiple app IDs. This change allows for more granular control over configuration management, especially in environments with multiple applications.

Correctness: The method signature and JavaDoc comments are correctly defined.

Consistency: This method complements the existing register method by adding an appId parameter, which is consistent with the need for handling multiple applications.

Documentation: The JavaDoc is clear and provides adequate information about the parameters and their purpose.

Overall, this is a well-implemented change that enhances the interface's functionality.

50-57: Review of the new getFactory method with appId.

Similar to the register method, the addition of the appId parameter to the getFactory method enhances the interface's capability to handle configurations for multiple applications. This method allows retrieving the configuration factory based on both appId and namespace, which is crucial for environments where configurations are specific to each application.

Correctness: The method signature and JavaDoc comments are correctly defined.

Consistency: This method complements the existing getFactory method by adding an appId parameter, maintaining consistency in the interface's design.

Documentation: The JavaDoc clearly explains the purpose of the parameters and the method's functionality.

This change is well-aligned with the PR's objectives and is implemented correctly.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactory.java (2)

35-42: Approved: New method for app-specific configuration.

The addition of Config create(String appId, String namespace) method is well-implemented and documented. It enhances the flexibility of the configuration system by allowing configurations to be specific to an application ID and namespace.

Ensure that existing implementations that use the old create(String namespace) method are not adversely affected by this change.

51-57: Approved: Enhanced method for creating config files with app context.

The addition of ConfigFile createConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) method is well-implemented and documented. It allows for the creation of configuration files that are specific to an application ID, namespace, and configuration file format, enhancing the system's usability and flexibility.

Ensure that existing implementations that use the old createConfigFile(String namespace, ConfigFileFormat configFileFormat) method are not adversely affected by this change.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEvent.java (2)

44-46: Review of Constructor Changes

The constructor of InterestedConfigChangeEvent has been modified to include an additional parameter appId. This change is consistent with the PR's objective to support multiple app IDs. The updated constructor correctly initializes the superclass with this new parameter, ensuring that the appId is now part of the event context.

This modification enhances the flexibility of the configuration change event handling by allowing events to be associated with specific applications. It is crucial to ensure that all parts of the system that instantiate this class are updated to pass the new appId parameter to avoid runtime errors.

Line range hint 54-56: Review of Method interestedChangedKeys

The method interestedChangedKeys remains unchanged and continues to function as expected. It correctly returns an unmodifiable set of the keys that triggered the change event, which is a good practice to ensure data integrity and prevent unintended modifications.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/TxtConfigFileTest.java (2)

34-34: Approved: Addition of someAppId variable.

The introduction of the someAppId variable is well-aligned with the PR's objectives to support multiple app IDs. It is correctly declared and initialized in the setUp method.

41-41: Approved: Initialization in setUp method.

The initialization of someAppId and someNamespace in the setUp method ensures that each test case starts with a consistent and isolated environment, which is a best practice in unit testing.

apollo-core/src/main/java/com/ctrip/framework/foundation/internals/provider/NullProvider.java (1)

87-90: Approved: New method getAccessKeySecret(String appId)

The addition of the method getAccessKeySecret(String appId) is consistent with the class's design of returning default values. The implementation correctly adheres to the expected behavior of the interface, enhancing the class's compliance without altering its existing functionality.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepository.java (1)
46-46: Verify integration and impact of the new appId parameter in fireRepositoryChange.

The addition of configFile.getAppId() as a parameter to fireRepositoryChange enhances the granularity of configuration change tracking. However, it is crucial to verify that this change integrates seamlessly with the existing system and does not introduce any unintended side effects.

Run the following script to verify the usage of the new method signature across the codebase:

Verification successful

Integration of appId parameter in fireRepositoryChange is consistent and complete.

The addition of configFile.getAppId() to the fireRepositoryChange method is consistently applied across the codebase, ensuring seamless integration without any unintended side effects.

The method is used with the new signature in RemoteConfigRepository, PropertiesCompatibleFileConfigRepository, and LocalFileConfigRepository.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the usage of the new `fireRepositoryChange` method signature across the codebase.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'fireRepositoryChange'
Length of output: 3269
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigFileChangeEvent.java (4)
26-26: Clarification needed for the addition of appId.

Based on the previous comment by nobodyiam questioning the absence of a getAppId method, it appears that the addition of the appId field and its corresponding getter method in this PR addresses that concern. However, it would be beneficial to confirm that this addition aligns with the intended design and usage within the broader application context.

50-52: Approval of the new getAppId method.

The addition of the getAppId method is a straightforward implementation that provides access to the appId field. This method enhances the class's interface by allowing external access to the appId, which is crucial for handling configurations specific to different applications.

73-74: Review of toString method updates.

The toString method is updated to include the appId in its output. This change ensures that the string representation of the class instances now accurately reflects the new state of the object, including the appId. This is particularly useful for logging and debugging purposes, where a complete description of the event is necessary.

35-43: Review of constructor changes.

The constructor now includes appId as the first parameter, which is a significant change as it affects how instances of ConfigFileChangeEvent are created throughout the application. This change is aligned with the PR's objective to support multiple app IDs. Ensure that all instantiations of this class in the codebase are updated to reflect this new parameter order.

Verification successful

Constructor changes verified successfully.

The instantiation of ConfigFileChangeEvent in the codebase has been updated to include appId as the first parameter, aligning with the new constructor signature. No other instances were found that require modification.

AbstractConfigFile.java: Correct usage of the new constructor signature.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all instantiations of `ConfigFileChangeEvent` are updated to include `appId`.

# Test: Search for the constructor usage. Expect: Only occurrences with the new signature.
rg --type java -A 5 $'new ConfigFileChangeEvent'
Length of output: 774

Script:
#!/bin/bash
# Description: Search for all instantiations of `ConfigFileChangeEvent` to ensure they use the new constructor signature.

# Test: Search for all occurrences of `ConfigFileChangeEvent` instantiation.
rg --type java 'new ConfigFileChangeEvent'
Length of output: 241
apollo-client/src/main/java/com/ctrip/framework/apollo/model/ConfigChangeEvent.java (3)
28-28: Field addition approved.

The addition of the private final String m_appId; field is appropriate for maintaining immutability and encapsulation in the class.

78-84: Method addition approved.

The addition of the getAppId() method is a standard practice for providing external access to private fields in Java. This method correctly returns the m_appId field.

36-38: Verify the impact of constructor parameter order change.

The constructor now includes an additional parameter appId, which is correctly initialized. However, this change in parameter order could impact existing code where this constructor is used.

Run the following script to verify the constructor usage:

Verification successful

Constructor parameter order change verified successfully.

The constructor ConfigChangeEvent is only used in test files, and all instances match the new signature with three parameters. There is no impact on production code.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `ConfigChangeEvent` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'new ConfigChangeEvent'
Length of output: 3495

Script:
#!/bin/bash
# Description: Verify all constructor calls to `ConfigChangeEvent` across the entire codebase to ensure they match the new signature.

# Test: Search for the constructor usage across all Java files. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'new ConfigChangeEvent'
Length of output: 3495
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigRegistryTest.java (2)

66-69: Approved: New create method overload with appId.

The addition of the appId parameter in the create method is correctly implemented and aligns with the PR objectives to enhance configuration capabilities.

76-79: Approved: New createConfigFile method overload with appId.

The addition of the appId parameter in the createConfigFile method is correctly implemented and aligns with the PR objectives to enhance configuration capabilities.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigRepository.java (1)
66-69: Review of the Modified fireRepositoryChange Method

The addition of the appId parameter to the fireRepositoryChange method is a significant enhancement that aligns with the PR's objectives to support multiple app IDs. This change allows the method to provide more contextual information about the application associated with the configuration change, which is crucial for systems managing configurations across multiple applications.

Key Observations:

The method now iterates over m_listeners and invokes onRepositoryChange on each listener with the new appId parameter.

Exception handling within the loop is robust, logging the error and continuing with the next listener, which ensures that a failure in one listener does not halt the notification process for others.

Suggestions:

**** Ensure that all implementations of RepositoryChangeListener have been updated to handle the new method signature. This is crucial to prevent runtime method signature mismatches.

**** Consider adding more detailed logging at the debug level to trace the values of appId, namespace, and newProperties being passed to each listener. This could be valuable for troubleshooting and verifying that the correct data is being used.

**** If there are any concerns about how to update the listeners or if additional testing is required, I can assist with these updates or provide guidance on setting up appropriate tests.

Verification successful

Verification of onRepositoryChange Method Signature Update

The implementations of RepositoryChangeListener have been updated to handle the new onRepositoryChange method signature with the appId parameter. The presence of both method signatures in the RepositoryChangeListener interface ensures backward compatibility. Test files have been updated to verify the new functionality.

Implementations Updated: LocalFileConfigRepository, AbstractConfigFile, SimpleConfig, DefaultConfig.

Tests Updated: JsonConfigFileTest, XmlConfigFileTest, DefaultConfigTest, SimpleConfigTest, RemoteConfigRepositoryTest, and others.

The changes align with the PR's objectives to support multiple app IDs, and the backward compatibility is maintained. No further action is required.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all implementations of RepositoryChangeListener handle the new method signature.

# Test: Search for implementations of RepositoryChangeListener. Expect: Updated method signatures.
rg --type java -A 5 $'onRepositoryChange'
Length of output: 63127
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/EnableApolloConfig.java (1)

73-78: Well-documented addition of multipleConfigs() method.

The addition of the multipleConfigs() method is well-documented and clearly explains its purpose and non-participation in the bootstrap process. The use of the @since tag is a good practice, helping track the introduction of new features. The default value being an empty array ensures backward compatibility.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/PropertiesConfigFile.java (1)
39-41: Review the modified constructor with the new appId parameter.

The addition of the appId parameter to the constructor is a significant change that aligns with the PR's objectives to support multiple app IDs. This change allows for more granular control over configurations per application. Ensure that:

The superclass AbstractConfigFile has been modified to accept this new parameter.

All instantiations of PropertiesConfigFile throughout the codebase are updated to include the appId parameter.

Run the following script to verify the superclass modification and usage of the new constructor:

Verification successful

Superclass AbstractConfigFile supports the new appId parameter.

The AbstractConfigFile constructor has been updated to accept the appId parameter, ensuring compatibility with the changes made to the PropertiesConfigFile constructor. The codebase has been updated to use the new constructor signature where necessary.

The AbstractConfigFile constructor now includes appId, namespace, and configRepository parameters.

Instantiations of PropertiesConfigFile have been updated to pass the appId parameter.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the superclass and usage of the new constructor.

# Test: Search for the superclass constructor modification.
ast-grep --lang java --pattern $'class AbstractConfigFile {
  $$$
  AbstractConfigFile($_, $_, $_) {
    $$$
  }
  $$$  
}'

# Test: Search for the usage of the new constructor.
rg --type java -A 5 $'new PropertiesConfigFile'
Length of output: 7018

Script:
#!/bin/bash
# Description: Verify the implementation of AbstractConfigFile to check for appId support.

# Search for the AbstractConfigFile class definition and its constructor.
ast-grep --lang java --pattern $'class AbstractConfigFile {
  $$$
  AbstractConfigFile($$$) {
    $$$
  }
  $$$  
}'
Length of output: 117

Script:
#!/bin/bash
# Description: Locate the AbstractConfigFile.java file and inspect its constructor.

# Find the AbstractConfigFile.java file and display its contents.
fd -e java -x bash -c 'echo "File: {}"; cat {}' | grep -A 10 'class AbstractConfigFile'
Length of output: 747

Script:
#!/bin/bash
# Description: Search for the constructor of AbstractConfigFile to verify its parameters.

# Find the AbstractConfigFile.java file and search for its constructor.
fd -e java -x bash -c 'echo "File: {}"; cat {}' | grep -A 10 'AbstractConfigFile('
Length of output: 767
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/YamlConfigFile.java (2)
39-40: Proper Error Handling in tryTransformToProperties

The method tryTransformToProperties demonstrates good error handling practices by catching exceptions, logging them appropriately, and returning a boolean status. This ensures that the application can gracefully handle errors during the properties transformation process.

39-40: Constructor Signature Change: Ensure Compatibility

The constructor of YamlConfigFile now includes an appId parameter. This is a significant change that enhances the class's functionality by allowing it to handle configurations specific to an application ID. Ensure that all instantiations of YamlConfigFile across the codebase are updated to pass this new parameter.

Run the following script to verify the constructor usage:

Verification successful

Constructor Signature Change Verified

The constructor of YamlConfigFile has been successfully updated to include the appId parameter across the codebase. All instances of YamlConfigFile creation, including those in test files and the main source file DefaultConfigFactory.java, have been correctly modified to use the new constructor signature. No further action is required.

Instances in AbstractSpringIntegrationTest.java and YamlConfigFileTest.java confirm the update in test scenarios.

The update in DefaultConfigFactory.java confirms the change in production code.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `YamlConfigFile` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'new YamlConfigFile'
Length of output: 7131
apollo-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java (1)
60-60: Review: Updated constructor call for LocalFileConfigRepository.

The change to the constructor of LocalFileConfigRepository from a single argument to two arguments (someAppId, someNamespace) is consistent with the PR's objective to support multiple app IDs. This update is crucial for ensuring that the configuration repository can handle configurations specific to an application ID alongside the namespace.

However, it's essential to verify that this change integrates well with the rest of the system, especially since it affects how configurations are loaded and managed. The test should ensure that the new functionality does not disrupt existing functionalities and that it aligns with the overall system architecture.

Verification successful

Verified: Consistent use of the new constructor for LocalFileConfigRepository.

The constructor change to LocalFileConfigRepository(someAppId, someNamespace) is consistently applied across the codebase, including both main and test files. This indicates that the new functionality is well-integrated and adequately tested, aligning with the PR's objective to support multiple app IDs.

Files with updated constructor usage:

ApolloTestingServer.java

ApolloMockServerApiWhileCacheDirSpecifiedTest.java

LocalFileConfigRepositoryTest.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that the new constructor is used consistently across the project.

# Test: Search for the constructor usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'new LocalFileConfigRepository'
Length of output: 8309
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryFileCachePropertyTest.java (3)

41-41: Approved: Introduction of someAppId.

The addition of someAppId enhances the flexibility of the test setup by allowing dynamic configuration of the app ID, aligning with the PR's objectives.

Also applies to: 46-46

58-58: Approved: Correct usage of someAppId in enabled file caching scenario.

The dynamic passing of someAppId to createLocalConfigRepository is correctly implemented and verified. Consider verifying the integration of this change with other parts of the system to ensure overall compatibility.

Also applies to: 61-62

70-70: Approved: Correct usage of someAppId in disabled file caching scenario.

The dynamic passing of someAppId to createRemoteConfigRepository is correctly implemented and verified. Consider verifying the integration of this change with other parts of the system to ensure overall compatibility.

Also applies to: 74-74

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/InterestedConfigChangeEventTest.java (2)

45-45: Approved: Addition of someAppId variable.

The addition of the someAppId variable aligns with the PR's objective to support multiple app IDs and enhances maintainability by centralizing the app ID value.

Line range hint 45-77: Summary: Well-integrated changes.

The changes made in the InterestedConfigChangeEventTest class are well-integrated and align with the PR's objectives to support multiple app IDs. The use of someAppId enhances the flexibility and maintainability of the test configurations.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesCompatibleFileConfigRepositoryTest.java (3)

43-43: Approved: Addition of someAppId variable.

The addition of the someAppId variable is consistent with the PR's objective to enhance configuration capabilities by supporting multiple app IDs.

51-53: Approved: Changes in the setUp method.

The initialization of someAppId and the update to the when clause for configFile.getAppId() are well-implemented to support the new functionality of handling multiple app IDs in the configuration.

135-135: Approved: Changes in the testOnChange method.

The modification to include someAppId in the onRepositoryChange method call is correctly implemented to test the new behavior of handling multiple app IDs.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/CachedCompositePropertySourceTest.java (4)

Line range hint 29-48: Setup method is correctly implemented.

The setUp method properly initializes the test environment with necessary mocks and listener setups.

Line range hint 95-99: Property source addition and listener registration are correctly tested.

The testAddPropertySource method correctly verifies the addition of property sources and the registration of listeners.

Line range hint 101-120: First property source addition and listener registration are correctly tested.

The testAddFirstPropertySource method correctly tests the addition of a property source at the beginning and ensures that listeners are correctly added.

85-85: Verify the updated onChange method invocation.

The change to pass three null parameters in the onChange method needs verification to ensure it aligns with the intended functionality and does not alter the expected behavior of the test.

Run the following script to verify the method signature and its usage:

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/JsonConfigFileTest.java (6)

Line range hint 42-51: Setup method correctly initializes someAppId.

The introduction of someAppId in the setUp method is correctly implemented and crucial for testing the new functionality.

67-67: Correct usage of updated constructor in testWhenHasContent.

The test method testWhenHasContent correctly uses the updated JsonConfigFile constructor with the new someAppId parameter.

80-80: Correct usage of updated constructor in testWhenHasNoContent.

The test method testWhenHasNoContent correctly uses the updated JsonConfigFile constructor with the new someAppId parameter.

90-90: Correct usage of updated constructor in testWhenConfigRepositoryHasError.

The test method testWhenConfigRepositoryHasError correctly uses the updated JsonConfigFile constructor with the new someAppId parameter.

Line range hint 110-121: Correct usage of updated method signature in testOnRepositoryChange.

The test method testOnRepositoryChange correctly uses the updated onRepositoryChange method with the new someAppId parameter.

139-145: Correct usage of updated constructor and method in testWhenConfigRepositoryHasErrorAndThenRecovered.

The test method testWhenConfigRepositoryHasErrorAndThenRecovered correctly uses the updated JsonConfigFile constructor and onRepositoryChange method with the new someAppId parameter.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfigFile.java (5)
48-48: Constructor and Field Initialization Approved

The addition of the m_appId field and its initialization in the constructor are correctly implemented to support multiple app IDs as per the PR's objectives.

Also applies to: 61-63

85-88: New Method getAppId() Approved

The implementation of the getAppId() method is correct and provides a necessary accessor for the m_appId field.

99-100: Incorporation of Suggested Change

The change suggested by nobodyiam has been successfully incorporated into the new onRepositoryChange method.

99-100: Modified Method onRepositoryChange Approved

The changes to the onRepositoryChange method correctly incorporate the application ID into configuration change events. It is recommended to verify the integration of this method with other components that handle configuration changes.

Also applies to: 102-125

Verification successful

Integration of onRepositoryChange Method Verified

The onRepositoryChange method, with the new appId parameter, is well-integrated and tested across various components in the codebase. The test cases confirm its correct functionality in different scenarios. No further action is required.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify integration of the new `onRepositoryChange` method with other components.

# Test: Search for method usage in other components. Expect: Proper handling of the new method signature.
rg --type java -A 5 $'onRepositoryChange'
Length of output: 63127
Line range hint 48-125: Overall Changes Approved

The modifications in the AbstractConfigFile class are well-integrated and consistent with the PR's objectives to handle multiple app IDs. It is recommended to conduct further testing to ensure robustness and compatibility.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java (1)

46-46: Addition of someAppId variable.

The introduction of the someAppId variable aligns with the PR's objective to handle multiple app IDs. This variable is used effectively across the test configurations and method signatures to simulate a multi-app environment.

Also applies to: 54-54

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/SimpleConfigTest.java (5)

52-52: Addition of someAppId instance variable.

The addition of the someAppId variable is consistent with the PR's objective to support multiple app IDs. This change is crucial for the new functionality and is well-integrated into the existing test setup.

62-62: Initialization of someAppId in the setUp method.

The initialization of someAppId in the setUp method ensures that it is available for all tests, supporting the new constructor signature of SimpleConfig. This is a necessary change to accommodate the new functionality.

91-91: Updated constructor calls to include someAppId.

The constructor of SimpleConfig has been updated across multiple test methods (testGetProperty, testLoadConfigFromConfigRepositoryError, testOnRepositoryChange) to include the new someAppId parameter. This change is necessary to test the new functionality introduced in the PR and ensures that the SimpleConfig class can handle configurations for specific applications.

Also applies to: 104-104, 138-138

147-147: Updated onRepositoryChange method to pass someAppId.

The onRepositoryChange method now correctly passes someAppId as an argument, reflecting the updated constructor requirements. This change is crucial for ensuring that the configuration changes are correctly propagated and handled within the application context.

151-151: Updated assertions to verify someAppId.

The assertions in the testOnRepositoryChange method have been updated to verify that the changeEvent correctly reflects the new someAppId value. This is an important update to ensure that the tests accurately check the functionality related to handling multiple app IDs.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/AbstractConfigTest.java (4)
47-47: Approved: Introduction of someAppId variable.

The introduction of the someAppId variable centralizes the application ID, enhancing maintainability and reducing the risk of hardcoded strings. This change aligns well with the PR's objectives.

111-112: Approved: Consistent use of someAppId.

The consistent use of someAppId in the ConfigChange and ConfigChangeEvent objects across different test methods supports the PR's objective and enhances configuration flexibility.

156-158: Approved: Consistent use of someAppId.

The consistent use of someAppId in the ConfigChange and ConfigChangeEvent objects across different test methods supports the PR's objective and enhances configuration flexibility.

68-72: Approved: Use of someAppId in test method.

The use of someAppId in creating ConfigChange and ConfigChangeEvent objects, and the new call to fireConfigChange enhance flexibility and maintain consistency. Ensure that the new method signature for fireConfigChange is correctly implemented across the codebase.

Verification successful

Verified: Consistent usage of fireConfigChange method signature.

The fireConfigChange method with the new signature is used consistently across the codebase, and the use of someAppId in the test method aligns with the defined method signature. No issues were found.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the new method signature for `fireConfigChange` is used consistently.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'fireConfigChange'
Length of output: 11103
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessorTest.java (5)

53-54: Addition of someAppId constant.

The introduction of a constant for someAppId is a good practice as it avoids hardcoding the same string multiple times across the test methods. This change enhances maintainability and readability.

58-59: Setting someAppId as a system property in setUp.

Setting the appId as a system property is crucial for the tests that depend on this configuration. This setup ensures that the environment is correctly prepared for the tests to run under the conditions that mimic the actual usage of the application with the specified appId.

Line range hint 81-107: Enhanced test cases with someAppId.

The modifications in the testInitializePropertySources method to include someAppId in the configuration and assertions are aligned with the new functionality of handling multiple app IDs. These changes ensure that the tests are robust and accurately reflect the expected behavior when different app IDs are used.

The use of someAppId in mockConfig calls ensures that the configuration is correctly mocked with respect to the application ID.

The assertions have been updated to check for the correct formation of property source names, which now incorporate the application ID.

This thorough testing approach helps in validating the functionality introduced in the PR.

113-116: Testing application event handling with someAppId.

The changes in the testApplicationEvent method to include someAppId in the configuration and event handling tests are appropriate. This ensures that the application events are correctly processed when different app IDs are used, which is crucial for the new feature's functionality.

The mockConfig call with someAppId ensures that the configuration is correctly set up for the test.

The handling of the ConfigChangeEvent and the publishing of the ApolloConfigChangeEvent are critical to ensure that the system reacts appropriately to configuration changes.

These tests are essential for verifying that the application's event system works correctly with multiple app IDs.

161-161: Assertion on system properties override.

The assertion in the testOverrideSystemProperties method that checks the name of the first property source ensures that the system properties are correctly overridden as specified by the configuration. This is an important aspect of the configuration system that needs to be tested to ensure that the application behaves as expected under different configuration settings.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/DefaultConfigFactory.java (9)

67-68: Refactor: Simplify method delegation.

The method create(String namespace) now delegates to create(String appId, String namespace) using the appId fetched from ConfigUtil. This is a clean implementation that maintains backward compatibility by using the default appId.

Line range hint 70-91: Review: Core configuration creation logic with appId.

The method create(String appId, String namespace) has been refactored to include the appId parameter. This change allows for more granular control over configurations per application. The logic inside the method correctly handles different ConfigFileFormat types and delegates to the appropriate repository creation methods based on the format compatibility.

93-95: Refactor: Simplify method delegation for ConfigFile creation.

The method createConfigFile(String namespace, ConfigFileFormat configFileFormat) now delegates to createConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) using the appId fetched from ConfigUtil. This maintains backward compatibility and simplifies the API for clients that do not need to specify an appId.

98-117: Review: Enhanced ConfigFile creation with appId.

The method createConfigFile(String appId, String namespace, ConfigFileFormat configFileFormat) has been updated to handle different file formats and create the appropriate ConfigFile instances using the appId. This change ensures that configuration files are correctly associated with their respective applications, enhancing the system's ability to manage configurations in a multi-application environment.

123-127: Review: ConfigRepository creation logic with appId.

The method createConfigRepository(String appId, String namespace) has been updated to decide between local and remote repositories based on the property file cache setting. This method now correctly uses the appId to create the appropriate repository, ensuring that configurations are managed per application.

133-144: Review: LocalFileConfigRepository creation with appId.

The method createLocalConfigRepository(String appId, String namespace) has been updated to include the appId in its signature and logic. This change is crucial for ensuring that local configurations are correctly associated with the specified application, especially in environments where Apollo is running in local mode.

147-148: Review: RemoteConfigRepository creation with appId.

The method createRemoteConfigRepository(String appId, String namespace) now correctly uses the appId to create a remote configuration repository. This change is essential for ensuring that remote configurations are managed per application, aligning with the overall goal of enhancing configuration management in multi-application environments.

152-153: Review: PropertiesCompatibleFileConfigRepository creation with appId.

The method createPropertiesCompatibleFileConfigRepository(String appId, String namespace, ConfigFileFormat format) now includes the appId in its logic, ensuring that the properties-compatible configuration files are correctly associated with the specified application. This method correctly handles the namespace formatting based on the file format and application ID, enhancing the flexibility and accuracy of configuration management.

172-172: Review: Namespace trimming with appId.

The method trimNamespaceFormat(String appId, String namespaceName, ConfigFileFormat format) has been updated to include the appId in its logic. This change ensures that the namespace is correctly formatted per application, which is crucial for maintaining consistency and accuracy in configuration file naming and management.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/PropertiesConfigFileTest.java (7)

Line range hint 51-60: Initialization of someAppId is correct.

The setup method correctly initializes someAppId, which is essential for the tests to accurately reflect the new constructor parameter.

85-85: Test method update is correct.

The update to include someAppId in the PropertiesConfigFile constructor call is correctly implemented in the testWhenHasContent method.

97-97: Test method update is correct.

The update to include someAppId in the PropertiesConfigFile constructor call is correctly implemented in the testWhenHasNoContent method.

107-107: Test method update is correct.

The update to include someAppId in the PropertiesConfigFile constructor call is correctly implemented in the testWhenConfigRepositoryHasError method.

123-123: Test method update is correct.

The update to include someAppId in the PropertiesConfigFile constructor call is correctly implemented in the testOnRepositoryChange method.

162-162: Test method update is correct.

The update to include someAppId in the PropertiesConfigFile constructor call is correctly implemented in the testWhenConfigRepositoryHasErrorAndThenRecovered method.

182-182: Test method updates are correct.

The updates to include someAppId in the PropertiesConfigFile constructor calls are correctly implemented in the testIfCompatibleWithProperties and testIfCompatibleWithEmptyProperties methods.

Also applies to: 197-197

apollo-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java (2)

59-59: Refactor: Use of Multimap for Namespace Management

The introduction of APP_NAMESPACE_NAMES as a Multimap to manage namespaces associated with multiple app IDs is a significant improvement. This change aligns with the PR's objective to support multiple app IDs and enhances the flexibility of namespace management.

172-172: Consistency Maintained in Reset Method

The update to the reset method to clear APP_NAMESPACE_NAMES ensures that the new data structure is properly reset, maintaining consistency across the class.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderTest.java (6)

39-39: Introduction of someAppId variable.

The introduction of the someAppId variable is a positive change as it centralizes the application ID, making the tests more maintainable and adaptable to changes in configuration. This aligns with the PR's objective to handle multiple app IDs.

66-66: Proper use of someAppId in mockConfig.

The replacement of the hardcoded namespace with someAppId in the mockConfig call is correctly implemented and enhances the flexibility of the configuration.

74-74: Consistent use of someAppId in mockConfig.

Using someAppId in the mockConfig method call maintains consistency and supports the PR's goal of flexible configuration management.

88-88: Correct implementation of someAppId in mockConfig.

The use of someAppId in the mockConfig call is correctly implemented, enhancing the test's adaptability to configuration changes.

101-106: Flexible use of someAppId across different namespaces.

The use of someAppId for different namespaces within the same test method effectively demonstrates the enhanced flexibility and adaptability of the configuration system.

117-122: Consistent and flexible use of someAppId in preparation method.

The use of someAppId in the prepare method for different namespaces is consistent with its usage in other test methods, supporting the PR's objectives of enhanced configuration flexibility.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/LocalFileConfigRepositoryTest.java (5)

122-122: Approved: Constructor updated to include someAppId.

The update to include someAppId in the constructor call aligns with the PR objectives. Consider adding specific tests to verify the behavior when someAppId is different or not set.

138-138: Approved: Constructor updated to include someAppId with fallback repository.

The update to include someAppId in the constructor call aligns with the PR objectives. Consider adding specific tests to verify the behavior when someAppId is different or not set, especially in the presence of a fallback repository.

150-150: Approved: Constructor updated to include someAppId for scenarios without a local file.

The update to include someAppId in the constructor call aligns with the PR objectives. Consider adding specific tests to verify the behavior when someAppId is different or not set, particularly when there is no local file.

164-164: Approved: Constructor updated to include someAppId for multiple access scenarios.

The updates to include someAppId in the constructor calls align with the PR objectives. Consider adding specific tests to verify the behavior when someAppId is different or not set, especially when accessing the local file multiple times.

Also applies to: 171-171

187-187: Approved: Constructor and method updated to include someAppId for repository change scenarios.

The updates to include someAppId in the constructor call and onRepositoryChange method align with the PR objectives. Consider adding specific tests to verify the behavior when someAppId is different or not set, particularly when the repository changes.

Also applies to: 204-204

apollo-client/src/test/java/com/ctrip/framework/apollo/util/ConfigUtilTest.java (1)

226-226: Ensure consistency in cache directory path construction.

The test testLocalCacheDirWithSystemProperty correctly checks if the local cache directory path includes the appId. However, it's important to ensure that the path construction is consistent across different environments and does not lead to issues with path delimiters on different operating systems.

Consider adding a utility method to handle path constructions or ensure that File.separator is consistently used across the application to avoid hardcoding path delimiters.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/XmlConfigFileTest.java (4)
Line range hint 52-61: Initialization of test variables is well-implemented.

The setup method correctly initializes someAppId and someNamespace, which are crucial for the tests that follow. This ensures that each test can operate with its own set of parameters, enhancing test reliability and isolation.

88-88: Consistent use of the new constructor across test methods.

All instances of XmlConfigFile instantiation in the test methods have been updated to include the new appId parameter. This change is consistent with the modifications described in the PR summary and ensures that the new functionality is being tested correctly.

Also applies to: 100-100, 110-110, 126-126, 162-162, 199-199, 235-235

143-143: Correct adaptation of the onRepositoryChange method to include appId.

The onRepositoryChange method calls have been correctly updated to include the appId parameter. This adaptation is crucial for ensuring that the method aligns with the changes made to the XmlConfigFile class, reflecting the new requirement to handle configurations based on the application ID.

Also applies to: 240-240

162-162: Potential issue with method call in testOnRepositoryChangeWithContentAdded.

The call to onRepositoryChange in the testOnRepositoryChangeWithContentAdded method does not include the appId parameter. This seems to be an oversight and could lead to test failures or incorrect behavior since the method signature expects the appId as the first parameter.

Consider updating the method call to include appId:
- configFile.onRepositoryChange(someNamespace, anotherProperties);
+ configFile.onRepositoryChange(someAppId, someNamespace, anotherProperties);
Likely invalid or redundant comment.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/YamlConfigFileTest.java (7)

42-42: Setup of someAppId and someNamespace is correct.

The initialization of someAppId and someNamespace in the setUp method is correctly done. This setup is crucial for the tests that follow, ensuring that the YamlConfigFile instances are correctly instantiated with these parameters.

Also applies to: 55-55

89-89: Correct instantiation of YamlConfigFile.

The YamlConfigFile is correctly instantiated with the new someAppId parameter along with someNamespace and configRepository. This change aligns with the PR objectives to handle configurations based on application context.

115-115: Correct instantiation of YamlConfigFile with ordered properties.

The instantiation of YamlConfigFile in the testWhenHasContentWithOrder method correctly includes the someAppId parameter. This ensures that the test reflects the new functionality introduced in the PR.

129-129: Correct instantiation of YamlConfigFile in no-content scenario.

The instantiation of YamlConfigFile in the testWhenHasNoContent method correctly includes the someAppId parameter, ensuring that the test accurately tests scenarios where no content is available.

152-152: Correct instantiation of YamlConfigFile in invalid YAML content scenario.

The instantiation of YamlConfigFile in the testWhenInvalidYamlContent method correctly includes the someAppId parameter. This ensures that the test accurately tests scenarios with invalid YAML content.

171-171: Correct instantiation of YamlConfigFile in error scenario.

The instantiation of YamlConfigFile in the testWhenConfigRepositoryHasError method correctly includes the someAppId parameter. This ensures that the test accurately tests scenarios where the config repository encounters an error.

203-203: Correct handling of YamlConfigFile in repository change scenarios.

The instantiation and method calls in the testOnRepositoryChange and testWhenConfigRepositoryHasErrorAndThenRecovered methods correctly include the someAppId parameter. This ensures that the tests accurately reflect the new functionality for handling repository changes based on the application ID.

Also applies to: 215-215, 238-238, 245-245
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java (1)
179-179: Verify the impact of using someAppId on configuration loading.

The introduction of someAppId as an argument in LocalFileConfigRepository is a significant change. It's crucial to verify that this modification does not affect the existing functionality of configuration loading and that it correctly supports the new feature of handling multiple app IDs.

Run the following script to verify the impact on configuration loading:

Verification successful

Verified: Usage of someAppId in ApolloTestingServer.java

The usage of someAppId as an argument in LocalFileConfigRepository is consistent with its role as a placeholder for testing purposes. This change does not impact the configuration loading process in a production environment. The static declaration of someAppId ensures that it remains a fixed value, aligning with the testing objectives.

ApolloTestingServer.java: someAppId is declared and used as a static string.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify the impact of using `someAppId` on configuration loading.

# Test: Search for the usage of `LocalFileConfigRepository` with `someAppId`. Expect: Correct instantiation and usage.
rg --type java -A 5 $'new LocalFileConfigRepository(someAppId, $_)'
Length of output: 68

Script:
#!/bin/bash
# Description: Search for the declaration and usage of `someAppId` in the codebase.

# Find the declaration of `someAppId`.
rg --type java 'someAppId'

# Find all usages of `someAppId` to understand its context.
rg --type java '\bsomeAppId\b'
Length of output: 167870
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java (6)

68-68: Refactor: Use Table for CONFIG_REGISTRY.

The change from a Map to a Table allows for a more structured storage of configurations, associating both an application ID and a namespace with each configuration object. This is a positive change that aligns with the PR's objectives to support multiple app IDs.

94-101: Update: prepareConfig method signature and logic.

The addition of the appId parameter to the prepareConfig method is consistent with the PR's goal of supporting multiple app IDs. The method now correctly uses the appId when creating a SimpleConfig instance and when mocking the config. This change enhances the flexibility and specificity of configuration management within the test framework.

122-130: Update: prepareYamlConfigFile method signature and logic.

The method now requires an appId as an argument, which is used to create and mock a YamlConfigFile. This change is necessary for the support of multiple app IDs and ensures that configuration files are fetched correctly based on the new key format.

170-171: Refactor: mockConfig and mockConfigFile methods to support multiple app IDs.

The methods have been updated to include appId in their signatures and logic. This is crucial for correctly storing and retrieving configurations based on both the app ID and namespace, supporting the new Table structure in CONFIG_REGISTRY and the modified key format in CONFIG_FILE_REGISTRY.

Also applies to: 178-180

219-220: Refactor: Default appId handling in MockConfigManager.

The MockConfigManager class now defaults to using a specific appId ("someAppId") when only the namespace is provided. This change ensures backward compatibility with existing tests that may not specify an appId. However, it's important to ensure that this default behavior is clearly documented to avoid confusion.

Consider adding documentation or comments in the code to explain the default behavior of using "someAppId" when appId is not specified.

Also applies to: 233-234

223-228: Update: getConfig and getConfigFile methods in MockConfigManager.

These methods have been updated to handle the new appId parameter, allowing them to retrieve configurations and configuration files based on the new registry structure. The implementation ensures that configurations are fetched correctly based on both the app ID and namespace, which is essential for the new functionality introduced in this PR.

Also applies to: 237-238
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XMLConfigAnnotationTest.java (2)
53-53: Addition of someAppId constant.

The addition of the someAppId constant is a good practice for maintaining consistency across test cases. This ensures that all tests are aligned with the new feature of handling multiple app IDs.

60-61: Updated mockConfig method calls to include someAppId.

The changes to the mockConfig method calls across various test methods are consistent with the PR's objective to support multiple app IDs. By adding someAppId as the first parameter, the tests now reflect the updated method signature and are aligned with the new configuration handling capabilities.

It's important to ensure that these changes are reflected in the actual implementation of the mockConfig method to avoid any runtime errors or misconfigurations. Additionally, the consistency of these changes across different test methods helps maintain uniformity in testing scenarios.

Also applies to: 74-74, 84-85, 139-139, 148-148, 158-159

Verification successful

Verification Successful: mockConfig Method Updated

The mockConfig method has been updated to accept three parameters, including someAppId, as confirmed by its definition in AbstractSpringIntegrationTest.java. This aligns with the review comment and supports the changes made in the test files to handle multiple app IDs.

Location of mockConfig method definition:

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/AbstractSpringIntegrationTest.java
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that the `mockConfig` method has been updated to accept three parameters.

# Test: Search for the `mockConfig` method definition. Expect: Method definition with three parameters.
rg --type java -A 5 $'mockConfig'
Length of output: 95736

Script:
#!/bin/bash
# Use ast-grep to find the definition of the `mockConfig` method and verify its parameters.
ast-grep --lang java --pattern $'void mockConfig($_, $_, $_) { $$$ }'
Length of output: 70

Script:
#!/bin/bash
# Use rg to search for any method named `mockConfig` to verify its parameters.
rg --type java 'void mockConfig' -A 3
Length of output: 3955
apollo-client/src/test/java/com/ctrip/framework/apollo/spi/DefaultConfigFactoryTest.java (7)
62-62: Initialization of someAppId

The initialization of someAppId with "someAppId" is straightforward and aligns with the PR's objective to handle multiple app IDs. This setup is crucial for the tests that follow, ensuring they operate under the correct application context.

191-204: Use of appIdAndNamespace helper method in checkNamespaceName

The introduction of the appIdAndNamespace method suggests a future integration of the appId into namespace handling. Currently, it returns only the namespace, but this setup indicates potential enhancements. It's important to monitor this method for future changes that might incorporate the appId more directly into the logic.

84-84: Modification to createConfigRepository method call

The method createConfigRepository now requires an appId along with the namespace. This change is consistent with the PR's goal to support multiple app IDs. It's important to ensure that all calls to this method across the codebase have been updated to pass the appId as this could break existing functionality if overlooked.

Verification successful

Verification Successful: All createConfigRepository Calls Updated

All instances of the createConfigRepository method in the codebase have been updated to include the appId parameter, ensuring consistency with the updated method signature. No issues were found.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all calls to `createConfigRepository` include the `appId`.

# Test: Search for the method usage. Expect: Only occurrences with two parameters.
rg --type java -A 5 $'createConfigRepository'
Length of output: 5181
117-117: Modification to createPropertiesCompatibleFileConfigRepository method call

The method now includes an additional parameter for the appId, which is in line with the changes needed to support multiple app IDs. This adjustment is critical and should be checked across the codebase to ensure consistency and avoid runtime errors.

Verification successful

Verification Successful: All calls to createPropertiesCompatibleFileConfigRepository include the appId parameter. The codebase is consistent with the changes needed to support multiple app IDs, as described in the review comment. No further action is required.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all calls to `createPropertiesCompatibleFileConfigRepository` include the `appId`.

# Test: Search for the method usage. Expect: Only occurrences with three parameters.
rg --type java -A 5 $'createPropertiesCompatibleFileConfigRepository'
Length of output: 2599
136-138: Repeated modifications to createLocalConfigRepository method call

The method is called multiple times with different namespaces but now includes the appId. This repetition confirms the need to handle multiple configurations simultaneously, aligning with the PR's objectives. It's essential to ensure that these changes are reflected wherever this method is used.

Verification successful

Consistent usage of createLocalConfigRepository method with appId parameter

The createLocalConfigRepository method is consistently used with the appId parameter across the codebase, including in test files. This confirms that the changes are reflected as intended. No further action is needed.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify repeated calls to `createLocalConfigRepository` include the `appId`.

# Test: Search for the method usage. Expect: Multiple occurrences with two parameters.
rg --type java -A 5 $'createLocalConfigRepository'
Length of output: 6773
216-216: Modification to trimNamespaceFormat method call

The method now includes someAppId as a parameter, which is necessary for the new functionality to handle multiple app IDs. This change is crucial and should be verified across the project to ensure that it doesn't introduce any regressions.

Verification successful

Verification Successful: All calls to trimNamespaceFormat include appId

The method trimNamespaceFormat is consistently called with the appId parameter across the codebase, as required by the recent changes. No regressions are introduced by this modification.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all calls to `trimNamespaceFormat` include the `appId`.

# Test: Search for the method usage. Expect: Only occurrences with three parameters.
rg --type java -A 5 $'trimNamespaceFormat'
Length of output: 2363
99-99: Modification to createLocalConfigRepository method call

This change reflects the addition of someAppId to the method signature, which is necessary for the new multi-app ID support. It's crucial to verify that this method's usage throughout the project has been updated accordingly.

Verification successful

All usages of createLocalConfigRepository have been updated correctly. The method is consistently called with two parameters, appId and namespace, across the codebase, confirming the update for multi-app ID support. No further action is needed.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all calls to `createLocalConfigRepository` include the `appId`.

# Test: Search for the method usage. Expect: Only occurrences with two parameters.
rg --type java -A 5 $'createLocalConfigRepository'
Length of output: 6773
apollo-client/src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java (3)

109-109: Enhancement: Support for multiple app IDs.

The use of StringUtils.defaultIfBlank ensures that a default appId is used if none is specified in the annotation. This change supports the PR's objective to handle multiple app IDs effectively.

111-111: Correct use of environment placeholders.

Resolving placeholders for both appId and namespace ensures that the configuration can dynamically adapt to different environments, which is crucial for applications that may operate under varying configurations.

113-113: Update to ConfigService.getConfig call.

The method now correctly handles multiple configurations by accepting both resolvedAppId and resolvedNamespace. This change is essential for the flexibility required in multi-application environments.

apollo-core/src/main/java/com/ctrip/framework/apollo/core/utils/StringUtils.java (1)

343-345: Review of the defaultIfBlank method.

The method defaultIfBlank is correctly implemented and aligns with the PR's objective to enhance configuration capabilities by allowing a default value for blank strings. The use of isBlank to check the string condition is appropriate and leverages existing functionality effectively.

Correctness: The method correctly checks if the string s is blank and returns appId if true, otherwise returns s.

Logic: The logic is straightforward and efficient, using a ternary operator for a concise return statement.

Performance: The method is efficient as it uses existing logic from isBlank and involves minimal computation.

Integration: This method integrates well with the existing utility class and follows the established pattern of utility methods.

Overall, this addition is a valuable enhancement to the StringUtils class, providing a useful utility method for handling blank strings in configurations.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java (3)

68-69: Refactor: Simplified constructor delegation.

The constructor now delegates to the more general constructor using null for appId. This is a good use of constructor chaining to avoid code duplication.

240-241: Refactor: Method Overloading for onRepositoryChange.

The method now supports handling changes with an appId, which aligns with the new multi-app ID feature. This change ensures that the correct app context is maintained when processing repository changes.

274-274: Refactor: Calculation of property changes.

The method calcPropertyChanges is now correctly scoped to handle appId and namespace, which is essential for accurate configuration management in environments with multiple apps. Ensure that this method is robust against potential edge cases where properties might not change but the environment does.
apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigRepository.java (5)
75-75: Addition of m_appId field.

The addition of the m_appId field is consistent with the PR's objective to handle multiple app IDs. This change allows the class to store the application ID, which is crucial for the new functionality.

160-160: Update to fireRepositoryChange method call.

The method now includes m_appId as an argument, which is necessary for the enhanced functionality of handling multiple app IDs. This change ensures that the application ID is considered when firing repository changes.

191-194: Use of m_appId in loadApolloConfig method.

The method now directly uses m_appId instead of calling m_configUtil.getAppId(). This change is crucial for ensuring that the correct application ID is used in configuration retrieval processes, especially in environments with multiple applications.

336-336: Modification in scheduleLongPollingRefresh method.

The method now passes m_appId along with m_namespace to the remoteConfigLongPollService.submit method. This update is essential for maintaining the functionality of long polling in the context of multiple app IDs.

93-97: Constructor modification to include appId.

The constructor now requires an appId parameter, which is a significant change. This modification aligns with the PR's goal to support multiple app IDs. It's important to ensure that all instantiations of this class across the project are updated to pass the appId parameter.

Verification successful

Constructor usage updated successfully.

All instances of the RemoteConfigRepository constructor across the codebase have been updated to include the appId parameter, as intended. No further action is required.

Instances found in RemoteConfigRepositoryTest.java and DefaultConfigFactory.java confirm the update.
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all constructor calls to `RemoteConfigRepository` match the new signature.

# Test: Search for the constructor usage. Expect: Only occurrences of the new signature.
rg --type java -A 5 $'new RemoteConfigRepository'
Length of output: 8134
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/BootstrapConfigTest.java (10)

60-61: Introduction of someAppId constant.

The addition of the someAppId constant is a positive change, promoting better maintainability and readability by centralizing the application ID used in tests. This aligns with the PR's objective to handle multiple app IDs.

96-96: Proper use of someAppId in configuration setup.

The use of someAppId in the mockConfig method call correctly reflects the PR's intent to enhance configuration flexibility by supporting multiple app IDs.

140-141: Effective use of someAppId for multiple namespace configurations.

The use of someAppId for configuring multiple namespaces in the test setup is a clear demonstration of the enhanced flexibility provided by the PR. This is a crucial aspect of supporting multiple app IDs.

176-180: Consistent use of someAppId in YAML configuration setup.

The introduction of someAppId in the preparation of YAML configuration files and its consistent use in configuring multiple namespaces enhances the test's configurability and aligns well with the PR's goals.

218-218: Correct application of someAppId in failed conditional test setup.

The use of someAppId in the configuration setup for a test scenario where the condition fails is appropriate and demonstrates the PR's intent to enhance configurability across different test scenarios.

250-250: Appropriate use of someAppId in YAML file preparation for conditional off scenario.

The use of someAppId to prepare a YAML configuration file for a test scenario with conditions turned off is consistent with the PR's goals and enhances test configurability.

284-284: Proper application of someAppId in conditional off test setup.

The use of someAppId in the configuration setup for a test scenario with conditions turned off is appropriate and demonstrates the PR's intent to enhance configurability across different test scenarios.

414-414: Effective use of someAppId in eager load enabled test setup.

The use of someAppId in the configuration setup for a test scenario with eager load enabled is appropriate and demonstrates the PR's intent to enhance configurability across different test scenarios.

353-353: Correct application of someAppId in bootstrap disabled and conditional on test setup.

The use of someAppId in the configuration setup for a test scenario where bootstrap is disabled and conditions are on is appropriate and demonstrates the PR's intent to enhance configurability across different test scenarios.

382-382: Proper application of someAppId in bootstrap disabled and conditional off test setup.

The use of someAppId in the configuration setup for a test scenario where bootstrap is disabled and conditions are off is appropriate and demonstrates the PR's intent to enhance configurability across different test scenarios.

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java (2)

78-80: Review the initialization of ConcurrentHashMap and Maps for tracking long polling and namespaces.

The use of ConcurrentHashMap for m_longPollStarted and Maps.newConcurrentMap() for m_longPollNamespaces is appropriate given the need for thread-safe operations in a potentially concurrent environment. This change aligns with the PR's goal to handle multiple app IDs effectively.

299-300: Review the assembleNamespaces method adaptation for multi-app support.

The method has been adapted to handle multiple app IDs by fetching namespaces specific to an app ID. This change is crucial for ensuring that the long polling URLs are correctly assembled with the appropriate namespaces for each app. The implementation is straightforward and aligns well with the overall enhancements made for multi-app support.

apollo-client/src/test/java/com/ctrip/framework/apollo/integration/ConfigIntegrationTest.java (1)

374-374: Approved change in timeout duration, suggest verifying test performance impact.

The increase in timeout duration from 5000 to 10000 milliseconds in the longPollFinished.get() method call is approved as it likely accommodates slower network conditions or server responses. However, it's important to verify that this change does not negatively impact the overall test performance or its effectiveness in catching issues within a reasonable time frame.

Run the following script to verify the impact on test performance:

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java (1)

272-272: Verify: Ensure correct parameters in onRepositoryChange method.

The onRepositoryChange method is called with someAppId and someNamespace as parameters. Verify that these parameters are correctly used within the method and that the method's implementation aligns with the new requirements for handling multiple app IDs.

Also applies to: 321-321

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/AbstractConfig.java (2)

457-465: Review of fireConfigChange method changes:

The addition of the appId parameter to the fireConfigChange method aligns with the PR's objectives to support multiple app IDs. The method correctly passes the appId to the InterestedConfigChangeEvent.

Suggestion:
Ensure that the appId is correctly integrated and utilized in other parts of the system where this method is called.

Line range hint 570-605: Review of calcPropertyChanges method changes:

The method now includes an appId parameter and uses it when creating ConfigChange objects. This change supports the PR's goal of handling configurations for multiple apps and ensures that changes are associated with the correct app ID.

Suggestion:
Verify that the appId is being used correctly across all instances where ConfigChange objects are created or processed.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/XmlConfigPlaceholderAutoUpdateTest.java (14)

44-44: Good practice: Centralizing configuration values.

The introduction of the someAppId constant is a positive change, enhancing maintainability by avoiding hardcoded values throughout the test cases.

Line range hint 56-68: Correct usage of the new constant in test scenarios.

The method testAutoUpdateWithOneNamespace correctly uses the someAppId constant, aligning with the PR's objectives to centralize configuration values. This change enhances the readability and maintainability of the test code.

Line range hint 91-103: Consistent application of the new constant.

The method testAutoUpdateDisabled consistently uses the someAppId constant in its configuration setup and repository change calls. This consistency is crucial for maintaining the integrity of the test suite.

123-125: Effective demonstration of configuration flexibility.

The method testAutoUpdateWithMultipleNamespaces effectively uses the someAppId constant to demonstrate the new configuration's flexibility across different namespaces. This is a key enhancement in the context of the PR's objectives.

Line range hint 168-181: Robust testing of complex scenarios.

The method testAutoUpdateWithMultipleNamespacesWithSameProperties uses the someAppId constant to robustly test the scenario where multiple namespaces are updated with the same properties. This ensures that the new configuration approach can handle complex updates effectively.

Line range hint 198-212: Proper testing of new properties integration.

The method testAutoUpdateWithNewProperties correctly uses the someAppId constant to test the integration of new properties. This aligns with the PR's objectives and demonstrates the test's adaptability to configuration changes.

Line range hint 233-247: Testing robustness against irrelevant property changes.

The method testAutoUpdateWithIrrelevantProperties effectively uses the someAppId constant to test the system's robustness against irrelevant property changes. This is crucial for ensuring that the configuration system is resilient.

Line range hint 263-274: Effective handling of property deletions.

The method testAutoUpdateWithDeletedProperties uses the someAppId constant to effectively test the configuration system's handling of property deletions. This ensures that the system can adapt to changes in configuration without issues.

293-295: Robust testing of property deletions across multiple namespaces.

The method testAutoUpdateWithMultipleNamespacesWithSamePropertiesDeleted uses the someAppId constant to robustly test the scenario where properties are deleted across multiple namespaces. This ensures that the new configuration approach can handle complex updates effectively.

Line range hint 323-334: Testing handling of deletions without default values.

The method testAutoUpdateWithDeletedPropertiesWithNoDefaultValue uses the someAppId constant to test the configuration system's handling of property deletions without default values. This scenario is important for ensuring robustness in more dynamic configurations.

Line range hint 352-364: Testing handling of type mismatches in properties.

The method testAutoUpdateWithTypeMismatch uses the someAppId constant to test the configuration system's handling of type mismatches in properties. This is crucial for ensuring that the system can adapt to incorrect data types without causing failures.

Line range hint 382-394: Highlighting limitations with constructor-injected values.

The method testAutoUpdateWithValueInjectedAsConstructorArgs uses the someAppId constant but importantly highlights a limitation of the current system where updates for constructor-injected values are not supported. This is a crucial observation for future enhancements.

Line range hint 413-425: Effective handling of both value and property updates.

The method testAutoUpdateWithValueAndProperty uses the someAppId constant to effectively test the configuration system's handling of both value and property updates. This ensures that the system can adapt to changes in configuration without issues.

Line range hint 472-501: Comprehensive testing of updates across all data types.

The method testAutoUpdateWithAllKindsOfDataTypes uses the someAppId constant to comprehensively test the configuration system's handling of updates across all data types. This is crucial for ensuring that the system can adapt to diverse configuration needs.
apollo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java (2)
595-600: Implementation of getAccessKeySecret in MockConfigUtil.

The new method getAccessKeySecret in the MockConfigUtil class is designed to return a secret based on the provided appId. This is a critical addition for testing access control based on application identifiers.

The implementation checks if the provided appId matches someAppId and returns someSecret if true, otherwise returns null. This logic is straightforward and aligns with the requirements for differentiating access based on app IDs. Ensure that this method is covered by unit tests to verify its behavior under different conditions.

149-149: Updated method signatures in test cases to include someAppId.

The method submit now correctly includes someAppId as part of its signature in all test cases, aligning with the changes in the RemoteConfigLongPollService class. This is crucial for ensuring that the tests reflect the new functionality of handling multiple app IDs.

It's important to verify that all instances where submit is called have been updated to pass someAppId. Additionally, ensure that the new parameter is being used correctly within the method implementations.

Also applies to: 200-200, 262-262, 333-333, 336-336, 404-404, 405-405, 465-465

Verification successful

Verification Successful: Updated method signatures include someAppId.

The submit method in RemoteConfigLongPollService correctly includes someAppId in its signature and uses it appropriately within the method implementation. All instances where submit is called have been updated to pass someAppId, aligning with the changes in the RemoteConfigLongPollService class.

RemoteConfigLongPollService.java: Lines 110-126
Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify all calls to `submit` include `someAppId` and check its usage within the method.

# Test: Search for the method usage. Expect: Only occurrences with `someAppId`.
rg --type java -A 5 $'submit'
Length of output: 17743

Script:
#!/bin/bash
# Extract the implementation of the `submit` method from `RemoteConfigLongPollService` to verify the usage of `someAppId`.
ast-grep --lang java --pattern $'class RemoteConfigLongPollService {
  $$$
  public boolean submit(String appId, String namespace, RemoteConfigRepository remoteConfigRepository) {
    $$$
  }
  $$$
}'
Length of output: 201

Script:
#!/bin/bash
# Adjust the pattern to more flexibly capture the `submit` method implementation in `RemoteConfigLongPollService`.
ast-grep --lang java --pattern $'public boolean submit(String appId, String namespace, RemoteConfigRepository remoteConfigRepository) {
  $$$
}'
Length of output: 2670
apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java (21)

63-63: Introduction of someAppId constant.

The introduction of the someAppId constant is a positive change as it centralizes the application ID, enhancing maintainability and readability. This change aligns with the PR's objective to support multiple app IDs.

74-74: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

82-82: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

95-95: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

111-111: Refactor to include someAppId in mockConfigFile.

The modification to include someAppId as an argument in mockConfigFile is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

127-127: Refactor to include someAppId in mockConfigFile.

The modification to include someAppId as an argument in mockConfigFile is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

139-139: Refactor to include someAppId in mockConfig for multiple namespaces.

The modification to include someAppId as an argument in mockConfig for both ConfigConsts.NAMESPACE_APPLICATION and FX_APOLLO_NAMESPACE is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID for multiple namespaces.

Also applies to: 143-143

161-161: Refactor to include someAppId in mockConfigFile and mockConfig.

The modification to include someAppId as an argument in both mockConfigFile and mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID for multiple configurations.

Also applies to: 165-165

180-180: Refactor to include someAppId in mockConfig for multiple namespaces.

The modification to include someAppId as an argument in mockConfig for both ConfigConsts.NAMESPACE_APPLICATION and FX_APOLLO_NAMESPACE is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID for multiple namespaces.

Also applies to: 184-184

199-199: Refactor to include someAppId in mockConfig for multiple namespaces.

The modification to include someAppId as an argument in mockConfig for both ConfigConsts.NAMESPACE_APPLICATION and FX_APOLLO_NAMESPACE is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID for multiple namespaces.

Also applies to: 203-203

217-217: Refactor to include someAppId in mockConfig for multiple namespaces.

The modification to include someAppId as an argument in mockConfig for both ConfigConsts.NAMESPACE_APPLICATION and FX_APOLLO_NAMESPACE is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID for multiple namespaces.

Also applies to: 221-221

235-235: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

254-254: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

276-276: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

297-297: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

315-315: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

338-338: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

360-360: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

378-378: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

409-409: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

428-428: Refactor to include someAppId in mockConfig.

The modification to include someAppId as an argument in mockConfig is consistent with the PR's objectives and enhances the test's clarity by explicitly specifying the app ID.

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java (4)

74-74: Addition of someAppId variable.

The addition of the someAppId variable aligns with the PR's objective to handle multiple app IDs. This change is crucial for testing configurations specific to different applications.

95-95: Initialization of someAppId and someNamespace.

Proper initialization of someAppId and someNamespace in the setUp method ensures that these variables are correctly set up before each test. This is a necessary change to support the new functionality.

142-142: File path construction using someAppId and someNamespace.

The construction of file paths using both someAppId and someNamespace is a significant change that supports the feature of handling configurations for multiple apps. This approach allows for more granular management of configuration files.

719-719: Modification in ConfigChangeEvent constructor.

The update to include someAppId in the ConfigChangeEvent constructor is crucial for tracking configuration changes per app ID. This change supports the PR's goal of enhancing configuration management by allowing more detailed tracking and management of changes.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigAnnotationTest.java (19)

81-81: Introduction of someAppId variable.

The introduction of the someAppId static variable is a key part of supporting multiple app IDs. This change aligns with the PR's objectives to enhance configuration capabilities.

116-117: Correct use of someAppId in configuration mocking.

The method correctly uses the someAppId variable to mock configurations, ensuring that the configurations are associated with the correct application ID. This is a crucial part of supporting multiple app IDs.

Also applies to: 119-119

135-135: Consistent use of someAppId for specific application ID testing.

The method correctly uses the someAppId variable to mock configurations for testing configurations with a wrong field type, aligning with the PR's objectives.

145-147: Proper use of someAppId in inheritance testing.

The method correctly uses the someAppId variable to mock configurations in a scenario involving inheritance, demonstrating the flexibility and robustness of the new feature.

161-164: Effective use of someAppId in expression resolution testing.

The method effectively uses the someAppId variable to mock configurations for testing the resolution of expressions, aligning with the PR's objectives to enhance configuration capabilities.

Line range hint 176-185: Accurate use of someAppId in system property expression resolution testing.

The method accurately uses the someAppId variable to mock configurations for testing the resolution of expressions from system properties, demonstrating the robustness of the new feature in handling complex scenarios.

197-198: Proper handling of someAppId in unresolved value testing.

The method properly uses the someAppId variable to mock configurations for testing the handling of unresolved values, aligning with the PR's objectives to enhance configuration capabilities.

212-213: Effective use of someAppId in configuration change listener testing.

The method effectively uses the someAppId variable to mock configurations and set up listeners for testing the configuration change listener functionalities, demonstrating the robustness of the new feature in handling dynamic scenarios.

266-266: Consistent use of someAppId for specific application ID testing in error scenarios.

The method correctly uses the someAppId variable to mock configurations for testing configurations with a wrong parameter type, aligning with the PR's objectives.

275-275: Consistent use of someAppId for specific application ID testing in error scenarios.

The method correctly uses the someAppId variable to mock configurations for testing configurations with a wrong parameter count, aligning with the PR's objectives.

285-286: Proper use of someAppId in inheritance testing for configuration change listeners.

The method correctly uses the someAppId variable to mock configurations and set up listeners in a scenario involving inheritance, demonstrating the flexibility and robustness of the new feature.

342-343: Effective use of someAppId in configuration change listener testing with interested keys.

The method effectively uses the someAppId variable to mock configurations and set up listeners for testing the configuration change listener functionalities with interested keys, demonstrating the robustness of the new feature in handling dynamic scenarios.

374-374: Accurate use of someAppId in configuration change listener testing with interested key prefixes.

The method accurately uses the someAppId variable to mock configurations for testing the configuration change listener functionalities with interested key prefixes, aligning with the PR's objectives to enhance configuration capabilities.

400-407: Effective use of someAppId in dynamic testing of configuration change listeners with interested key prefixes.

The method effectively uses the someAppId variable to mock configurations and set up listeners for testing the configuration change listener functionalities with interested key prefixes in a dynamic scenario, demonstrating the robustness of the new feature in handling changes.

Line range hint 439-450: Accurate use of someAppId in configuration change listener testing with a YAML file.

The method accurately uses the someAppId variable to prepare a YAML configuration file and set up listeners for testing the configuration change listener functionalities with a YAML file, aligning with the PR's objectives to enhance configuration capabilities.

464-467: Effective use of someAppId in expression resolution testing for configuration change listeners.

The method effectively uses the someAppId variable to mock configurations for testing the resolution of expressions in configuration change listeners, demonstrating the robustness of the new feature in handling complex scenarios.

502-508: Effective use of someAppId in configuration change listener testing with comma-separated namespaces.

The method effectively uses the someAppId variable to mock configurations for multiple namespaces, demonstrating the robustness of the new feature in handling scenarios involving multiple configurations.

527-533: Accurate use of someAppId in configuration change listener testing with comma-separated namespaces merged with ones in value.

The method accurately uses the someAppId variable to mock configurations for multiple namespaces, aligning with the PR's objectives to enhance configuration capabilities in scenarios involving complex configurations.

548-553: Accurate use of someAppId in system property expression resolution testing for configuration change listeners.

The method accurately uses the someAppId variable to mock configurations for testing the resolution of expressions from system properties in configuration change listeners, aligning with the PR's objectives to enhance configuration capabilities.

apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java (16)

55-55: Introduction of someAppId constant.

The introduction of the someAppId constant is a positive change as it centralizes the application ID, enhancing maintainability and reducing the risk of errors associated with hardcoded strings. This change aligns well with the PR's objective to support multiple app IDs.

74-74: Consistent use of someAppId in testAutoUpdateWithOneNamespace.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 129-129

101-101: Consistent use of someAppId in testAutoUpdateWithOneYamlFile.

The method correctly uses the someAppId constant in the prepareYamlConfigFile call, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 161-161

129-129: Consistent use of someAppId in testAutoUpdateWithValueAndXmlProperty.

The method correctly uses the someAppId constant in the prepareConfig call, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 199-199

229-229: Consistent use of someAppId in testAutoUpdateWithMultipleNamespaces.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 241-241

270-270: Consistent use of someAppId in testAutoUpdateWithMultipleNamespacesWithSameProperties.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 283-283

298-298: Consistent use of someAppId in testAutoUpdateWithMultipleNamespacesWithSamePropertiesWithYamlFile.

The method correctly uses the someAppId constant in both prepareYamlConfigFile and prepareConfig calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 303-303

328-328: Consistent use of someAppId in testAutoUpdateWithNewProperties.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 340-340

354-354: Consistent use of someAppId in testAutoUpdateWithNewPropertiesWithYamlFile.

The method correctly uses the someAppId constant in the prepareYamlConfigFile call, aligning with the PR's objectives and enhancing code maintainability.

385-385: Consistent use of someAppId in testAutoUpdateWithIrrelevantProperties.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 397-397

413-413: Consistent use of someAppId in testAutoUpdateWithDeletedProperties.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 424-424

437-437: Consistent use of someAppId in testAutoUpdateWithDeletedPropertiesWithYamlFile.

The method correctly uses the someAppId constant in the prepareYamlConfigFile call, aligning with the PR's objectives and enhancing code maintainability.

Also applies to: 447-447

465-465: Consistent use of someAppId in testAutoUpdateWithMultipleNamespacesWithSamePropertiesDeleted.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

494-494: Consistent use of someAppId in testAutoUpdateWithDeletedPropertiesWithNoDefaultValue.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

523-523: Consistent use of someAppId in testAutoUpdateWithTypeMismatch.

The method correctly uses the someAppId constant in both prepareConfig and onRepositoryChange calls, aligning with the PR's objectives and enhancing code maintainability.

549-549: Consistent use of someAppId in testAutoUpdateWithTypeMismatchWithYamlFile.

The method correctly uses the someAppId constant in the prepareYamlConfigFile call, aligning with the PR's objectives and enhancing code maintainability.

apollo-client/src/main/java/com/ctrip/framework/apollo/spi/ConfigFactoryManager.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/TxtConfigFileTest.java

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigManagerTest.java

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

coderabbitai · 2024-09-05T10:10:07Z

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

+    return addNamespaces(ApolloInjector.getInstance(ConfigUtil.class).getAppId(), namespaces, order);
+  }
+
+  public static boolean addNamespaces(String appId, Collection<String> namespaces, int order) {
+    Multimap<String, String> appNamespaceNames = LinkedHashMultimap.create();
+    appNamespaceNames.putAll(appId, namespaces);
+    return APP_NAMESPACE_NAMES.put(order, appNamespaceNames);


Optimization: Avoid Unnecessary Creation of Multimap Instances

The method addNamespaces creates a new Multimap instance each time it is called. This could lead to performance issues if the method is called frequently with the same appId. Consider caching or reusing Multimap instances to optimize performance.

Consider modifying the method to check if a Multimap for the given appId already exists and reuse it:

- Multimap<String, String> appNamespaceNames = LinkedHashMultimap.create(); + Multimap<String, String> appNamespaceNames = APP_NAMESPACE_NAMES.get(order).get(appId); + if (appNamespaceNames == null) { + appNamespaceNames = LinkedHashMultimap.create(); + APP_NAMESPACE_NAMES.put(order, appNamespaceNames); + } appNamespaceNames.putAll(appId, namespaces);

Committable suggestion was skipped due to low confidence.

This suggestion is valid; otherwise, we would override the app's namespaces.

@nobodyiam, glad to hear that the suggestion is helpful!

(_/)
(•‿•)
/ \

...lo-client/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigRepositoryTest.java

coderabbitai · 2024-09-05T10:10:08Z

...ient/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java

+    someAppId = "someAppId";
+    someCluster = "someCluster";
+
+    System.setProperty(ApolloClientSystemConsts.APP_ID, someAppId);
+


Initialization of someAppId and someCluster in setUp method.

The addition of someAppId and someCluster in the setUp method is a good practice as it centralizes the initialization of these variables which are used across multiple test methods. This ensures that each test is performed with a consistent application context.

However, the commented-out code block (lines 103-106) should be removed if it is no longer needed to keep the codebase clean and maintainable.

Also applies to: 103-106

apollo-client/src/test/java/com/ctrip/framework/apollo/internals/DefaultConfigTest.java

nobodyiam · 2024-09-14T08:48:29Z

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/DefaultConfig.java

-  private Properties loadFromResource(String namespace) {
-    String name = String.format("META-INF/config/%s.properties", namespace);
+  private Properties loadFromResource(String appId, String namespace) {
+    String name = String.format("META-INF/config/%s+%s.properties", appId, namespace);


Do you plan to change the logic so that it's backward compatible?

nobodyiam · 2024-09-14T08:50:25Z

...o-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java

-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadLocalRandom;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;


It's better to expand the imports.

nobodyiam · 2024-09-14T08:54:06Z

apollo-client/src/main/java/com/ctrip/framework/apollo/internals/SimpleConfig.java

+  private static ConfigUtil m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);
+


This is not needed.

Suggested change

private static ConfigUtil m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);

nobodyiam · 2024-09-14T09:14:17Z

...-client/src/main/java/com/ctrip/framework/apollo/spring/config/PropertySourcesProcessor.java

+    return addNamespaces(ApolloInjector.getInstance(ConfigUtil.class).getAppId(), namespaces, order);
+  }
+
+  public static boolean addNamespaces(String appId, Collection<String> namespaces, int order) {
+    Multimap<String, String> appNamespaceNames = LinkedHashMultimap.create();
+    appNamespaceNames.putAll(appId, namespaces);
+    return APP_NAMESPACE_NAMES.put(order, appNamespaceNames);


This suggestion is valid; otherwise, we would override the app's namespaces.

nobodyiam · 2024-09-14T09:19:48Z

...o-client/src/main/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollService.java

+  public boolean submit(String appId, String namespace, RemoteConfigRepository remoteConfigRepository) {
+    Multimap<String, RemoteConfigRepository> repositoryMultimap = m_longPollNamespaces.get(
+        appId);
+    boolean result = false;
+    if(repositoryMultimap == null){
+      repositoryMultimap = Multimaps.synchronizedSetMultimap(HashMultimap.<String, RemoteConfigRepository>create());
+      result = repositoryMultimap.put(namespace, remoteConfigRepository);
+      m_longPollNamespaces.put(appId, repositoryMultimap);
+    }else{
+      result = repositoryMultimap.put(namespace, remoteConfigRepository);
    }
-    return added;
+    m_notifications.put(appId, namespace, INIT_NOTIFICATION_ID);
+    if (m_longPollStarted.get(appId) == null) {
+      startLongPolling(appId);
+    }
+    return result;


This suggestion is also valid.

nobodyiam · 2024-09-14T09:26:44Z

...ient/src/test/java/com/ctrip/framework/apollo/internals/RemoteConfigLongPollServiceTest.java

+/*    Field field = PropertySourcesProcessor.class.getDeclaredField("configUtil");
+    field.setAccessible(true);
+    field.set(null, ApolloInjector.getInstance(ConfigUtil.class));*/
+


Why are these lines commented out?

coderabbitai bot reviewed Jul 10, 2024

View reviewed changes

coderabbitai bot reviewed Jul 16, 2024

View reviewed changes

nobodyiam requested changes Jul 20, 2024

View reviewed changes

coderabbitai bot reviewed Aug 14, 2024

View reviewed changes

nobodyiam reviewed Aug 17, 2024

View reviewed changes

coderabbitai bot reviewed Aug 26, 2024

View reviewed changes

nobodyiam requested changes Aug 31, 2024

View reviewed changes

coderabbitai bot reviewed Sep 5, 2024

View reviewed changes

merge branch

38469c4

TerryLam2010 force-pushed the 20240703-apollo-multiple-appid branch from 132e2b7 to 38469c4 Compare September 5, 2024 10:03

coderabbitai bot reviewed Sep 5, 2024

View reviewed changes

Merge branch 'main' into 20240703-apollo-multiple-appid

67a7da7

nobodyiam requested changes Sep 14, 2024

View reviewed changes

		private static ConfigUtil m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);

add multiple appId,pull other appId config into config. #70

Are you sure you want to change the base?

add multiple appId,pull other appId config into config. #70

Conversation

TerryLam2010 commented Jul 10, 2024 • edited by coderabbitai bot Loading

What's the purpose of this PR

Which issue(s) this PR fixes:

Summary by CodeRabbit

github-actions bot commented Jul 10, 2024 • edited Loading

coderabbitai bot commented Jul 10, 2024 • edited Loading

Walkthrough

Changes

Sequence Diagrams

Configuration Retrieval Flow

Configuration Change Notification Flow

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

nobodyiam commented Jul 14, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

TerryLam2010 commented Jul 16, 2024

nobodyiam commented Jul 20, 2024

Anilople commented Aug 12, 2024

Rawven commented Aug 12, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

TerryLam2010 commented Aug 14, 2024

nobodyiam left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

nobodyiam commented Aug 31, 2024

nobodyiam left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024 • edited Loading

Choose a reason for hiding this comment

coderabbitai bot Sep 14, 2024

Choose a reason for hiding this comment

coderabbitai bot Sep 5, 2024

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024 • edited Loading

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024 • edited Loading

Choose a reason for hiding this comment

nobodyiam Sep 14, 2024

Choose a reason for hiding this comment

TerryLam2010 commented Jul 10, 2024 •

edited by coderabbitai bot

Loading

github-actions bot commented Jul 10, 2024 •

edited

Loading

coderabbitai bot commented Jul 10, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

coderabbitai bot Sep 5, 2024 •

edited

Loading

nobodyiam Sep 14, 2024 •

edited

Loading

nobodyiam Sep 14, 2024 •

edited

Loading

nobodyiam Sep 14, 2024 •

edited

Loading