diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java index 298de2c295..c890ac5b4c 100644 --- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java +++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscovery.java @@ -253,8 +253,6 @@ private ClouderaManagerCluster discoverCluster(DiscoveryApiClient client, String log.discoveringCluster(clusterName); - repository.registerCluster(client.getConfig()); - Set serviceModels = new HashSet<>(); List serviceList = getClusterServices(client.getConfig(), servicesResourceApi); @@ -340,7 +338,7 @@ private boolean shouldSkipServiceDiscovery(List modelGene private List getClusterServices(ServiceDiscoveryConfig serviceDiscoveryConfig, ServicesResourceApi servicesResourceApi) throws ApiException { log.lookupClusterServicesFromRepository(); List services = repository.getServices(serviceDiscoveryConfig); - if (services == null || services.isEmpty()) { + if (services.isEmpty()) { try { log.lookupClusterServicesFromCM(); final ApiServiceList serviceList = servicesResourceApi.readServices(serviceDiscoveryConfig.getCluster(), VIEW_SUMMARY); diff --git a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java index 5329cd8ce2..115717f69b 100644 --- a/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java +++ b/gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepository.java @@ -59,23 +59,18 @@ void clear() { repository.clear(); } - void registerCluster(ServiceDiscoveryConfig serviceDiscoveryConfig) { - repository.putIfAbsent(RepositoryKey.of(serviceDiscoveryConfig), Caffeine.newBuilder().expireAfterWrite(Duration.ofSeconds(cacheEntryTTL)).build()); - } - void addService(ServiceDiscoveryConfig serviceDiscoveryConfig, ApiService service) { getClusterServices(serviceDiscoveryConfig).put(service, new ServiceDetails()); } List getServices(ServiceDiscoveryConfig serviceDiscoveryConfig) { final Cache clusterServices = getClusterServices(serviceDiscoveryConfig); - return clusterServices == null ? null - : clusterServices.asMap().entrySet().stream().filter(entry -> entry.getValue() != null).map(entry -> entry.getKey()) + return clusterServices.asMap().entrySet().stream().filter(entry -> entry.getValue() != null).map(entry -> entry.getKey()) .collect(Collectors.toList()); } private Cache getClusterServices(ServiceDiscoveryConfig serviceDiscoveryConfig) { - return repository.get(RepositoryKey.of(serviceDiscoveryConfig)); + return repository.computeIfAbsent(RepositoryKey.of(serviceDiscoveryConfig), (k) -> Caffeine.newBuilder().expireAfterWrite(Duration.ofSeconds(cacheEntryTTL)).build()); } void addServiceConfig(ServiceDiscoveryConfig serviceDiscoveryConfig, ApiService service, ApiServiceConfig serviceConfig) { diff --git a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java index b20f93d6f0..d654082729 100644 --- a/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java +++ b/gateway-discovery-cm/src/test/java/org/apache/knox/gateway/topology/discovery/cm/ClouderaManagerServiceDiscoveryRepositoryTest.java @@ -56,14 +56,13 @@ public static void init() { @Before public void setUp() { repository.clear(); - assertNull(repository.getServices(serviceDiscoveryConfig)); repository.setCacheEntryTTL(GatewayConfig.DEFAULT_CM_SERVICE_DISCOVERY_CACHE_ENTRY_TTL); + assertTrue(repository.getServices(serviceDiscoveryConfig).isEmpty()); } @Test public void testRegisterCluster() throws Exception { - assertNull(repository.getServices(serviceDiscoveryConfig)); - repository.registerCluster(serviceDiscoveryConfig); + assertTrue(repository.getServices(serviceDiscoveryConfig).isEmpty()); final List services = repository.getServices(serviceDiscoveryConfig); assertNotNull(services); assertTrue(services.isEmpty()); @@ -72,7 +71,6 @@ public void testRegisterCluster() throws Exception { @Test public void testAddService() throws Exception { final String serviceName = "HDFS-1"; - repository.registerCluster(serviceDiscoveryConfig); assertFalse(containsService(serviceName)); final ApiService service = EasyMock.createNiceMock(ApiService.class); EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes(); @@ -86,7 +84,6 @@ public void testAddServiceConfig() throws Exception { final String serviceName = "HDFS-1"; final String serviceConfigName = "myServiceConfigName"; final String serviceConfigValue = "myServiceConfigValue"; - repository.registerCluster(serviceDiscoveryConfig); final ApiService service = EasyMock.createNiceMock(ApiService.class); EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes(); final ApiConfig serviceConfig = EasyMock.createNiceMock(ApiConfig.class); @@ -105,7 +102,6 @@ public void testAddServiceConfig() throws Exception { public void testAddRole() throws Exception { final String serviceName = "HDFS-1"; final String roleName = "NAMENODE-1"; - repository.registerCluster(serviceDiscoveryConfig); final ApiService service = EasyMock.createNiceMock(ApiService.class); EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes(); final ApiRole role = EasyMock.createNiceMock(ApiRole.class); @@ -121,7 +117,6 @@ public void testAddRole() throws Exception { @Test public void testAddNullRoles() throws Exception { - repository.registerCluster(serviceDiscoveryConfig); final ApiService service = EasyMock.createNiceMock(ApiService.class); EasyMock.expect(service.getName()).andReturn(ClouderaManagerServiceDiscovery.CORE_SETTINGS_TYPE).anyTimes(); EasyMock.replay(service); @@ -132,7 +127,6 @@ public void testAddNullRoles() throws Exception { @Test public void testAddNullRoleItems() throws Exception { - repository.registerCluster(serviceDiscoveryConfig); final ApiService service = EasyMock.createNiceMock(ApiService.class); EasyMock.expect(service.getName()).andReturn(ClouderaManagerServiceDiscovery.CORE_SETTINGS_TYPE).anyTimes(); final ApiRoleList roles = EasyMock.createNiceMock(ApiRoleList.class); @@ -150,7 +144,6 @@ public void testAddRoleConfig() throws Exception { final String roleConfigName = "myRoleConfig"; final String roleConfigValue = "myRoleConfigValue"; - repository.registerCluster(serviceDiscoveryConfig); final ApiService service = EasyMock.createNiceMock(ApiService.class); EasyMock.expect(service.getName()).andReturn(serviceName).anyTimes(); final ApiRole role = EasyMock.createNiceMock(ApiRole.class); @@ -174,6 +167,7 @@ public void testAddRoleConfig() throws Exception { public void testCacheAutoEviction() throws Exception { final long entryTTL = 1; repository.setCacheEntryTTL(entryTTL); + repository.clear(); testAddService(); TimeUnit.SECONDS.sleep(entryTTL + 1); assertFalse(containsService("HDFS-1"));