From ff357dd5dbc70b11ea814f8ca9318f816458cae8 Mon Sep 17 00:00:00 2001 From: Antoine Bouhours Date: Wed, 9 Oct 2024 14:12:45 +0200 Subject: [PATCH 1/3] 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)); From 95ef94661880dfebfc09f61abd267fc9724ec6d1 Mon Sep 17 00:00:00 2001 From: Antoine Bouhours Date: Thu, 17 Oct 2024 14:19:44 +0200 Subject: [PATCH 2/3] Ensure existing resources/extension attributes in the cache are not overwritten when loading from server (#467) Signed-off-by: BOUHOURS Antoine --- .../client/CachedNetworkStoreClientTest.java | 229 +++++++++++++++++- .../iidm/impl/CachedNetworkStoreClient.java | 3 +- .../store/iidm/impl/CollectionCache.java | 78 ++++-- .../store/iidm/impl/CollectionCacheTest.java | 205 +++++++++++++--- .../iidm/impl/MockNetworkStoreClient.java | 45 ++-- 5 files changed, 477 insertions(+), 83 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 b2351b3b1..7981cd808 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 @@ -495,7 +495,7 @@ public void testGetExtensionCache() throws IOException { String identifiableId = "GEN"; // Load the identifiable in the cache - loadIdentifiableToCache(identifiableId, networkUuid, cachedClient); + loadGeneratorToCache(identifiableId, networkUuid, cachedClient); // Two successive ExtensionAttributes retrieval, only the first should send a REST request, the second uses the cache ActivePowerControlAttributes apc1 = ActivePowerControlAttributes.builder() @@ -574,7 +574,7 @@ public void testGetExtensionsCache() throws IOException { String identifiableId = "GEN"; // Load the identifiable in the cache - loadIdentifiableToCache(identifiableId, networkUuid, cachedClient); + loadGeneratorToCache(identifiableId, networkUuid, cachedClient); // Two successive ExtensionAttributes retrieval, only the first should send a REST request, the second uses the cache ActivePowerControlAttributes apc1 = ActivePowerControlAttributes.builder() @@ -627,7 +627,7 @@ public void testGetExtensionCacheWithClonedNetwork() throws IOException { String targetVariantId = "new_variant"; String identifiableId = "GEN"; - loadIdentifiableToCache(identifiableId, networkUuid, cachedClient); + loadGeneratorToCache(identifiableId, networkUuid, cachedClient); ActivePowerControlAttributes apc1 = ActivePowerControlAttributes.builder() .droop(5.2) .participate(true) @@ -655,7 +655,162 @@ public void testGetExtensionCacheWithClonedNetwork() throws IOException { server.reset(); } - private void loadIdentifiableToCache(String identifiableId, UUID networkUuid, CachedNetworkStoreClient cachedClient) throws JsonProcessingException { + @Test + /* + * Following sequence should not overwrite resource in the cache + * getGenerator() + * getVoltageLevelGenerator() + * getGenerators() + */ + public void testGetExtensionOverwriteCache1() throws IOException { + CachedNetworkStoreClient cachedClient = new CachedNetworkStoreClient(new BufferedNetworkStoreClient(restStoreClient, ForkJoinPool.commonPool())); + UUID networkUuid = UUID.fromString("7928181c-7977-4592-ba19-88027e4254e4"); + String identifiableId = "GEN"; + + // Load the identifiable in the cache + loadGeneratorToCache(identifiableId, networkUuid, cachedClient); + + // Load extension attributes in the cache + loadExtensionAttributesToCache(networkUuid, identifiableId, cachedClient); + Optional extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load generators for voltage level VL_1 (by container), this should not overwrite existing resource in the cache + loadVoltageLevelGenerators(identifiableId, networkUuid, cachedClient); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load all generators, this should not overwrite existing resource in the cache + loadGeneratorCollection(identifiableId, networkUuid, cachedClient); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + } + + @Test + /* + * Following sequence should not overwrite resource in the cache + * getGenerator() + * getGenerators() + * getVoltageLevelGenerator() + */ + public void testGetExtensionOverwriteCache2() throws IOException { + CachedNetworkStoreClient cachedClient = new CachedNetworkStoreClient(new BufferedNetworkStoreClient(restStoreClient, ForkJoinPool.commonPool())); + UUID networkUuid = UUID.fromString("7928181c-7977-4592-ba19-88027e4254e4"); + String identifiableId = "GEN"; + + // Load the identifiable in the cache + loadGeneratorToCache(identifiableId, networkUuid, cachedClient); + + // Load extension attributes in the cache + loadExtensionAttributesToCache(networkUuid, identifiableId, cachedClient); + Optional extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load all generators, this should not overwrite existing resource in the cache + loadGeneratorCollection(identifiableId, networkUuid, cachedClient); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load generators for voltage level VL_1 (by container), this should not overwrite existing resource in the cache + cachedClient.getVoltageLevelGenerators(networkUuid, Resource.INITIAL_VARIANT_NUM, "VL_1"); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + } + + @Test + /* + * Following sequence should not overwrite resource in the cache + * getIdentifiable() + * getVoltageLevelGenerator() + * getGenerators() + */ + public void testGetExtensionOverwriteCache3() throws IOException { + CachedNetworkStoreClient cachedClient = new CachedNetworkStoreClient(new BufferedNetworkStoreClient(restStoreClient, ForkJoinPool.commonPool())); + UUID networkUuid = UUID.fromString("7928181c-7977-4592-ba19-88027e4254e4"); + String identifiableId = "GEN"; + + // Load the identifiable in the cache + loadIdentifiableToCache(identifiableId, networkUuid, cachedClient); + + // Load extension attributes in the cache + loadExtensionAttributesToCache(networkUuid, identifiableId, cachedClient); + Optional extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load generators for voltage level VL_1 (by container), this should not overwrite existing resource in the cache + loadVoltageLevelGenerators(identifiableId, networkUuid, cachedClient); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load all generators, this should not overwrite existing resource in the cache + loadGeneratorCollection(identifiableId, networkUuid, cachedClient); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + } + + @Test + /* + * Following sequence should not overwrite resource in the cache + * getIdentifiable() + * getGenerators() + * getVoltageLevelGenerator() + */ + public void testGetExtensionOverwriteCache4() throws IOException { + CachedNetworkStoreClient cachedClient = new CachedNetworkStoreClient(new BufferedNetworkStoreClient(restStoreClient, ForkJoinPool.commonPool())); + UUID networkUuid = UUID.fromString("7928181c-7977-4592-ba19-88027e4254e4"); + String identifiableId = "GEN"; + + // Load the identifiable in the cache + loadIdentifiableToCache(identifiableId, networkUuid, cachedClient); + + // Load extension attributes in the cache + loadExtensionAttributesToCache(networkUuid, identifiableId, cachedClient); + Optional extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load all generators, this should not overwrite existing resource in the cache + loadGeneratorCollection(identifiableId, networkUuid, cachedClient); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load generators for voltage level VL_1 (by container), this should not overwrite existing resource in the cache + cachedClient.getVoltageLevelGenerators(networkUuid, Resource.INITIAL_VARIANT_NUM, "VL_1"); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + } + + @Test + /* + * Following sequence should not overwrite resource in the cache + * getGenerators() + * getVoltageLevelGenerator() + * getGenerator() + */ + public void testGetExtensionOverwriteCache5() throws IOException { + CachedNetworkStoreClient cachedClient = new CachedNetworkStoreClient(new BufferedNetworkStoreClient(restStoreClient, ForkJoinPool.commonPool())); + UUID networkUuid = UUID.fromString("7928181c-7977-4592-ba19-88027e4254e4"); + String identifiableId = "GEN"; + + // Load all generators, this should not overwrite existing resource in the cache + loadGeneratorCollection(identifiableId, networkUuid, cachedClient); + + // Load extension attributes in the cache + loadExtensionAttributesToCache(networkUuid, identifiableId, cachedClient); + Optional extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load generators for voltage level VL_1 (by container), this should not overwrite existing resource in the cache + cachedClient.getVoltageLevelGenerators(networkUuid, Resource.INITIAL_VARIANT_NUM, "VL_1"); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + + // Load the identifiable in the cache, this should not overwrite existing resource in the cache + cachedClient.getGenerator(networkUuid, Resource.INITIAL_VARIANT_NUM, identifiableId); + extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + } + + private void loadGeneratorToCache(String identifiableId, UUID networkUuid, CachedNetworkStoreClient cachedClient) throws JsonProcessingException { Resource g1Resource = Resource.generatorBuilder() .id(identifiableId) .attributes(GeneratorAttributes.builder() @@ -670,4 +825,70 @@ private void loadIdentifiableToCache(String identifiableId, UUID networkUuid, Ca server.verify(); server.reset(); } + + private void loadIdentifiableToCache(String identifiableId, UUID networkUuid, CachedNetworkStoreClient cachedClient) throws JsonProcessingException { + Resource g1Resource = Resource.generatorBuilder() + .id(identifiableId) + .attributes(GeneratorAttributes.builder() + .voltageLevelId("VL_1") + .build()) + .build(); + String generatorJson = objectMapper.writeValueAsString(TopLevelDocument.of(List.of(g1Resource))); + server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/identifiables/" + identifiableId)) + .andExpect(method(GET)) + .andRespond(withSuccess(generatorJson, MediaType.APPLICATION_JSON)); + cachedClient.getIdentifiable(networkUuid, Resource.INITIAL_VARIANT_NUM, identifiableId); + server.verify(); + server.reset(); + } + + private void loadGeneratorCollection(String identifiableId, UUID networkUuid, CachedNetworkStoreClient cachedClient) throws JsonProcessingException { + String voltageLevelId = "VL_1"; + Resource g1Resource = Resource.generatorBuilder() + .id(identifiableId) + .attributes(GeneratorAttributes.builder() + .voltageLevelId(voltageLevelId) + .build()) + .build(); + String generatorJson = objectMapper.writeValueAsString(TopLevelDocument.of(List.of(g1Resource))); + server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/generators")) + .andExpect(method(GET)) + .andRespond(withSuccess(generatorJson, MediaType.APPLICATION_JSON)); + cachedClient.getGenerators(networkUuid, Resource.INITIAL_VARIANT_NUM); + server.verify(); + server.reset(); + } + + private void loadVoltageLevelGenerators(String identifiableId, UUID networkUuid, CachedNetworkStoreClient cachedClient) throws JsonProcessingException { + String voltageLevelId = "VL_1"; + Resource g1Resource = Resource.generatorBuilder() + .id(identifiableId) + .attributes(GeneratorAttributes.builder() + .voltageLevelId(voltageLevelId) + .build()) + .build(); + String generatorJson = objectMapper.writeValueAsString(TopLevelDocument.of(List.of(g1Resource))); + server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/voltage-levels/" + voltageLevelId + "/generators")) + .andExpect(method(GET)) + .andRespond(withSuccess(generatorJson, MediaType.APPLICATION_JSON)); + cachedClient.getVoltageLevelGenerators(networkUuid, Resource.INITIAL_VARIANT_NUM, voltageLevelId); + server.verify(); + server.reset(); + } + + private void loadExtensionAttributesToCache(UUID networkUuid, String identifiableId, CachedNetworkStoreClient cachedClient) throws JsonProcessingException { + ActivePowerControlAttributes apc1 = ActivePowerControlAttributes.builder() + .droop(5.2) + .participate(true) + .participationFactor(0.5) + .build(); + String oneExtensionAttributes = objectMapper.writeValueAsString(ExtensionAttributesTopLevelDocument.of(List.of(apc1))); + server.expect(ExpectedCount.once(), requestTo("/networks/" + networkUuid + "/" + Resource.INITIAL_VARIANT_NUM + "/identifiables/" + identifiableId + "/extensions/" + ActivePowerControl.NAME)) + .andExpect(method(GET)) + .andRespond(withSuccess(oneExtensionAttributes, MediaType.APPLICATION_JSON)); + Optional extensionAttributesResult = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME); + assertTrue(extensionAttributesResult.isPresent()); + server.verify(); + server.reset(); + } } diff --git a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CachedNetworkStoreClient.java b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CachedNetworkStoreClient.java index 0db15691b..89a3d9603 100644 --- a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CachedNetworkStoreClient.java +++ b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CachedNetworkStoreClient.java @@ -1175,7 +1175,8 @@ public Optional> getIdentifiable(UUID networkUu Optional> resource = delegate.getIdentifiable(networkUuid, variantNum, id); resource.ifPresent(r -> { CollectionCache collection = (CollectionCache) networkContainersCaches.get(r.getType()).getCollection(networkUuid, variantNum); - collection.addResource(r); + // we already checked that the resource is not in the cache so we can directly put it in the cache + collection.addOrReplaceResource(r); }); identifiableCallCountByNetworkVariant.computeIfAbsent(p, k -> new MutableInt()) 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 91e585ffb..90a385976 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 @@ -24,7 +24,11 @@ public class CollectionCache { /** - * Resources indexed by id. + * Resources indexed by id.
+ * We enforce a single resource per variant because they are referenced both in these maps + * and directly in any identifiable object created via the IIDM API.
+ * Overwriting a resource creates a new reference, which breaks synchronization with + * the IIDM object managed in the NetworkObjectIndex. */ private final Map> resources = new HashMap<>(); @@ -34,7 +38,11 @@ public class CollectionCache { private boolean fullyLoaded = false; /** - * Resources indexed by container id. A container is either a substation or a voltage level. + * Resources indexed by container id. A container is either a substation or a voltage level.
+ * We enforce a single resource per variant because they are referenced both in these maps + * and directly in any identifiable object created via the IIDM API.
+ * Overwriting a resource creates a new reference, which breaks synchronization with + * the IIDM object managed in the NetworkObjectIndex. */ private final Map>> resourcesByContainerId = new HashMap<>(); @@ -147,7 +155,8 @@ public Optional> getResource(UUID networkUuid, int variantNum, Strin resource = oneLoaderFunction.apply(networkUuid, variantNum, id).orElse(null); // if resource has been found on server side we add it to the cache if (resource != null) { - addResource(resource); + // we already checked that the resource is not in the cache so we can directly put it in the cache + addOrReplaceResource(resource); } } } @@ -161,8 +170,10 @@ private void loadAll(UUID networkUuid, int variantNum) { List> resourcesToAdd = allLoaderFunction.apply(networkUuid, variantNum); // we update the full cache and set it as fully loaded - // notice: it might overwrite already loaded resource (single or container) - resourcesToAdd.forEach(resource -> resources.put(resource.getId(), resource)); + // notice: even if it adds some checks and reduces performance by a tiny bit, we avoid to overwrite already + // loaded resource (single or container) because they are referenced in the resources or resourcesByContainerId map, + // but also directly in any identifiable with the iidm api. + resourcesToAdd.forEach(resource -> resources.putIfAbsent(resource.getId(), resource)); fullyLoaded = true; // we update by container cache @@ -172,7 +183,10 @@ private void loadAll(UUID networkUuid, int variantNum) { Set containerIds = ((Contained) attributes).getContainerIds(); containerIds.forEach(containerId -> { // we add container resources and update container fully loaded status - getResourcesByContainerId(containerId).put(resource.getId(), resource); + // notice: even if it adds some checks and reduces performance by a tiny bit, we avoid to overwrite already + // loaded resource (single or container) because they are referenced in the resources or resourcesByContainerId map, + // but also directly in any identifiable with the iidm api. + getResourcesByContainerId(containerId).putIfAbsent(resource.getId(), resource); containerFullyLoaded.add(containerId); }); } @@ -213,20 +227,27 @@ public List> getContainerResources(UUID networkUuid, int variantNum, List> resourcesToAdd = containerLoaderFunction.apply(networkUuid, variantNum, containerId) .stream().filter(resource -> !removedResources.contains(resource.getId())).collect(Collectors.toList()); - // by container cache update - getResourcesByContainerId(containerId).putAll(resourcesToAdd.stream().collect(Collectors.toMap(Resource::getId, resource -> resource))); - containerFullyLoaded.add(containerId); - - // full cache update resourcesToAdd.forEach(resource -> { - resources.put(resource.getId(), resource); - removedResources.remove(resource.getId()); + String resourceId = resource.getId(); + // notice: even if it adds some checks and reduces performance by a tiny bit, we avoid to overwrite already + // loaded resource (single or container) because they are referenced in the resources or resourcesByContainerId map, + // but also directly in any identifiable with the iidm api. + getResourcesByContainerId(containerId).putIfAbsent(resourceId, resource); + resources.putIfAbsent(resourceId, resource); + removedResources.remove(resourceId); }); + containerFullyLoaded.add(containerId); } return new ArrayList<>(getResourcesByContainerId(containerId).values()); } - public void addResource(Resource resource) { + /** + * Adds or replaces the given resource in the cache.
+ * If the resource already exists in the cache, it will be overridden. + * + * @param resource the resource to add or replace in the cache + */ + public void addOrReplaceResource(Resource resource) { Objects.requireNonNull(resource); // full cache update @@ -247,16 +268,21 @@ public void addResource(Resource resource) { * @param resource the newly created resources */ public void createResource(Resource resource) { - addResource(resource); + String resourceId = resource.getId(); + if (resources.containsKey(resourceId)) { + throw new PowsyblException("The collection cache already contains a " + resource.getType() + " with the id '" + resourceId + "'"); + } + // we already checked that the resource is not in the cache so we can directly put it in the cache + addOrReplaceResource(resource); } /** - * Update (replace) a resource of the collection. + * Replace resource of the collection with {@code resource}. * - * @param resource the resources to update + * @param resource the resource to update */ public void updateResource(Resource resource) { - addResource(resource); + addOrReplaceResource(resource); } /** @@ -379,12 +405,15 @@ private boolean isExtensionAttributesCached(String id, String extensionName) { } /** - * Add extension attributes in the cache for single extension attributes loading + * Add extension attributes in the cache for single extension attributes loading.
+ * This method is only used to get extension attributes from the server so even if it adds some checks and reduces performance by a tiny bit, + * we avoid to overwrite already loaded extension attributes because they are referenced in the extensionAttributes field of the resources or resourcesByContainerId map, + * but also directly in any identifiable with the iidm api. */ private void addExtensionAttributesToCache(String identifiableId, String extensionName, ExtensionAttributes extensionAttributes) { Objects.requireNonNull(extensionAttributes); - getCachedExtensionAttributes(identifiableId).put(extensionName, extensionAttributes); + getCachedExtensionAttributes(identifiableId).putIfAbsent(extensionName, extensionAttributes); Set extensions = removedExtensionAttributes.get(identifiableId); if (extensions != null) { extensions.remove(extensionName); @@ -428,7 +457,7 @@ public Map getAllExtensionsAttributesByIdentifiable Map extensionAttributes = delegate.getAllExtensionsAttributesByIdentifiableId(networkUuid, variantNum, type, identifiableId); if (extensionAttributes != null) { addAllExtensionAttributesToCache(identifiableId, extensionAttributes); - return extensionAttributes; + return getCachedExtensionAttributes(identifiableId); } } return Map.of(); @@ -443,12 +472,15 @@ private boolean isExtensionAttributesCached(String identifiableId) { } /** - * Add extension attributes to the cache when loading all the extension attributes of an identifiable + * Add extension attributes to the cache when loading all the extension attributes of an identifiable.
+ * This method is only used to get extension attributes from the server so even if it adds some checks and reduces performance by a tiny bit, + * we avoid to overwrite already loaded extension attributes because they are referenced in the extensionAttributes field of the resources or resourcesByContainerId map, + * but also directly in any identifiable with the iidm api. */ private void addAllExtensionAttributesToCache(String id, Map extensionAttributes) { Objects.requireNonNull(extensionAttributes); - getCachedExtensionAttributes(id).putAll(extensionAttributes); + extensionAttributes.forEach(getCachedExtensionAttributes(id)::putIfAbsent); fullyLoadedExtensionsByIdentifiableIds.add(id); removedExtensionAttributes.remove(id); } 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 d614fcfac..499987a00 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 @@ -6,7 +6,9 @@ */ package com.powsybl.network.store.iidm.impl; +import com.fasterxml.jackson.databind.ObjectMapper; import com.powsybl.commons.PowsyblException; +import com.powsybl.iidm.network.extensions.ActivePowerControl; import com.powsybl.network.store.iidm.impl.util.TriFunction; import com.powsybl.network.store.model.*; import org.junit.Before; @@ -51,30 +53,10 @@ public void setUp() { containerLoaderCalled = false; allLoaderCalled = false; - l1 = Resource.loadBuilder() - .id("l1") - .attributes(LoadAttributes.builder() - .voltageLevelId("vl1") - .build()) - .build(); - l2 = Resource.loadBuilder() - .id("l2") - .attributes(LoadAttributes.builder() - .voltageLevelId("vl1") - .build()) - .build(); - l3 = Resource.loadBuilder() - .id("l3") - .attributes(LoadAttributes.builder() - .voltageLevelId("vl2") - .build()) - .build(); - l4 = Resource.loadBuilder() - .id("l4") - .attributes(LoadAttributes.builder() - .voltageLevelId("vl2") - .build()) - .build(); + l1 = createResource("l1", "vl1"); + l2 = createResource("l2", "vl1"); + l3 = createResource("l3", "vl2"); + l4 = createResource("l4", "vl2"); apc1 = ActivePowerControlAttributes.builder() .droop(5.2) @@ -94,23 +76,20 @@ public void setUp() { oneLoader = (networkUuid, variantNum, id) -> { oneLoaderCalled = true; - if (id.equals("l1")) { - return Optional.of(l1); - } else if (id.equals("l2")) { - return Optional.of(l2); - } else if (id.equals("l3")) { - return Optional.of(l3); - } else { - return Optional.empty(); - } + return switch (id) { + case "l1" -> Optional.of(createResource("l1", "vl1")); + case "l2" -> Optional.of(createResource("l2", "vl1")); + case "l3" -> Optional.of(createResource("l3", "vl2")); + default -> Optional.empty(); + }; }; containerLoader = (networkUuid, variantNum, containerId) -> { containerLoaderCalled = true; if (containerId.equals("vl1")) { - return Arrays.asList(l1, l2); + return Arrays.asList(createResource("l1", "vl1"), createResource("l2", "vl1")); } else if (containerId.equals("vl2")) { - return Collections.singletonList(l3); + return Collections.singletonList(createResource("l3", "vl2")); } else { return Collections.emptyList(); } @@ -118,12 +97,25 @@ public void setUp() { allLoader = (networkUuid, variantNum) -> { allLoaderCalled = true; - return Arrays.asList(l1, l2, l3); + return Arrays.asList( + createResource("l1", "vl1"), + createResource("l2", "vl1"), + createResource("l3", "vl2") + ); }; - mockNetworkStoreClient = new MockNetworkStoreClient(apc1, apc2, os1); + mockNetworkStoreClient = new MockNetworkStoreClient(); collectionCache = new CollectionCache<>(oneLoader, containerLoader, allLoader, mockNetworkStoreClient); } + private Resource createResource(String id, String voltageLevelId) { + return Resource.loadBuilder() + .id(id) + .attributes(LoadAttributes.builder() + .voltageLevelId(voltageLevelId) + .build()) + .build(); + } + @Test public void getResourcesFirstTest() { assertFalse(oneLoaderCalled); @@ -426,7 +418,7 @@ public void extensionIsCachedAndResourceIsRemovedAndAddedAgainWithoutExtensions( .voltageLevelId("vl1") .build()) .build(); - collectionCache.addResource(l1WithoutExtensions); + collectionCache.createResource(l1WithoutExtensions); assertNull(collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl").orElse(null)); assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); @@ -456,7 +448,7 @@ public void extensionIsCachedAndResourceIsRemovedAndAddedAgainWithDifferentExten assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); l1.getAttributes().getExtensionAttributes().put("activePowerControl", apc2); - collectionCache.addResource(l1); + collectionCache.createResource(l1); assertEquals(apc2, collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl").orElse(null)); assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); @@ -554,4 +546,139 @@ public void getExtensionAttributesLoaderByResourceType() { assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); } + + @Test + public void createThrowTest() { + assertFalse(oneLoaderCalled); + assertFalse(containerLoaderCalled); + assertFalse(allLoaderCalled); + // Create a resource in the cache + collectionCache.createResource(l4); + // Create it again, it should throw because the resource already exists in the cache + // This does not happen when we use the IIDM api because we check if the resource already exists in the network + assertThrows(PowsyblException.class, () -> collectionCache.createResource(l4)); + } + + @Test + public void extensionGetExtensionThenUpdateThenGetExtensions() { + // Load resources in cache + assertEquals(l1, collectionCache.getResource(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, "l1").orElse(null)); + assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); + // Load extension in the resource + ActivePowerControlAttributes apc1Result = (ActivePowerControlAttributes) collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl").orElse(null); + assertEquals(apc1, apc1Result); + assertTrue(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); + mockNetworkStoreClient.setExtensionAttributeLoaderCalled(false); + // Update the extension + apc1Result.setDroop(8.7); + // Load extension again with getExtensions + Map extensionsAttributesByIdentifiableIdResult = collectionCache.getAllExtensionsAttributesByIdentifiableId(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1"); + apc1Result = (ActivePowerControlAttributes) extensionsAttributesByIdentifiableIdResult.get(ActivePowerControl.NAME); + ActivePowerControlAttributes updatedApc1 = ActivePowerControlAttributes.builder() + .droop(8.7) + .participate(true) + .participationFactor(0.5) + .build(); + assertEquals(updatedApc1, apc1Result); + assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); + assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); + } + + @Test + public void extensionGetExtensionThenUpdateThenGetExtensionPreloadingCollection() { + // Load resourcess in cache + assertEquals(l1, collectionCache.getResource(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, "l1").orElse(null)); + assertEquals(l2, collectionCache.getResource(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, "l2").orElse(null)); + assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); + // Load extension in the resource with getExtension in preloading collection + collectionCache.getAllExtensionsAttributesByResourceTypeAndExtensionName(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, ActivePowerControl.NAME); + ActivePowerControlAttributes apc1Result = (ActivePowerControlAttributes) collectionCache.getExtensionAttributes(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1", "activePowerControl").orElse(null); + assertEquals(apc1, apc1Result); + assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); + assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); + mockNetworkStoreClient.setExtensionAttributesLoaderByResourceTypeAndNameCalled(false); + // Update the extension in gen1 + apc1Result.setDroop(8.7); + // Load extension again with getExtensions in preloading collection + collectionCache.getAllExtensionsAttributesByResourceType(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD); + Map extensionsAttributesByIdentifiableIdResult = collectionCache.getAllExtensionsAttributesByIdentifiableId(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, ResourceType.LOAD, "l1"); + apc1Result = (ActivePowerControlAttributes) extensionsAttributesByIdentifiableIdResult.get(ActivePowerControl.NAME); + ActivePowerControlAttributes updatedApc1 = ActivePowerControlAttributes.builder() + .droop(8.7) + .participate(true) + .participationFactor(0.5) + .build(); + assertEquals(updatedApc1, apc1Result); + assertFalse(mockNetworkStoreClient.isExtensionAttributeLoaderCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeAndNameCalled()); + assertFalse(mockNetworkStoreClient.isExtensionAttributesLoaderByIdCalled()); + assertTrue(mockNetworkStoreClient.isExtensionAttributesLoaderByResourceTypeCalled()); + } + + @Test + public void cloneCollectionContainerLoading() { + // Load resources in cache + Resource l1Result = collectionCache.getResource(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, "l1").orElse(null); + assertEquals(l1, l1Result); + assertTrue(oneLoaderCalled); + assertFalse(containerLoaderCalled); + assertFalse(allLoaderCalled); + oneLoaderCalled = false; + // Modify resource in cache + l1Result.getAttributes().setVoltageLevelId("foo"); + // Load resources by container (this should not overwrite the cache) + collectionCache.getContainerResources(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, "vl1"); + assertFalse(oneLoaderCalled); + assertTrue(containerLoaderCalled); + assertFalse(allLoaderCalled); + containerLoaderCalled = false; + // Clone the cache and check if modification is still accounted for + int newVariantNum = 1; + collectionCache.clone(new ObjectMapper(), newVariantNum, null); + l1Result = collectionCache.getResource(NETWORK_UUID, newVariantNum, "l1").orElse(null); + assertEquals(createResource("l1", "foo"), l1Result); + assertFalse(oneLoaderCalled); + assertFalse(containerLoaderCalled); + assertFalse(allLoaderCalled); + } + + @Test + public void cloneCollectionFullCollectionLoading() { + // Load resources in cache + Resource l1Result = collectionCache.getResource(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM, "l1").orElse(null); + assertEquals(l1, l1Result); + assertTrue(oneLoaderCalled); + assertFalse(containerLoaderCalled); + assertFalse(allLoaderCalled); + oneLoaderCalled = false; + // Modify resource in cache + l1Result.getAttributes().setVoltageLevelId("foo"); + // Load resources (this should not overwrite the cache) + collectionCache.getResources(NETWORK_UUID, Resource.INITIAL_VARIANT_NUM); + assertFalse(oneLoaderCalled); + assertFalse(containerLoaderCalled); + assertTrue(allLoaderCalled); + allLoaderCalled = false; + // Clone the cache and check if modification is still accounted for + int newVariantNum = 1; + collectionCache.clone(new ObjectMapper(), newVariantNum, null); + l1Result = collectionCache.getResource(NETWORK_UUID, newVariantNum, "l1").orElse(null); + assertEquals(createResource("l1", "foo"), l1Result); + assertFalse(oneLoaderCalled); + assertFalse(containerLoaderCalled); + assertFalse(allLoaderCalled); + } } diff --git a/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/MockNetworkStoreClient.java b/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/MockNetworkStoreClient.java index 03316e2d4..df4718fc4 100644 --- a/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/MockNetworkStoreClient.java +++ b/network-store-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/MockNetworkStoreClient.java @@ -21,30 +21,21 @@ @Getter @Setter class MockNetworkStoreClient implements NetworkStoreClient { - private final ActivePowerControlAttributes apc1; - private final ActivePowerControlAttributes apc2; - private final OperatingStatusAttributes os1; private boolean extensionAttributeLoaderCalled = false; private boolean extensionAttributesLoaderByResourceTypeAndNameCalled = false; private boolean extensionAttributesLoaderByIdCalled = false; private boolean extensionAttributesLoaderByResourceTypeCalled = false; - public MockNetworkStoreClient(ActivePowerControlAttributes apc1, ActivePowerControlAttributes apc2, OperatingStatusAttributes os1) { - this.apc1 = apc1; - this.apc2 = apc2; - this.os1 = os1; - } - // Methods used in tests @Override public Optional getExtensionAttributes(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId, String extensionName) { extensionAttributeLoaderCalled = true; if (identifiableId.equals("l1") && extensionName.equals("operatingStatus")) { - return Optional.of(os1); + return Optional.of(createOperatinStatusAttributes()); } else if (identifiableId.equals("l1") && extensionName.equals("activePowerControl")) { - return Optional.of(apc1); + return Optional.of(createActivePowerControlAttributes1()); } else if (identifiableId.equals("l2") && extensionName.equals("activePowerControl")) { - return Optional.of(apc2); + return Optional.of(createActivePowerControlAttributes2()); } return Optional.empty(); } @@ -53,7 +44,7 @@ public Optional getExtensionAttributes(UUID networkUuid, in public Map getAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) { extensionAttributesLoaderByResourceTypeAndNameCalled = true; if (extensionName.equals("activePowerControl")) { - return Map.of("l1", apc1, "l2", apc2); + return Map.of("l1", createActivePowerControlAttributes1(), "l2", createActivePowerControlAttributes2()); } return Map.of(); } @@ -62,9 +53,9 @@ public Map getAllExtensionsAttributesByResourceType public Map getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) { extensionAttributesLoaderByIdCalled = true; if (identifiableId.equals("l1")) { - return Map.of("activePowerControl", apc1, "operatingStatus", os1); + return Map.of("activePowerControl", createActivePowerControlAttributes1(), "operatingStatus", createOperatinStatusAttributes()); } else if (identifiableId.equals("l2")) { - return Map.of("activePowerControl", apc2); + return Map.of("activePowerControl", createActivePowerControlAttributes2()); } return Map.of(); } @@ -73,11 +64,33 @@ public Map getAllExtensionsAttributesByIdentifiable public Map> getAllExtensionsAttributesByResourceType(UUID networkUuid, int variantNum, ResourceType resourceType) { extensionAttributesLoaderByResourceTypeCalled = true; if (resourceType == ResourceType.LOAD) { - return Map.of("l1", Map.of("activePowerControl", apc1, "operatingStatus", os1), "l2", Map.of("activePowerControl", apc2)); + return Map.of("l1", Map.of("activePowerControl", createActivePowerControlAttributes1(), "operatingStatus", createOperatinStatusAttributes()), "l2", Map.of("activePowerControl", createActivePowerControlAttributes2())); } return Map.of(); } + private ActivePowerControlAttributes createActivePowerControlAttributes1() { + return ActivePowerControlAttributes.builder() + .droop(5.2) + .participate(true) + .participationFactor(0.5) + .build(); + } + + private ActivePowerControlAttributes createActivePowerControlAttributes2() { + return ActivePowerControlAttributes.builder() + .droop(6) + .participate(false) + .participationFactor(0.5) + .build(); + } + + private OperatingStatusAttributes createOperatinStatusAttributes() { + return OperatingStatusAttributes.builder() + .operatingStatus("foo") + .build(); + } + // Methods below are not used in tests @Override public void removeExtensionAttributes(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId, String extensionName) { From 9da0e416c5af740ac731fdd55a398136f455cea0 Mon Sep 17 00:00:00 2001 From: Joris Mancini <53527338+TheMaskedTurtle@users.noreply.github.com> Date: Fri, 18 Oct 2024 15:44:23 +0200 Subject: [PATCH 3/3] Bump to 1.19.0 (#471) Signed-off-by: Joris Mancini --- network-store-client-distribution/pom.xml | 2 +- network-store-client/pom.xml | 2 +- network-store-iidm-impl/pom.xml | 2 +- network-store-model/pom.xml | 2 +- pom.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/network-store-client-distribution/pom.xml b/network-store-client-distribution/pom.xml index eb9704f61..cce050eec 100644 --- a/network-store-client-distribution/pom.xml +++ b/network-store-client-distribution/pom.xml @@ -15,7 +15,7 @@ com.powsybl powsybl-network-store-client-parent - 1.19.0-SNAPSHOT + 1.19.0 powsybl-network-store-client-distribution diff --git a/network-store-client/pom.xml b/network-store-client/pom.xml index 616a56e2b..d39bb6a36 100644 --- a/network-store-client/pom.xml +++ b/network-store-client/pom.xml @@ -15,7 +15,7 @@ com.powsybl powsybl-network-store-client-parent - 1.19.0-SNAPSHOT + 1.19.0 powsybl-network-store-client diff --git a/network-store-iidm-impl/pom.xml b/network-store-iidm-impl/pom.xml index f4c000b15..9a6c160aa 100644 --- a/network-store-iidm-impl/pom.xml +++ b/network-store-iidm-impl/pom.xml @@ -12,7 +12,7 @@ powsybl-network-store-client-parent com.powsybl - 1.19.0-SNAPSHOT + 1.19.0 powsybl-network-store-iidm-impl diff --git a/network-store-model/pom.xml b/network-store-model/pom.xml index 8f92b1717..6d2119aac 100644 --- a/network-store-model/pom.xml +++ b/network-store-model/pom.xml @@ -13,7 +13,7 @@ com.powsybl powsybl-network-store-client-parent - 1.19.0-SNAPSHOT + 1.19.0 powsybl-network-store-model diff --git a/pom.xml b/pom.xml index e25b7a6b8..f7a3e40a8 100644 --- a/pom.xml +++ b/pom.xml @@ -18,7 +18,7 @@ powsybl-network-store-client-parent - 1.19.0-SNAPSHOT + 1.19.0 pom Network store client parent