-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support incremental configuration synchronization client #5325
Changes from all commits
4e8a699
26a0e1b
2dd9352
b13968c
24bda1d
cf5145f
8cafda2
72676c0
065a092
e69f098
a6a412a
07bb41c
a4b356c
5e37f55
8512776
5a76da9
a4a2d05
04f50ac
1b20e4e
4f06613
26e8db5
56df3a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
*/ | ||
package com.ctrip.framework.apollo.configservice.controller; | ||
|
||
import com.ctrip.framework.apollo.biz.config.BizConfig; | ||
import com.ctrip.framework.apollo.biz.entity.Release; | ||
import com.ctrip.framework.apollo.common.entity.AppNamespace; | ||
import com.ctrip.framework.apollo.common.utils.WebUtils; | ||
|
@@ -26,12 +27,16 @@ | |
import com.ctrip.framework.apollo.core.ConfigConsts; | ||
import com.ctrip.framework.apollo.core.dto.ApolloConfig; | ||
import com.ctrip.framework.apollo.core.dto.ApolloNotificationMessages; | ||
import com.ctrip.framework.apollo.core.dto.ConfigurationChange; | ||
import com.ctrip.framework.apollo.core.enums.ConfigSyncType; | ||
import com.ctrip.framework.apollo.tracer.Tracer; | ||
import com.google.common.base.Strings; | ||
import com.google.common.collect.Lists; | ||
import com.google.common.collect.Maps; | ||
import com.google.common.collect.Sets; | ||
import com.google.gson.Gson; | ||
import com.google.gson.reflect.TypeToken; | ||
import java.util.regex.Pattern; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
|
@@ -42,6 +47,9 @@ | |
import javax.servlet.http.HttpServletResponse; | ||
import java.io.IOException; | ||
import java.lang.reflect.Type; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.LinkedHashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
|
@@ -59,21 +67,25 @@ public class ConfigController { | |
private final NamespaceUtil namespaceUtil; | ||
private final InstanceConfigAuditUtil instanceConfigAuditUtil; | ||
private final Gson gson; | ||
private final BizConfig bizConfig; | ||
|
||
|
||
private static final Type configurationTypeReference = new TypeToken<Map<String, String>>() { | ||
}.getType(); | ||
}.getType(); | ||
|
||
public ConfigController( | ||
final ConfigService configService, | ||
final AppNamespaceServiceWithCache appNamespaceService, | ||
final NamespaceUtil namespaceUtil, | ||
final InstanceConfigAuditUtil instanceConfigAuditUtil, | ||
final Gson gson) { | ||
final Gson gson, | ||
final BizConfig bizConfig) { | ||
this.configService = configService; | ||
this.appNamespaceService = appNamespaceService; | ||
this.namespaceUtil = namespaceUtil; | ||
this.instanceConfigAuditUtil = instanceConfigAuditUtil; | ||
this.gson = gson; | ||
this.bizConfig = bizConfig; | ||
} | ||
|
||
@GetMapping(value = "/{appId}/{clusterName}/{namespace:.+}") | ||
|
@@ -132,10 +144,10 @@ public ApolloConfig queryConfig(@PathVariable String appId, @PathVariable String | |
|
||
auditReleases(appId, clusterName, dataCenter, clientIp, releases); | ||
|
||
String mergedReleaseKey = releases.stream().map(Release::getReleaseKey) | ||
.collect(Collectors.joining(ConfigConsts.CLUSTER_NAMESPACE_SEPARATOR)); | ||
String latestMergedReleaseKey = releases.stream().map(Release::getReleaseKey) | ||
.collect(Collectors.joining(ConfigConsts.CLUSTER_NAMESPACE_SEPARATOR)); | ||
|
||
if (mergedReleaseKey.equals(clientSideReleaseKey)) { | ||
if (latestMergedReleaseKey.equals(clientSideReleaseKey)) { | ||
// Client side configuration is the same with server side, return 304 | ||
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); | ||
Tracer.logEvent("Apollo.Config.NotModified", | ||
|
@@ -144,8 +156,44 @@ public ApolloConfig queryConfig(@PathVariable String appId, @PathVariable String | |
} | ||
|
||
ApolloConfig apolloConfig = new ApolloConfig(appId, appClusterNameLoaded, originalNamespace, | ||
mergedReleaseKey); | ||
apolloConfig.setConfigurations(mergeReleaseConfigurations(releases)); | ||
latestMergedReleaseKey); | ||
|
||
Map<String, String> latestConfigurations = mergeReleaseConfigurations(releases); | ||
|
||
if (bizConfig.isConfigServiceIncrementalChangeEnabled()) { | ||
LinkedHashSet<String> clientSideReleaseKeys = Sets.newLinkedHashSet( | ||
Arrays.stream( | ||
clientSideReleaseKey.split(Pattern.quote(ConfigConsts.CLUSTER_NAMESPACE_SEPARATOR))) | ||
.collect(Collectors.toList())); | ||
|
||
Map<String, Release> historyReleases = configService.findReleasesByReleaseKeys( | ||
clientSideReleaseKeys); | ||
//find history releases | ||
if (historyReleases != null) { | ||
//order by clientSideReleaseKeys | ||
List<Release> historyReleasesWithOrder = new ArrayList<>(); | ||
for (String item : clientSideReleaseKeys) { | ||
Release release = historyReleases.get(item); | ||
if (release != null) { | ||
historyReleasesWithOrder.add(release); | ||
} | ||
} | ||
|
||
Map<String, String> historyConfigurations = mergeReleaseConfigurations | ||
(historyReleasesWithOrder); | ||
|
||
List<ConfigurationChange> configurationChanges = configService.calcConfigurationChanges( | ||
latestConfigurations, historyConfigurations); | ||
|
||
apolloConfig.setConfigurationChanges(configurationChanges); | ||
|
||
apolloConfig.setConfigSyncType(ConfigSyncType.INCREMENTAL_SYNC.getValue()); | ||
return apolloConfig; | ||
} | ||
|
||
} | ||
|
||
apolloConfig.setConfigurations(latestConfigurations); | ||
Comment on lines
+163
to
+196
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Handle invalid or missing releaseKeys more gracefully.
|
||
|
||
Tracer.logEvent("Apollo.Config.Found", assembleKey(appId, appClusterNameLoaded, | ||
originalNamespace, dataCenter)); | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -21,9 +21,16 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.ctrip.framework.apollo.core.ConfigConsts; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.ctrip.framework.apollo.core.dto.ApolloNotificationMessages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.ctrip.framework.apollo.core.dto.ConfigurationChange; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.google.common.base.Strings; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.google.common.collect.Lists; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import com.google.common.collect.Sets; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.HashMap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.Map; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.Objects; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import java.util.Set; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* @author Jason Song([email protected]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -93,6 +100,52 @@ private Release findRelease(String clientAppId, String clientIp, String clientLa | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return release; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public List<ConfigurationChange> calcConfigurationChanges( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Map<String, String> latestReleaseConfigurations, Map<String, String> historyConfigurations) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (latestReleaseConfigurations == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
latestReleaseConfigurations = new HashMap<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (historyConfigurations == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
historyConfigurations = new HashMap<>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Set<String> previousKeys = historyConfigurations.keySet(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Set<String> currentKeys = latestReleaseConfigurations.keySet(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Set<String> commonKeys = Sets.intersection(previousKeys, currentKeys); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Set<String> newKeys = Sets.difference(currentKeys, commonKeys); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Set<String> removedKeys = Sets.difference(previousKeys, commonKeys); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
List<ConfigurationChange> changes = Lists.newArrayList(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (String newKey : newKeys) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
changes.add(new ConfigurationChange(newKey, latestReleaseConfigurations.get(newKey), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"ADDED")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (String removedKey : removedKeys) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
changes.add(new ConfigurationChange(removedKey, null, "DELETED")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (String commonKey : commonKeys) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
String previousValue = historyConfigurations.get(commonKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
String currentValue = latestReleaseConfigurations.get(commonKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (com.google.common.base.Objects.equal(previousValue, currentValue)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
changes.add( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
new ConfigurationChange(commonKey, currentValue, "MODIFIED")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return changes; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+103
to
+142
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance robustness and maintainability of configuration change calculation. Several improvements can be made to the implementation:
Here's the suggested implementation: + private static final String CHANGE_TYPE_ADDED = "ADDED";
+ private static final String CHANGE_TYPE_DELETED = "DELETED";
+ private static final String CHANGE_TYPE_MODIFIED = "MODIFIED";
public List<ConfigurationChange> calcConfigurationChanges(
Map<String, String> latestReleaseConfigurations, Map<String, String> historyConfigurations) {
if (latestReleaseConfigurations == null) {
latestReleaseConfigurations = new HashMap<>();
}
if (historyConfigurations == null) {
historyConfigurations = new HashMap<>();
}
Set<String> previousKeys = historyConfigurations.keySet();
Set<String> currentKeys = latestReleaseConfigurations.keySet();
Set<String> commonKeys = Sets.intersection(previousKeys, currentKeys);
Set<String> newKeys = Sets.difference(currentKeys, commonKeys);
Set<String> removedKeys = Sets.difference(previousKeys, commonKeys);
List<ConfigurationChange> changes = Lists.newArrayList();
for (String newKey : newKeys) {
+ String newValue = latestReleaseConfigurations.get(newKey);
+ if (newValue != null) {
- changes.add(new ConfigurationChange(newKey, latestReleaseConfigurations.get(newKey),
- "ADDED"));
+ changes.add(new ConfigurationChange(newKey, newValue, CHANGE_TYPE_ADDED));
+ }
}
for (String removedKey : removedKeys) {
- changes.add(new ConfigurationChange(removedKey, null, "DELETED"));
+ changes.add(new ConfigurationChange(removedKey, null, CHANGE_TYPE_DELETED));
}
for (String commonKey : commonKeys) {
String previousValue = historyConfigurations.get(commonKey);
String currentValue = latestReleaseConfigurations.get(commonKey);
- if (com.google.common.base.Objects.equal(previousValue, currentValue)) {
+ if (java.util.Objects.equals(previousValue, currentValue)) {
continue;
}
changes.add(
- new ConfigurationChange(commonKey, currentValue, "MODIFIED"));
+ new ConfigurationChange(commonKey, currentValue, CHANGE_TYPE_MODIFIED));
}
return changes;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public Map<String, Release> findReleasesByReleaseKeys(Set<String> releaseKeys) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+144
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return empty map instead of null. Returning null violates the interface contract and forces unnecessary null checks in clients. Return an empty map instead. @Override
public Map<String, Release> findReleasesByReleaseKeys(Set<String> releaseKeys) {
- return null;
+ return new HashMap<>();
} 📝 Committable suggestion
Suggested change
Comment on lines
+144
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implementation required: findReleasesByReleaseKeys returns null. The method should not return null as it's part of the interface contract. Implement proper release lookup logic. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
* Find active release by id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
/** | ||
* @author Jason Song([email protected]) | ||
*/ | ||
public interface ConfigService extends ReleaseMessageListener { | ||
public interface ConfigService extends ReleaseMessageListener, IncrementalSyncConfigService { | ||
|
||
/** | ||
* Load config | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add null check and documentation.
Consider these improvements:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Actions: build
[warning] Uses unchecked or unsafe operations