Skip to content

Commit

Permalink
Fix looking on all resources when retrieving extensions for preloadin…
Browse files Browse the repository at this point in the history 
…g collection (#463)

Signed-off-by: BOUHOURS Antoine <[email protected]>
  • Loading branch information
@antoinebhs
antoinebhs authored Oct 9, 2024
1 parent 9436977 commit ff357dd
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 14 deletions.
5 changes: 3 additions & 2 deletions ...e-client/src/test/java/com/powsybl/network/store/client/CachedNetworkStoreClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ExtensionAttributes> apc1Attributes = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME);
Optional<ExtensionAttributes> apc1Attributes = cachedClient.getExtensionAttributes(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId, ActivePowerControl.NAME);
assertTrue(apc1Attributes.isPresent());
Optional<ExtensionAttributes> os1Attributes = cachedClient.getExtensionAttributes(networkUuid, Resource.INITIAL_VARIANT_NUM, ResourceType.GENERATOR, identifiableId, OperatingStatus.NAME);
Optional<ExtensionAttributes> os1Attributes = cachedClient.getExtensionAttributes(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId, OperatingStatus.NAME);
assertFalse(os1Attributes.isPresent());
server.verify();
server.reset();
Expand Down
54 changes: 54 additions & 0 deletions ...ient/src/test/java/com/powsybl/network/store/client/PreloadingNetworkStoreClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Map<String, Map<String, ExtensionAttributes>>>() {
}).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<String, ExtensionAttributes> extensionAttributesByExtensionNameMap = cachedClient.getAllExtensionsAttributesByIdentifiableId(networkUuid, targetVariantNum, ResourceType.GENERATOR, identifiableId1);
assertEquals(2, extensionAttributesByExtensionNameMap.size());
server.verify();
server.reset();
}
}
22 changes: 14 additions & 8 deletions ...rk-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/CollectionCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,13 @@ public Map<String, ExtensionAttributes> 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;
}

/**
Expand Down Expand Up @@ -462,10 +465,13 @@ public Map<String, Map<String, ExtensionAttributes>> 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) {
Expand Down
8 changes: 4 additions & 4 deletions ...tore-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/CollectionCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit ff357dd

Please sign in to comment.