Skip to content

Commit

Permalink
Code improvements
Browse files Browse the repository at this point in the history
Signed-off-by: BOUHOURS Antoine <[email protected]>
  • Loading branch information
antoinebhs committed Oct 9, 2024
1 parent 26dea58 commit 195201d
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -874,13 +874,13 @@ public void removeConfiguredBuses(UUID networkUuid, int variantNum, List<String>

@Override
public Optional<ExtensionAttributes> getExtensionAttributes(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId, String extensionName) {
delegate.loadAllExtensionsAttributesByResourceTypeAndExtensionName(networkUuid, variantNum, resourceType, extensionName);
((CachedNetworkStoreClient) delegate).loadAllExtensionsAttributesByResourceTypeAndExtensionName(networkUuid, variantNum, resourceType, extensionName);
return delegate.getExtensionAttributes(networkUuid, variantNum, resourceType, identifiableId, extensionName);
}

@Override
public Map<String, ExtensionAttributes> getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String id) {
delegate.loadAllExtensionsAttributesByResourceType(networkUuid, variantNum, resourceType);
((CachedNetworkStoreClient) delegate).loadAllExtensionsAttributesByResourceType(networkUuid, variantNum, resourceType);
return delegate.getAllExtensionsAttributesByIdentifiableId(networkUuid, variantNum, resourceType, id);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -902,11 +902,6 @@ public Map<String, ExtensionAttributes> getAllExtensionsAttributesByResourceType
return getExtensionAttributesMap("/networks/{networkUuid}/{variantNum}/identifiables/types/{type}/extensions/{extensionName}", networkUuid, variantNum, resourceType, extensionName);
}

@Override
public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) {
// nothing to do
}

@Override
public Map<String, ExtensionAttributes> getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) {
return getExtensionAttributesMap("/networks/{networkUuid}/{variantNum}/identifiables/{identifiableId}/extensions", networkUuid, variantNum, identifiableId);
Expand All @@ -917,11 +912,6 @@ public Map<String, Map<String, ExtensionAttributes>> getAllExtensionsAttributesB
return getExtensionAttributesNestedMap("/networks/{networkUuid}/{variantNum}/identifiables/types/{resourceType}/extensions", networkUuid, variantNum, resourceType);
}

@Override
public void loadAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType) {
// nothing to do
}

@Override
public void removeExtensionAttributes(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId, String extensionName) {
restClient.delete("/networks/{networkUuid}/{variantNum}/identifiables/{identifiableId}/extensions/{extensionName}", networkUuid, variantNum, identifiableId, extensionName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ public void testGetExtensionsCacheWithClonedNetwork() throws IOException {
server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/identifiables/types/" + ResourceType.GENERATOR + "/extensions"))
.andExpect(method(GET))
.andRespond(withSuccess(multipleExtensionAttributes, MediaType.APPLICATION_JSON));
cachedClient.getAllExtensionsAttributesByResourceType(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR);
cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId1);
server.verify();
server.reset();

Expand All @@ -1008,9 +1008,8 @@ public void testGetExtensionsCacheWithClonedNetwork() throws IOException {
cachedClient.cloneNetwork(networkUuid, Resource.INITIAL_VARIANT_NUM, targetVariantNum, targetVariantId);

// Verify that the cache is copied and there is no new fetch
cachedClient.getAllExtensionsAttributesByResourceType(networkUuid, targetVariantNum, ResourceType.GENERATOR);
Map<String, ExtensionAttributes> extensionAttributesByExtensionNameMap = cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId1);
assertEquals(2, extensionAttributesByExtensionNameMap.size());
Map<String, ExtensionAttributes> extensionAttributesByIdentifiableId = cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId1);
assertEquals(2, extensionAttributesByIdentifiableId.size());
server.verify();
server.reset();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,6 @@ public Optional<ExtensionAttributes> getExtensionAttributes(UUID networkUuid, in
return getCache(resourceType).getCollection(networkUuid, variantNum).getExtensionAttributes(networkUuid, variantNum, resourceType, identifiableId, extensionName);
}

@Override
public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) {
getCache(resourceType).getCollection(networkUuid, variantNum).loadAllExtensionsAttributesByResourceTypeAndExtensionName(networkUuid, variantNum, resourceType, extensionName);
}
Expand All @@ -1086,7 +1085,6 @@ public Map<String, ExtensionAttributes> getAllExtensionsAttributesByIdentifiable
return getCache(resourceType).getCollection(networkUuid, variantNum).getAllExtensionsAttributesByIdentifiableId(networkUuid, variantNum, resourceType, identifiableId);
}

