From 195201dcbc7ac84e7be2f9cd85ed200e6e36a40b Mon Sep 17 00:00:00 2001 From: BOUHOURS Antoine Date: Wed, 9 Oct 2024 17:49:20 +0200 Subject: [PATCH] Code improvements Signed-off-by: BOUHOURS Antoine --- .../client/PreloadingNetworkStoreClient.java | 4 ++-- .../store/client/RestNetworkStoreClient.java | 10 --------- .../PreloadingNetworkStoreClientTest.java | 7 +++--- .../iidm/impl/CachedNetworkStoreClient.java | 2 -- .../store/iidm/impl/CollectionCache.java | 22 ++++--------------- .../store/iidm/impl/NetworkStoreClient.java | 4 ---- .../iidm/impl/OfflineNetworkStoreClient.java | 10 --------- .../store/iidm/impl/CollectionCacheTest.java | 12 +++++----- .../iidm/impl/MockNetworkStoreClient.java | 10 --------- 9 files changed, 15 insertions(+), 66 deletions(-) diff --git a/network-store-client/src/main/java/com/powsybl/network/store/client/PreloadingNetworkStoreClient.java b/network-store-client/src/main/java/com/powsybl/network/store/client/PreloadingNetworkStoreClient.java index 5ec34c74c..7b0cc3969 100644 --- a/network-store-client/src/main/java/com/powsybl/network/store/client/PreloadingNetworkStoreClient.java +++ b/network-store-client/src/main/java/com/powsybl/network/store/client/PreloadingNetworkStoreClient.java @@ -874,13 +874,13 @@ public void removeConfiguredBuses(UUID networkUuid, int variantNum, List @Override public Optional 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 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); } } diff --git a/network-store-client/src/main/java/com/powsybl/network/store/client/RestNetworkStoreClient.java b/network-store-client/src/main/java/com/powsybl/network/store/client/RestNetworkStoreClient.java index 3b17e27e2..c94cdebf8 100644 --- a/network-store-client/src/main/java/com/powsybl/network/store/client/RestNetworkStoreClient.java +++ b/network-store-client/src/main/java/com/powsybl/network/store/client/RestNetworkStoreClient.java @@ -902,11 +902,6 @@ public Map 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 getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) { return getExtensionAttributesMap("/networks/{networkUuid}/{variantNum}/identifiables/{identifiableId}/extensions", networkUuid, variantNum, identifiableId); @@ -917,11 +912,6 @@ public Map> 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); 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 dbe87a1d2..f5634b6db 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 @@ -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(); @@ -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 extensionAttributesByExtensionNameMap = cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId1); - assertEquals(2, extensionAttributesByExtensionNameMap.size()); + Map extensionAttributesByIdentifiableId = cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId1); + assertEquals(2, extensionAttributesByIdentifiableId.size()); 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 ce0a1e15f..d902c748e 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 @@ -1076,7 +1076,6 @@ public Optional 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); } @@ -1086,7 +1085,6 @@ public Map 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); } 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..a8708d246 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 @@ -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 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 extensionAttributesMap = delegate.getAllExtensionsAttributesByResourceTypeAndExtensionName(networkUuid, variantNum, type, extensionName); @@ -406,13 +406,6 @@ public Map 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; } /** @@ -454,9 +447,9 @@ private void addAllExtensionAttributesToCache(String id, Map> 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> extensionAttributesMap = delegate.getAllExtensionsAttributesByResourceType(networkUuid, variantNum, type); @@ -465,13 +458,6 @@ public Map> 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) { diff --git a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/NetworkStoreClient.java b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/NetworkStoreClient.java index 8f0f441cd..785309dfc 100644 --- a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/NetworkStoreClient.java +++ b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/NetworkStoreClient.java @@ -318,8 +318,6 @@ public interface NetworkStoreClient { */ Map 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}. @@ -333,8 +331,6 @@ public interface NetworkStoreClient { */ Map> 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> getIdentifiable(UUID networkUuid, int variantNum, String id); diff --git a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/OfflineNetworkStoreClient.java b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/OfflineNetworkStoreClient.java index 28d1f6b48..fa3d46323 100644 --- a/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/OfflineNetworkStoreClient.java +++ b/network-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/OfflineNetworkStoreClient.java @@ -616,11 +616,6 @@ public Map getAllExtensionsAttributesByResourceType return Map.of(); } - @Override - public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) { - // nothing to do - } - @Override public Map getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) { return Map.of(); @@ -631,11 +626,6 @@ public Map> 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 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..834f0cf2b 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 @@ -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")); } @@ -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()); @@ -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)); 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 2eb418d1a..03316e2d4 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 @@ -58,11 +58,6 @@ public Map getAllExtensionsAttributesByResourceType return Map.of(); } - @Override - public void loadAllExtensionsAttributesByResourceTypeAndExtensionName(UUID networkUuid, int variantNum, ResourceType resourceType, String extensionName) { -//TODO - } - @Override public Map getAllExtensionsAttributesByIdentifiableId(UUID networkUuid, int variantNum, ResourceType resourceType, String identifiableId) { extensionAttributesLoaderByIdCalled = true; @@ -83,11 +78,6 @@ public Map> 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) {