From ff357dd5dbc70b11ea814f8ca9318f816458cae8 Mon Sep 17 00:00:00 2001 From: Antoine Bouhours Date: Wed, 9 Oct 2024 14:12:45 +0200 Subject: [PATCH] Fix looking on all resources when retrieving extensions for preloading collection (#463) Signed-off-by: BOUHOURS Antoine --- .../client/CachedNetworkStoreClientTest.java | 5 +- .../PreloadingNetworkStoreClientTest.java | 54 +++++++++++++++++++ .../store/iidm/impl/CollectionCache.java | 22 +++++--- .../store/iidm/impl/CollectionCacheTest.java | 8 +-- 4 files changed, 75 insertions(+), 14 deletions(-) diff --git a/network-store-client/src/test/java/com/powsybl/network/store/client/CachedNetworkStoreClientTest.java b/network-store-client/src/test/java/com/powsybl/network/store/client/CachedNetworkStoreClientTest.java index 0de899341..b2351b3b1 100644 --- a/network-store-client/src/test/java/com/powsybl/network/store/client/CachedNetworkStoreClientTest.java +++ b/network-store-client/src/test/java/com/powsybl/network/store/client/CachedNetworkStoreClientTest.java @@ -645,10 +645,11 @@ public void testGetExtensionCacheWithClonedNetwork() throws IOException { server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/to/" + targetVariantNum + "?targetVariantId=" + targetVariantId)) .andExpect(method(PUT)) .andRespond(withSuccess()); + // Clone network and verify that there is the expected extension in the cloned cache cachedClient.cloneNetwork(networkUuid, Resource.INITIAL_VARIANT_NUM, targetVariantNum, targetVariantId); - Optional apc1Attributes = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + Optional apc1Attributes = cachedClient.getExtensionAttributes(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); assertTrue(apc1Attributes.isPresent()); - Optional os1Attributes = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, OperatingStatus.NAME); + Optional os1Attributes = cachedClient.getExtensionAttributes(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId, OperatingStatus.NAME); assertFalse(os1Attributes.isPresent()); server.verify(); server.reset(); diff --git a/network-store-client/src/test/java/com/powsybl/network/store/client/PreloadingNetworkStoreClientTest.java b/network-store-client/src/test/java/com/powsybl/network/store/client/PreloadingNetworkStoreClientTest.java index c80c5a01d..dbe87a1d2 100644 --- a/network-store-client/src/test/java/com/powsybl/network/store/client/PreloadingNetworkStoreClientTest.java +++ b/network-store-client/src/test/java/com/powsybl/network/store/client/PreloadingNetworkStoreClientTest.java @@ -34,6 +34,7 @@ import static org.junit.Assert.*; import static org.springframework.http.HttpMethod.GET; +import static org.springframework.http.HttpMethod.PUT; import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; @@ -960,4 +961,57 @@ private void loadTwoIdentifiablesToCache(String identifiableId1, String identifi server.verify(); server.reset(); } + + @Test + public void testGetExtensionsCacheWithClonedNetwork() throws IOException { + int targetVariantNum = 1; + String targetVariantId = "new_variant"; + String identifiableId1 = "GEN"; + String identifiableId2 = "GEN1"; + + // Load the identifiables in the cache + loadTwoIdentifiablesToCache(identifiableId1, identifiableId2); + + // Two successive ExtensionAttributes retrieval, only the first should send a REST request, the second uses the cache + ActivePowerControlAttributes apc1 = ActivePowerControlAttributes.builder() + .droop(5.2) + .participate(true) + .participationFactor(0.5) + .build(); + GeneratorStartupAttributes gs1 = GeneratorStartupAttributes.builder() + .marginalCost(6.8) + .forcedOutageRate(35) + .plannedOutageRate(30) + .startupCost(28) + .plannedActivePowerSetpoint(5) + .build(); + ActivePowerControlAttributes apc2 = ActivePowerControlAttributes.builder() + .droop(5.2) + .participate(true) + .participationFactor(1) + .build(); + + // Load extensions to cache on initial variant + String multipleExtensionAttributes = objectMapper.writerFor(new TypeReference>>() { + }).writeValueAsString(Map.of(identifiableId1, Map.of(ActivePowerControl.NAME, apc1, GeneratorStartup.NAME, gs1), identifiableId2, Map.of(ActivePowerControl.NAME, apc2))); + 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); + server.verify(); + server.reset(); + + // Clone network + server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/to/" + targetVariantNum + "?targetVariantId=" + targetVariantId)) + .andExpect(method(PUT)) + .andRespond(withSuccess()); + 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 extensionAttributesByExtensionNameMap = cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId1); + assertEquals(2, extensionAttributesByExtensionNameMap.size()); + server.verify(); + server.reset(); + } } diff --git a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CollectionCache.java b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CollectionCache.java index b45734572..91e585ffb 100644 --- a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CollectionCache.java +++ b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CollectionCache.java @@ -406,10 +406,13 @@ public Map getAllExtensionsAttributesByResourceType extensionAttributesMap.forEach((identifiableId, extensionAttributes) -> addExtensionAttributesToCache(identifiableId, extensionName, extensionAttributes)); fullyLoadedExtensionsByExtensionName.add(extensionName); } - return resources.entrySet() - .stream() - .filter(resourceEntry -> resourceEntry.getValue().getAttributes().getExtensionAttributes().containsKey(extensionName)) - .collect(Collectors.toMap(Map.Entry::getKey, resourceEntry -> resourceEntry.getValue().getAttributes().getExtensionAttributes().get(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; } /** @@ -462,10 +465,13 @@ public Map> getAllExtensionsAttributesB extensionAttributesMap.forEach(this::addAllExtensionAttributesToCache); fullyLoadedExtensions = true; } - return resources.entrySet() - .stream() - .filter(resourceEntry -> !resourceEntry.getValue().getAttributes().getExtensionAttributes().isEmpty()) - .collect(Collectors.toMap(Map.Entry::getKey, resourceEntry -> resourceEntry.getValue().getAttributes().getExtensionAttributes())); + //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) { diff --git a/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/CollectionCacheTest.java b/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/CollectionCacheTest.java index 301dcf693..d614fcfac 100644 --- a/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/CollectionCacheTest.java +++ b/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/CollectionCacheTest.java @@ -495,13 +495,13 @@ public void getExtensionAttributesLoaderByResourceTypeAndName() { assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); - assertEquals(Map.of("l1", apc1, "l2", apc2), collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl")); + assertNull(collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl")); assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); mockNetworkStoreClient.setExtensionAttributesLoaderByResourceTypeAndNameCalled(false); - assertEquals(Map.of("l1", apc1, "l2", apc2), collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "activePowerControl")); + assertNull(collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(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()); @@ -539,13 +539,13 @@ public void getExtensionAttributesLoaderByResourceType() { assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); - assertEquals(Map.of("l1", Map.of("activePowerControl", apc1, "operatingStatus", os1), "l2", Map.of("activePowerControl", apc2)), collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD)); + assertNull(collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD)); assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); mockNetworkStoreClient.setExtensionAttributesLoaderByResourceTypeCalled(false); - assertEquals(Map.of("l1", Map.of("activePowerControl", apc1, "operatingStatus", os1), "l2", Map.of("activePowerControl", apc2)), collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD)); + assertNull(collectionCache.getAllExtensionsAttributesByResourceType(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));