From 9e5c62a5b7ee54862a55287ef5e8610e5280a04a Mon Sep 17 00:00:00 2001 From: agulenko Date: Thu, 6 Oct 2016 19:00:23 +0300 Subject: [PATCH] KAA-1423: code review changes --- .../kaa/server/common/admin/AdminClient.java | 6 ++--- .../common/dao/ConfigurationService.java | 9 +++++++- .../dao/service/ConfigurationServiceImpl.java | 2 +- ...pointSpecificConfigurationServiceImpl.java | 7 +++--- .../service/UserConfigurationServiceImpl.java | 2 +- ...tSpecificConfigurationServiceImplTest.java | 15 ++++++------- .../controller/ConfigurationController.java | 6 ++--- .../services/ConfigurationServiceImpl.java | 1 + .../GlobalEndpointActorMessageProcessor.java | 8 ++----- .../service/delta/DefaultDeltaService.java | 22 +++++++++++++------ 10 files changed, 44 insertions(+), 34 deletions(-) diff --git a/server/common/admin-rest-client/src/main/java/org/kaaproject/kaa/server/common/admin/AdminClient.java b/server/common/admin-rest-client/src/main/java/org/kaaproject/kaa/server/common/admin/AdminClient.java index 3062010a3f..e555f3bcef 100644 --- a/server/common/admin-rest-client/src/main/java/org/kaaproject/kaa/server/common/admin/AdminClient.java +++ b/server/common/admin-rest-client/src/main/java/org/kaaproject/kaa/server/common/admin/AdminClient.java @@ -529,15 +529,15 @@ public void editUserConfiguration(EndpointUserConfigurationDto endpointUserConfi } public EndpointSpecificConfigurationDto editEndpointSpecificConfiguration(EndpointSpecificConfigurationDto configuration) throws Exception { - return restTemplate.postForObject(restTemplate.getUrl() + "endpointConfiguration", configuration, EndpointSpecificConfigurationDto.class); + return restTemplate.postForObject(restTemplate.getUrl() + "endpointSpecificConfiguration", configuration, EndpointSpecificConfigurationDto.class); } public void deleteEndpointSpecificConfiguration(String endpointKeyHash) throws Exception { - restTemplate.delete(restTemplate.getUrl() + "endpointConfiguration/" + endpointKeyHash); + restTemplate.delete(restTemplate.getUrl() + "endpointSpecificConfiguration/" + endpointKeyHash); } public EndpointSpecificConfigurationDto findEndpointSpecificConfiguration(String endpointKeyHash) throws Exception { - return restTemplate.getForObject(restTemplate.getUrl() + "endpointConfiguration/" + endpointKeyHash, EndpointSpecificConfigurationDto.class); + return restTemplate.getForObject(restTemplate.getUrl() + "endpointSpecificConfiguration/" + endpointKeyHash, EndpointSpecificConfigurationDto.class); } public ProfileFilterDto editProfileFilter(ProfileFilterDto profileFilter) throws Exception { diff --git a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/ConfigurationService.java b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/ConfigurationService.java index dbf41803ba..ad9597074c 100644 --- a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/ConfigurationService.java +++ b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/ConfigurationService.java @@ -192,7 +192,14 @@ public interface ConfigurationService { */ ConfigurationSchemaDto saveConfSchema(ConfigurationSchemaDto configurationSchema); - String validateConfiguration(String appId, int schemaVersion, String configurationBody); + /** + * Validate configuration according to override configuration schema. + * + * @param appId application ID + * @param schemaVersion configuration schema version + * @param configurationBody configuration to validate + */ + String validateOverrideConfigurationBody(String appId, int schemaVersion, String configurationBody); /** * Find configuration schema by id. diff --git a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/ConfigurationServiceImpl.java b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/ConfigurationServiceImpl.java index 3eefbcbb4b..eaf27e43d0 100644 --- a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/ConfigurationServiceImpl.java +++ b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/ConfigurationServiceImpl.java @@ -469,7 +469,7 @@ public ConfigurationSchemaDto saveConfSchema(ConfigurationSchemaDto configuratio } @Override - public String validateConfiguration(String appId, int schemaVersion, String configurationBody) { + public String validateOverrideConfigurationBody(String appId, int schemaVersion, String configurationBody) { ConfigurationSchemaDto schemaDto = this.findConfSchemaByAppIdAndVersion(appId, schemaVersion); if (schemaDto != null) { OverrideSchema overrideSchema = new OverrideSchema(schemaDto.getOverrideSchema()); diff --git a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImpl.java b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImpl.java index 43cab7f738..c302efa811 100644 --- a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImpl.java +++ b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImpl.java @@ -78,7 +78,7 @@ public Optional deleteByEndpointKeyHash(String public EndpointSpecificConfigurationDto save(EndpointSpecificConfigurationDto configurationDto) { EndpointProfileDto profileDto = getEndpointProfileDto(configurationDto.getEndpointKeyHash()); configurationDto.setConfigurationVersion(profileDto.getConfigurationVersion()); - validateConfigurationBody(configurationDto, profileDto); + validateEndpointSpecificConfiguration(configurationDto, profileDto); return endpointSpecificConfigurationDao.save(configurationDto).toDto(); } @@ -88,13 +88,12 @@ private EndpointProfileDto getEndpointProfileDto(String endpointKeyHash) { } - private void validateConfigurationBody(EndpointSpecificConfigurationDto configurationDto, EndpointProfileDto ep) { + private void validateEndpointSpecificConfiguration(EndpointSpecificConfigurationDto configurationDto, EndpointProfileDto ep) { validateString(configurationDto.getConfiguration(), "Endpoint specific configuration body is required"); int configurationVersion = configurationDto.getConfigurationVersion(); String appId = ep.getApplicationId(); String configurationBody = configurationDto.getConfiguration(); - configurationBody = configurationService.validateConfiguration(appId, configurationVersion, configurationBody); - validateString(configurationBody, "Provided configuration body is invalid"); + configurationBody = configurationService.validateOverrideConfigurationBody(appId, configurationVersion, configurationBody); configurationDto.setConfiguration(configurationBody); } diff --git a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/UserConfigurationServiceImpl.java b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/UserConfigurationServiceImpl.java index bb532cdebe..266d485cec 100644 --- a/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/UserConfigurationServiceImpl.java +++ b/server/common/dao/src/main/java/org/kaaproject/kaa/server/common/dao/service/UserConfigurationServiceImpl.java @@ -62,7 +62,7 @@ public EndpointUserConfigurationDto saveUserConfiguration(EndpointUserConfigurat ApplicationDto applicationDto = applicationService.findAppByApplicationToken(appToken); if (applicationDto != null) { int schemaVersion = userConfig.getSchemaVersion(); - userConfig.setBody(configurationService.validateConfiguration(applicationDto.getId(), schemaVersion, userConfigBody)); + userConfig.setBody(configurationService.validateOverrideConfigurationBody(applicationDto.getId(), schemaVersion, userConfigBody)); userConfigurationDto = getDto(endpointUserConfigurationDao.save(userConfig)); } else { LOG.warn("Can't find application with token {} for endpoint user configuration.", appToken); diff --git a/server/common/dao/src/test/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImplTest.java b/server/common/dao/src/test/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImplTest.java index 967ffa6f55..b44ff556c4 100644 --- a/server/common/dao/src/test/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImplTest.java +++ b/server/common/dao/src/test/java/org/kaaproject/kaa/server/common/dao/service/EndpointSpecificConfigurationServiceImplTest.java @@ -60,17 +60,17 @@ public void shouldSave() { dto.setEndpointKeyHash(KEY); dto.setConfiguration(CONFIG_BODY); when(endpointServiceMock.findEndpointProfileByKeyHash(DECODED_KEY)).thenReturn(generateProfile()); - when(configurationServiceMock.validateConfiguration(APP_ID, CONF_VERSION, CONFIG_BODY)).thenReturn("valid body"); + when(configurationServiceMock.validateOverrideConfigurationBody(APP_ID, CONF_VERSION, CONFIG_BODY)).thenReturn("valid body"); when(daoMock.save(dto)).thenReturn(configuration); when(configuration.toDto()).thenReturn(new EndpointSpecificConfigurationDto()); Assert.assertTrue(SERVICE.save(dto) != null); - verify(configurationServiceMock).validateConfiguration(APP_ID, CONF_VERSION, CONFIG_BODY); + verify(configurationServiceMock).validateOverrideConfigurationBody(APP_ID, CONF_VERSION, CONFIG_BODY); verify(endpointServiceMock).findEndpointProfileByKeyHash(DECODED_KEY); verify(daoMock).save(dto); } @Test - public void shouldFoundByEndpointProfile() { + public void testShouldFoundByEndpointProfile() { when(daoMock.findByEndpointKeyHashAndConfigurationVersion(KEY, CONF_VERSION)).thenReturn(configuration); when(configuration.toDto()).thenReturn(new EndpointSpecificConfigurationDto()); Assert.assertTrue(SERVICE.findByEndpointProfile(generateProfile()).isPresent()); @@ -78,7 +78,7 @@ public void shouldFoundByEndpointProfile() { } @Test - public void shouldFoundByEndpointKeyHash() { + public void testShouldFoundByEndpointKeyHash() { when(endpointServiceMock.findEndpointProfileByKeyHash(DECODED_KEY)).thenReturn(generateProfile()); when(daoMock.findByEndpointKeyHashAndConfigurationVersion(KEY, CONF_VERSION)).thenReturn(configuration); when(configuration.toDto()).thenReturn(new EndpointSpecificConfigurationDto()); @@ -88,14 +88,14 @@ public void shouldFoundByEndpointKeyHash() { } @Test - public void shouldNotFoundByEndpointKeyHashWhenProfileNotFound() { + public void testShouldNotFoundByEndpointKeyHashWhenProfileNotFound() { when(endpointServiceMock.findEndpointProfileByKeyHash(DECODED_KEY)).thenReturn(null); Assert.assertFalse(SERVICE.findByEndpointKeyHash(KEY).isPresent()); verify(endpointServiceMock).findEndpointProfileByKeyHash(DECODED_KEY); } @Test - public void shouldDeleteByEndpointKeyHash() { + public void testShouldDeleteByEndpointKeyHash() { when(endpointServiceMock.findEndpointProfileByKeyHash(DECODED_KEY)).thenReturn(generateProfile()); when(daoMock.findByEndpointKeyHashAndConfigurationVersion(KEY, CONF_VERSION)).thenReturn(configuration); when(configuration.toDto()).thenReturn(new EndpointSpecificConfigurationDto()); @@ -103,11 +103,10 @@ public void shouldDeleteByEndpointKeyHash() { verify(endpointServiceMock).findEndpointProfileByKeyHash(DECODED_KEY); verify(daoMock).findByEndpointKeyHashAndConfigurationVersion(KEY, CONF_VERSION); verify(daoMock).removeByEndpointKeyHash(KEY); - } @Test - public void shouldNotDeleteByEndpointKeyHashWhenProfileNotFound() { + public void testShouldNotDeleteByEndpointKeyHashWhenProfileNotFound() { when(endpointServiceMock.findEndpointProfileByKeyHash(DECODED_KEY)).thenReturn(generateProfile()); when(daoMock.findByEndpointKeyHashAndConfigurationVersion(KEY, CONF_VERSION)).thenReturn(null); Assert.assertFalse(SERVICE.deleteByEndpointKeyHash(KEY).isPresent()); diff --git a/server/node/src/main/java/org/kaaproject/kaa/server/admin/controller/ConfigurationController.java b/server/node/src/main/java/org/kaaproject/kaa/server/admin/controller/ConfigurationController.java index f855eb5b7b..36565657d6 100644 --- a/server/node/src/main/java/org/kaaproject/kaa/server/admin/controller/ConfigurationController.java +++ b/server/node/src/main/java/org/kaaproject/kaa/server/admin/controller/ConfigurationController.java @@ -301,7 +301,7 @@ public void editUserConfiguration( @ApiResponse(code = 404, message = "Specified endpoint does not exist"), @ApiResponse(code = 409, message = "Can't update entity with provided version. Entity already changed"), @ApiResponse(code = 500, message = "An unexpected error occurred on the server side")}) - @RequestMapping(value = "endpointConfiguration", method = RequestMethod.POST) + @RequestMapping(value = "endpointSpecificConfiguration", method = RequestMethod.POST) @ResponseStatus(value = HttpStatus.OK) @ResponseBody public EndpointSpecificConfigurationDto editEndpointSpecificConfiguration( @@ -327,7 +327,7 @@ public EndpointSpecificConfigurationDto editEndpointSpecificConfiguration( "of the application does not match the Tenant ID of the authenticated user"), @ApiResponse(code = 404, message = "Specified endpoint does not exist"), @ApiResponse(code = 500, message = "An unexpected error occurred on the server side")}) - @RequestMapping(value = "endpointConfiguration/{endpointKeyHash}", method = RequestMethod.GET) + @RequestMapping(value = "endpointSpecificConfiguration/{endpointKeyHash}", method = RequestMethod.GET) @ResponseStatus(value = HttpStatus.OK) @ResponseBody public EndpointSpecificConfigurationDto findEndpointSpecificConfiguration( @@ -352,7 +352,7 @@ public EndpointSpecificConfigurationDto findEndpointSpecificConfiguration( "of the application does not match the Tenant ID of the authenticated user"), @ApiResponse(code = 404, message = "Specified endpoint does not exist"), @ApiResponse(code = 500, message = "An unexpected error occurred on the server side")}) - @RequestMapping(value = "endpointConfiguration/{endpointKeyHash}", method = RequestMethod.DELETE) + @RequestMapping(value = "endpointSpecificConfiguration/{endpointKeyHash}", method = RequestMethod.DELETE) @ResponseStatus(value = HttpStatus.OK) public void deleteEndpointSpecificConfiguration( @PathVariable String endpointKeyHash) throws KaaAdminServiceException { diff --git a/server/node/src/main/java/org/kaaproject/kaa/server/admin/services/ConfigurationServiceImpl.java b/server/node/src/main/java/org/kaaproject/kaa/server/admin/services/ConfigurationServiceImpl.java index 832778374f..00002b90d9 100644 --- a/server/node/src/main/java/org/kaaproject/kaa/server/admin/services/ConfigurationServiceImpl.java +++ b/server/node/src/main/java/org/kaaproject/kaa/server/admin/services/ConfigurationServiceImpl.java @@ -260,6 +260,7 @@ public EndpointSpecificConfigurationDto findEndpointSpecificConfiguration(String @Override public EndpointSpecificConfigurationDto deleteEndpointSpecificConfiguration(String endpointKeyHash) throws KaaAdminServiceException { + checkAuthority(KaaAuthorityDto.TENANT_DEVELOPER, KaaAuthorityDto.TENANT_USER); try { checkEndpointProfile(endpointKeyHash); EndpointSpecificConfigurationDto configuration = controlService.deleteEndpointSpecificConfiguration(endpointKeyHash); diff --git a/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/akka/actors/core/endpoint/global/GlobalEndpointActorMessageProcessor.java b/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/akka/actors/core/endpoint/global/GlobalEndpointActorMessageProcessor.java index d4f89a12bd..dec6b1009e 100644 --- a/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/akka/actors/core/endpoint/global/GlobalEndpointActorMessageProcessor.java +++ b/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/akka/actors/core/endpoint/global/GlobalEndpointActorMessageProcessor.java @@ -95,17 +95,13 @@ private void processServerProfileUpdateMsg(ActorContext context, ThriftServerPro operationsService.syncServerProfile(appToken, endpointKey, key); ThriftServerProfileUpdateMessage localMsg = new ThriftServerProfileUpdateMessage(thriftMsg); localMsg.setActorClassifier(ThriftActorClassifier.LOCAL); - dispatchMsg(context, localMsg, (nodeId, msg) -> { - clusterService.sendServerProfileUpdateMessage(nodeId, msg); - }); + dispatchMsg(context, localMsg, clusterService::sendServerProfileUpdateMessage); } private void processUnicastNotificationMsg(ActorContext context, ThriftUnicastNotificationMessage thriftMsg) { ThriftUnicastNotificationMessage localMsg = new ThriftUnicastNotificationMessage(thriftMsg); localMsg.setActorClassifier(ThriftActorClassifier.LOCAL); - dispatchMsg(context, localMsg, (nodeId, msg) -> { - clusterService.sendUnicastNotificationMessage(nodeId, msg); - }); + dispatchMsg(context, localMsg, clusterService::sendUnicastNotificationMessage); } private void processEndpointConfigurationRefreshMsg(ActorContext context, ThriftEndpointConfigurationRefreshMessage thriftMsg) { diff --git a/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/delta/DefaultDeltaService.java b/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/delta/DefaultDeltaService.java index 26fbb592c5..e97e7be918 100644 --- a/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/delta/DefaultDeltaService.java +++ b/server/node/src/main/java/org/kaaproject/kaa/server/operations/service/delta/DefaultDeltaService.java @@ -197,14 +197,22 @@ private void logHashMismatch(GetDeltaRequest request, EndpointProfileDto profile */ private ConfigurationCacheEntry getDelta(final String endpointId, final String userId, DeltaCacheKey deltaKey, boolean useConfigurationRawSchema) throws GetDeltaException { EndpointUserConfigurationDto userConfiguration = findLatestUserConfiguration(userId, deltaKey); - Optional epsConfigOpt = Optional.empty(); - if (endpointId != null) { - epsConfigOpt = endpointSpecificConfigurationService.findByEndpointKeyHash(endpointId); - } - EndpointSpecificConfigurationDto epsConfig = epsConfigOpt.orElse(null); + + EndpointSpecificConfigurationDto epsConfig = Optional.ofNullable(endpointId) + .flatMap(endpointSpecificConfigurationService::findByEndpointKeyHash) + .orElse(null); + + EndpointObjectHash epsConfHash = Optional.ofNullable(epsConfig) + .map(EndpointSpecificConfigurationDto::getConfiguration) + .map(EndpointObjectHash::fromString) + .orElse(null); + + EndpointObjectHash userConfHash = Optional.ofNullable(userConfiguration) + .map(EndpointUserConfigurationDto::getBody) + .map(EndpointObjectHash::fromString) + .orElse(null); + final DeltaCacheKey newKey; - EndpointObjectHash userConfHash = userConfiguration != null ? EndpointObjectHash.fromString(userConfiguration.getBody()) : null; - EndpointObjectHash epsConfHash = epsConfigOpt.map(conf -> EndpointObjectHash.fromString(conf.getConfiguration())).orElse(null); if (userConfiguration != null || epsConfig != null) { newKey = new DeltaCacheKey( deltaKey.getAppConfigVersionKey(),