Skip to content

Commit

Permalink
refactor(apollo-biz): Use constructor injection instead of field inje…
Browse files Browse the repository at this point in the history
…ction (#4826)

* add tech-support-qq-4.png

* Update README.md

* Enhance the user experience in the scenario of submitting duplicate keys

* Modify the key-value conflict exception prompt, adjust the code style

* refactor(apollo-biz): Use constructor injection instead of field injection

* [Clean code] Remove unused imports

* Correct the test semantics of the test case in ItemSetControllerTest (#4823)

* add tech-support-qq-4.png

* Update README.md

* Enhance the user experience in the scenario of submitting duplicate keys

* Modify the key-value conflict exception prompt, adjust the code style

* test(apollo-biz): Correct the test semantics of the test case in ItemSetControllerTest

* test(apollo-biz): Optimize the test case for ItemSetControllerTest

* test(apollo-biz): Optimize the test case for ItemSetControllerTest

* test(apollo-biz): Optimize the test case for ItemSetControllerTest

---------

Co-authored-by: Jason Song <[email protected]>

* Refactor the code related to ReleaseMessage (#4822)

* add tech-support-qq-4.png

* Update README.md

* Enhance the user experience in the scenario of submitting duplicate keys

* Modify the key-value conflict exception prompt, adjust the code style

* refactor(apollo-biz): Refactor the code related to ReleaseMessage

* refactor(apollo-biz): Refactor the code related to ReleaseMessage

* test(apollo-admin): Optimize the test case

* test(apollo-admin): Optimize the test case

---------

Co-authored-by: Jason Song <[email protected]>

* refactor(apollo-biz): Use constructor injection instead of field injection

* chore(apollo-portal): Adjusting the code style

---------

Co-authored-by: Jason Song <[email protected]>
Co-authored-by: ZhangJian He <[email protected]>
  • Loading branch information
3 people authored Apr 4, 2023
1 parent 11303c7 commit 2800a00
Show file tree
Hide file tree
Showing 25 changed files with 188 additions and 118 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.util.CollectionUtils;

import java.util.List;
Expand All @@ -60,10 +59,8 @@ public class GrayReleaseRulesHolder implements ReleaseMessageListener, Initializ
private static final Logger logger = LoggerFactory.getLogger(GrayReleaseRulesHolder.class);
private static final Joiner STRING_JOINER = Joiner.on(ConfigConsts.CLUSTER_NAMESPACE_SEPARATOR);

@Autowired
private GrayReleaseRuleRepository grayReleaseRuleRepository;
@Autowired
private BizConfig bizConfig;
private final GrayReleaseRuleRepository grayReleaseRuleRepository;
private final BizConfig bizConfig;

private int databaseScanInterval;
private ScheduledExecutorService executorService;
Expand All @@ -74,7 +71,10 @@ public class GrayReleaseRulesHolder implements ReleaseMessageListener, Initializ
//an auto increment version to indicate the age of rules
private AtomicLong loadVersion;

public GrayReleaseRulesHolder() {
public GrayReleaseRulesHolder(final GrayReleaseRuleRepository grayReleaseRuleRepository,
final BizConfig bizConfig) {
this.grayReleaseRuleRepository = grayReleaseRuleRepository;
this.bizConfig = bizConfig;
loadVersion = new AtomicLong();
grayReleaseRuleCache = Multimaps.synchronizedSetMultimap(
TreeMultimap.create(String.CASE_INSENSITIVE_ORDER, Ordering.natural()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.util.CollectionUtils;

import com.ctrip.framework.apollo.biz.config.BizConfig;
Expand All @@ -46,17 +45,18 @@
public class ReleaseMessageScanner implements InitializingBean {
private static final Logger logger = LoggerFactory.getLogger(ReleaseMessageScanner.class);
private static final int missingReleaseMessageMaxAge = 10; // hardcoded to 10, could be configured via BizConfig if necessary
@Autowired
private BizConfig bizConfig;
@Autowired
private ReleaseMessageRepository releaseMessageRepository;
private final BizConfig bizConfig;
private final ReleaseMessageRepository releaseMessageRepository;
private int databaseScanInterval;
private final List<ReleaseMessageListener> listeners;
private final ScheduledExecutorService executorService;
private final Map<Long, Integer> missingReleaseMessages; // missing release message id => age counter
private long maxIdScanned;

public ReleaseMessageScanner() {
public ReleaseMessageScanner(final BizConfig bizConfig,
final ReleaseMessageRepository releaseMessageRepository) {
this.bizConfig = bizConfig;
this.releaseMessageRepository = releaseMessageRepository;
listeners = Lists.newCopyOnWriteArrayList();
executorService = Executors.newScheduledThreadPool(1, ApolloThreadFactory
.create("ReleaseMessageScanner", true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,19 @@ public class BizDBPropertySource extends RefreshablePropertySource {

private static final Logger logger = LoggerFactory.getLogger(BizDBPropertySource.class);

@Autowired
private ServerConfigRepository serverConfigRepository;
private final ServerConfigRepository serverConfigRepository;

public BizDBPropertySource(String name, Map<String, Object> source) {
public BizDBPropertySource(final String name,
final Map<String, Object> source,
final ServerConfigRepository serverConfigRepository) {
super(name, source);
this.serverConfigRepository = serverConfigRepository;
}

public BizDBPropertySource() {
@Autowired
public BizDBPropertySource(final ServerConfigRepository serverConfigRepository) {
super("DBConfig", Maps.newConcurrentMap());
this.serverConfigRepository = serverConfigRepository;
}

String getCurrentDataCenter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package com.ctrip.framework.apollo.biz.config;

import com.ctrip.framework.apollo.biz.repository.ServerConfigRepository;
import com.ctrip.framework.apollo.biz.service.BizDBPropertySource;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -36,12 +37,14 @@ public class BizConfigTest {

@Mock
private ConfigurableEnvironment environment;
@Mock
private ServerConfigRepository serverConfigRepository;

private BizConfig bizConfig;

@Before
public void setUp() throws Exception {
bizConfig = new BizConfig(new BizDBPropertySource());
bizConfig = new BizConfig(new BizDBPropertySource(serverConfigRepository));
ReflectionTestUtils.setField(bizConfig, "environment", environment);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.test.util.ReflectionTestUtils;

import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -64,11 +63,7 @@ public class GrayReleaseRulesHolderTest {

@Before
public void setUp() throws Exception {
grayReleaseRulesHolder = spy(new GrayReleaseRulesHolder());
ReflectionTestUtils.setField(grayReleaseRulesHolder, "bizConfig",
bizConfig);
ReflectionTestUtils.setField(grayReleaseRulesHolder, "grayReleaseRuleRepository",
grayReleaseRuleRepository);
grayReleaseRulesHolder = spy(new GrayReleaseRulesHolder(grayReleaseRuleRepository, bizConfig));
idCounter = new AtomicLong();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.springframework.test.util.ReflectionTestUtils;

import java.util.concurrent.TimeUnit;

Expand All @@ -52,10 +51,7 @@ public class ReleaseMessageScannerTest extends AbstractUnitTest {

@Before
public void setUp() throws Exception {
releaseMessageScanner = new ReleaseMessageScanner();
ReflectionTestUtils
.setField(releaseMessageScanner, "releaseMessageRepository", releaseMessageRepository);
ReflectionTestUtils.setField(releaseMessageScanner, "bizConfig", bizConfig);
releaseMessageScanner = new ReleaseMessageScanner(bizConfig, releaseMessageRepository);
databaseScanInterval = 100; //100 ms
when(bizConfig.releaseMessageScanIntervalInMilli()).thenReturn(databaseScanInterval);
releaseMessageScanner.afterPropertiesSet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.springframework.test.util.ReflectionTestUtils;

import java.util.List;

Expand All @@ -55,8 +54,7 @@ public class BizDBPropertySourceTest extends AbstractUnitTest {

@Before
public void initTestData() {
propertySource = spy(new BizDBPropertySource());
ReflectionTestUtils.setField(propertySource, "serverConfigRepository", serverConfigRepository);
propertySource = spy(new BizDBPropertySource(serverConfigRepository));

List<ServerConfig> configs = Lists.newLinkedList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
import com.ctrip.framework.apollo.biz.config.BizConfig;
import com.ctrip.framework.apollo.biz.grayReleaseRule.GrayReleaseRulesHolder;
import com.ctrip.framework.apollo.biz.message.ReleaseMessageScanner;
import com.ctrip.framework.apollo.biz.repository.GrayReleaseRuleRepository;
import com.ctrip.framework.apollo.biz.repository.ReleaseMessageRepository;
import com.ctrip.framework.apollo.biz.service.ReleaseMessageService;
import com.ctrip.framework.apollo.biz.service.ReleaseService;
import com.ctrip.framework.apollo.configservice.controller.ConfigFileController;
import com.ctrip.framework.apollo.configservice.controller.NotificationController;
import com.ctrip.framework.apollo.configservice.controller.NotificationControllerV2;
Expand All @@ -40,22 +44,32 @@
public class ConfigServiceAutoConfiguration {

private final BizConfig bizConfig;
private final ReleaseService releaseService;
private final ReleaseMessageService releaseMessageService;
private final GrayReleaseRuleRepository grayReleaseRuleRepository;

public ConfigServiceAutoConfiguration(final BizConfig bizConfig) {
public ConfigServiceAutoConfiguration(final BizConfig bizConfig,
final ReleaseService releaseService,
final ReleaseMessageService releaseMessageService,
final GrayReleaseRuleRepository grayReleaseRuleRepository) {
this.bizConfig = bizConfig;
this.releaseService = releaseService;
this.releaseMessageService = releaseMessageService;
this.grayReleaseRuleRepository = grayReleaseRuleRepository;
}

@Bean
public GrayReleaseRulesHolder grayReleaseRulesHolder() {
return new GrayReleaseRulesHolder();
return new GrayReleaseRulesHolder(grayReleaseRuleRepository, bizConfig);
}

@Bean
public ConfigService configService() {
if (bizConfig.isConfigServiceCacheEnabled()) {
return new ConfigServiceWithCache();
return new ConfigServiceWithCache(releaseService, releaseMessageService,
grayReleaseRulesHolder());
}
return new DefaultConfigService();
return new DefaultConfigService(releaseService, grayReleaseRulesHolder());
}

@Bean
Expand All @@ -64,8 +78,8 @@ public static NoOpPasswordEncoder passwordEncoder() {
}

@Bean
public FilterRegistrationBean clientAuthenticationFilter(AccessKeyUtil accessKeyUtil) {
FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean();
public FilterRegistrationBean<ClientAuthenticationFilter> clientAuthenticationFilter(AccessKeyUtil accessKeyUtil) {
FilterRegistrationBean<ClientAuthenticationFilter> filterRegistrationBean = new FilterRegistrationBean<>();

filterRegistrationBean.setFilter(new ClientAuthenticationFilter(bizConfig, accessKeyUtil));
filterRegistrationBean.addUrlPatterns("/configs/*");
Expand All @@ -83,25 +97,32 @@ static class MessageScannerConfiguration {
private final GrayReleaseRulesHolder grayReleaseRulesHolder;
private final ReleaseMessageServiceWithCache releaseMessageServiceWithCache;
private final ConfigService configService;
private final BizConfig bizConfig;
private final ReleaseMessageRepository releaseMessageRepository;

public MessageScannerConfiguration(
final NotificationController notificationController,
final ConfigFileController configFileController,
final NotificationControllerV2 notificationControllerV2,
final GrayReleaseRulesHolder grayReleaseRulesHolder,
final ReleaseMessageServiceWithCache releaseMessageServiceWithCache,
final ConfigService configService) {
final ConfigService configService,
final BizConfig bizConfig,
final ReleaseMessageRepository releaseMessageRepository) {
this.notificationController = notificationController;
this.configFileController = configFileController;
this.notificationControllerV2 = notificationControllerV2;
this.grayReleaseRulesHolder = grayReleaseRulesHolder;
this.releaseMessageServiceWithCache = releaseMessageServiceWithCache;
this.configService = configService;
this.bizConfig = bizConfig;
this.releaseMessageRepository = releaseMessageRepository;
}

@Bean
public ReleaseMessageScanner releaseMessageScanner() {
ReleaseMessageScanner releaseMessageScanner = new ReleaseMessageScanner();
ReleaseMessageScanner releaseMessageScanner = new ReleaseMessageScanner(bizConfig,
releaseMessageRepository);
//0. handle release message cache
releaseMessageScanner.addMessageListener(releaseMessageServiceWithCache);
//1. handle gray release rule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ public class NotificationControllerV2 implements ReleaseMessageListener {
private final Gson gson;
private final BizConfig bizConfig;

@Autowired
public NotificationControllerV2(
final WatchKeysUtil watchKeysUtil,
final ReleaseMessageServiceWithCache releaseMessageService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ public class AccessKeyServiceWithCache implements InitializingBean {
private ListMultimap<String, AccessKey> accessKeyCache;
private ConcurrentMap<Long, AccessKey> accessKeyIdCache;

@Autowired
public AccessKeyServiceWithCache(AccessKeyRepository accessKeyRepository, BizConfig bizConfig) {
public AccessKeyServiceWithCache(final AccessKeyRepository accessKeyRepository,
final BizConfig bizConfig) {
this.accessKeyRepository = accessKeyRepository;
this.bizConfig = bizConfig;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@
import com.google.common.base.Strings;

import java.util.Objects;
import org.springframework.beans.factory.annotation.Autowired;

/**
* @author Jason Song([email protected])
*/
public abstract class AbstractConfigService implements ConfigService {
@Autowired
private GrayReleaseRulesHolder grayReleaseRulesHolder;

private final GrayReleaseRulesHolder grayReleaseRulesHolder;

protected AbstractConfigService(final GrayReleaseRulesHolder grayReleaseRulesHolder) {
this.grayReleaseRulesHolder = grayReleaseRulesHolder;
}

@Override
public Release loadConfig(String clientAppId, String clientIp, String clientLabel, String configAppId, String configClusterName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package com.ctrip.framework.apollo.configservice.service.config;

import com.ctrip.framework.apollo.biz.grayReleaseRule.GrayReleaseRulesHolder;
import com.google.common.base.Strings;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
Expand All @@ -37,7 +38,6 @@

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;

import java.util.List;
import java.util.concurrent.TimeUnit;
Expand All @@ -59,19 +59,21 @@ public class ConfigServiceWithCache extends AbstractConfigService {
private static final String TRACER_EVENT_CACHE_GET = "ConfigCache.Get";
private static final String TRACER_EVENT_CACHE_GET_ID = "ConfigCache.GetById";

@Autowired
private ReleaseService releaseService;

@Autowired
private ReleaseMessageService releaseMessageService;
private final ReleaseService releaseService;
private final ReleaseMessageService releaseMessageService;

private LoadingCache<String, ConfigCacheEntry> configCache;

private LoadingCache<Long, Optional<Release>> configIdCache;

private ConfigCacheEntry nullConfigCacheEntry;

public ConfigServiceWithCache() {
public ConfigServiceWithCache(final ReleaseService releaseService,
final ReleaseMessageService releaseMessageService,
final GrayReleaseRulesHolder grayReleaseRulesHolder) {
super(grayReleaseRulesHolder);
this.releaseService = releaseService;
this.releaseMessageService = releaseMessageService;
nullConfigCacheEntry = new ConfigCacheEntry(ConfigConsts.NOTIFICATION_ID_PLACEHOLDER, null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,26 @@

import com.ctrip.framework.apollo.biz.entity.Release;
import com.ctrip.framework.apollo.biz.entity.ReleaseMessage;
import com.ctrip.framework.apollo.biz.grayReleaseRule.GrayReleaseRulesHolder;
import com.ctrip.framework.apollo.biz.service.ReleaseService;
import com.ctrip.framework.apollo.core.dto.ApolloNotificationMessages;

import org.springframework.beans.factory.annotation.Autowired;

/**
* config service with no cache
*
* @author Jason Song([email protected])
*/
public class DefaultConfigService extends AbstractConfigService {

@Autowired
private ReleaseService releaseService;
private final ReleaseService releaseService;
private final GrayReleaseRulesHolder grayReleaseRulesHolder;

public DefaultConfigService(final ReleaseService releaseService,
final GrayReleaseRulesHolder grayReleaseRulesHolder) {
super(grayReleaseRulesHolder);
this.releaseService = releaseService;
this.grayReleaseRulesHolder = grayReleaseRulesHolder;
}

@Override
protected Release findActiveOne(long id, ApolloNotificationMessages clientMessages) {
Expand Down
Loading

0 comments on commit 2800a00

Please sign in to comment.