@Override
public void loadAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType) {
getCache(resourceType).getCollection(networkUuid, variantNum).loadAllExtensionsAttributesByResourceType(networkUuid, variantNum, resourceType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,9 @@ private void addExtensionAttributesToCache(String identifiableId, String extensi
}

/**
* Get the extensions attributes with specified extension name for all the identifiables of the collection in the cache.
* Load all the extensions attributes with specified extension name for all the identifiables of the collection in the cache.
*/
public Map<String, ExtensionAttributes> getAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType type, String extensionName) {
public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType type, String extensionName) {
if (!isFullyLoadedExtension(extensionName)) {
// if collection has not yet been fully loaded we load it from the server
Map<String, ExtensionAttributes> extensionAttributesMap = delegate.getAllExtensionsAttributesByResourceTypeAndExtensionName(networkUuid, variantNum, type, extensionName);
Expand All @@ -406,13 +406,6 @@ public Map<String, ExtensionAttributes> getAllExtensionsAttributesByResourceType
extensionAttributesMap.forEach((identifiableId, extensionAttributes) -> addExtensionAttributesToCache(identifiableId, extensionName, extensionAttributes));
fullyLoadedExtensionsByExtensionName.add(extensionName);
}
//TODO This method is only used to load extension attributes in the collection cache when using preloading collection.
// The return is never used by the client as the call to getAllExtensionsAttributesByResourceTypeAndExtensionName() is always followed
// by a call to getExtensionAttributes(). The latter returns something meaningful for the client
// and it's used in the identifiable.getExtension() method. The map extensionAttributesMap can't be stored in the cache to be returned
// as we can't ensure synchronization with the resources map (if extensions or identifiables are updated/removed).
// We should refactor this method to return void.
return null;
}

/**
Expand Down Expand Up @@ -454,9 +447,9 @@ private void addAllExtensionAttributesToCache(String id, Map<String, ExtensionAt
}

/**
* Get all the extensions attributes for all the identifiables with specified resource type in the cache
* Load all the extensions attributes for all the identifiables with specified resource type in the cache
*/
public Map<String, Map<String, ExtensionAttributes>> getAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType type) {
public void loadAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType type) {
if (!fullyLoadedExtensions) {
// if collection has not yet been fully loaded we load it from the server
Map<String, Map<String, ExtensionAttributes>> extensionAttributesMap = delegate.getAllExtensionsAttributesByResourceType(networkUuid, variantNum, type);
Expand All @@ -465,13 +458,6 @@ public Map<String, Map<String, ExtensionAttributes>> getAllExtensionsAttributesB
extensionAttributesMap.forEach(this::addAllExtensionAttributesToCache);
fullyLoadedExtensions = true;
}
//TODO This method is only used to load extension attributes in the collection cache when using preloading collection.
// The return is never used by the client as the call to getAllExtensionsAttributesByResourceType() is always followed
// by a call to getAllExtensionsAttributesByIdentifiableId(). The latter returns something meaningful for the client
// and it's used in the identifiable.getExtensions() method. The map extensionAttributesMap can't be stored in the cache to be returned
// as we can't ensure synchronization with the resources map (if extensions or identifiables are updated/removed).
// We should refactor this method to return void.
return null;
}

public void removeExtensionAttributesByExtensionName(String identifiableId, String extensionName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,6 @@ public interface NetworkStoreClient {
*/
Map<String, ExtensionAttributes> getAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName);

void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName);

/**
* For one identifiable with a specific identifiable id, retrieves all extension attributes of this identifiable.
* @return A {@link Map} where keys are extension names and values are {@link ExtensionAttributes}.
Expand All @@ -333,8 +331,6 @@ public interface NetworkStoreClient {
*/
Map<String, Map<String, ExtensionAttributes>> getAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType);

void loadAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType);

void removeExtensionAttributes(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId, String extensionName);

Optional<Resource<IdentifiableAttributes>> getIdentifiable(UUID networkUuid, int variantNum, String id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,11 +616,6 @@ public Map<String, ExtensionAttributes> getAllExtensionsAttributesByResourceType
return Map.of();
}

@Override
public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) {
// nothing to do
}

@Override
public Map<String, ExtensionAttributes> getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) {
return Map.of();
Expand All @@ -631,11 +626,6 @@ public Map<String, Map<String, ExtensionAttributes>> getAllExtensionsAttributesB
return Map.of();
}

@Override
public void loadAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType) {
// nothing to do
}

@Override
public void removeExtensionAttributes(UUID uuid, int workingVariantNum, ResourceType resourceType, String identifiableId, String extensionName) {
// nothing to do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,11 @@ public void removeResourceThenGetContainerTest() {
public void getExtensionAttributesWithResourceNotCachedMustThrow() {
PowsyblException exception = assertThrows(PowsyblException.class, () -> collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl"));
assertTrue(exception.getMessage().startsWith("Cannot manipulate extensions for identifiable"));
exception = assertThrows(PowsyblException.class, () -> collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl"));
exception = assertThrows(PowsyblException.class, () -> collectionCache.loadAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl"));
assertTrue(exception.getMessage().startsWith("Cannot manipulate extensions for identifiable"));
exception = assertThrows(PowsyblException.class, () -> collectionCache.getAllExtensionsAttributesByIdentifiableId(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1"));
assertTrue(exception.getMessage().startsWith("Cannot manipulate extensions for identifiable"));
exception = assertThrows(PowsyblException.class, () -> collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD));
exception = assertThrows(PowsyblException.class, () -> collectionCache.loadAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD));
assertTrue(exception.getMessage().startsWith("Cannot manipulate extensions for identifiable"));
}

Expand Down Expand Up @@ -495,13 +495,13 @@ public void getExtensionAttributesLoaderByResourceTypeAndName() {
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled());
assertNull(collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl"));
collectionCache.loadAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl");
assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled());
assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled());
mockNetworkStoreClient.setExtensionAttributesLoaderByResourceTypeAndNameCalled(false);
assertNull(collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl"));
collectionCache.loadAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl");
assertEquals(apc1, collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl").orElse(null));
assertEquals(apc2, collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l2", "activePowerControl").orElse(null));
assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled());
Expand Down Expand Up @@ -539,13 +539,13 @@ public void getExtensionAttributesLoaderByResourceType() {
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled());
assertNull(collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD));
collectionCache.loadAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD);
assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled());
assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled());
assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled());
mockNetworkStoreClient.setExtensionAttributesLoaderByResourceTypeCalled(false);
assertNull(collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD));
collectionCache.loadAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD);
assertEquals(apc1, collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl").orElse(null));
assertEquals(os1, collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "operatingStatus").orElse(null));
assertEquals(apc2, collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l2", "activePowerControl").orElse(null));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ public Map<String, ExtensionAttributes> getAllExtensionsAttributesByResourceType
return Map.of();
}

@Override
public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) {
//TODO
}

@Override
public Map<String, ExtensionAttributes> getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) {
extensionAttributesLoaderByIdCalled = true;
Expand All @@ -83,11 +78,6 @@ public Map<String, Map<String, ExtensionAttributes>> getAllExtensionsAttributesB
return Map.of();
}

@Override
public void loadAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType) {
//TODO
}

// Methods below are not used in tests
@Override
public void removeExtensionAttributes(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId, String extensionName) {
Expand Down

0 comments on commit 195201d

Please sign in to comment.