diff --git a/src/main/java/org/phoebus/channelfinder/service/PropertyService.java b/src/main/java/org/phoebus/channelfinder/service/PropertyService.java index d9e6614c..66891a05 100644 --- a/src/main/java/org/phoebus/channelfinder/service/PropertyService.java +++ b/src/main/java/org/phoebus/channelfinder/service/PropertyService.java @@ -6,9 +6,11 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; import org.phoebus.channelfinder.common.TextUtil; @@ -204,7 +206,7 @@ public Iterable update(Iterable properties) { return properties; } - public void remove(String propertyName) { + public void removeBatch(String propertyName) { requireRole(ROLES.CF_PROPERTY, propertyName); Property existing = @@ -232,6 +234,61 @@ public void removeSingle(String propertyName, String channelName) { channelRepository.index(channel); } + /** + * Remove property from multiple channels. + * + *

Best-effort processing: non-existent channels are ignored, duplicates are deduplicated. Only + * channels where the property actually existed contribute to the returned count. + * + * @param propertyName name of the property to remove + * @param channelNames list of channel names to remove property from + * @return count of channels where property was actually removed + * @throws PropertyNotFoundException if property does not exist + * @throws UnauthorizedException if user lacks CF_PROPERTY role or is not property owner + */ + public long removeBatch(String propertyName, List channelNames) { + requireRole(ROLES.CF_PROPERTY, propertyName); + + Property property = + propertyRepository + .findById(propertyName) + .orElseThrow(() -> new PropertyNotFoundException(propertyName)); + requireOwner(property); + + // Normalize and deduplicate incoming channel names + Set uniqueChannelNames = new HashSet<>(channelNames); + + if (uniqueChannelNames.isEmpty()) { + return 0; + } + + List existingChannels = channelRepository.findAllById(uniqueChannelNames); + long removedCount = 0; + List modifiedChannels = new ArrayList<>(); + + for (Channel channel : existingChannels) { + // Check if property exists before removal + boolean hadProperty = + channel.getProperties().stream().anyMatch(p -> p.getName().equals(propertyName)); + + if (hadProperty) { + // Create a minimal Property object for removal (matching removeSingle pattern) + channel.removeProperty(new Property(propertyName, "")); + removedCount++; + modifiedChannels.add(channel); + audit.log( + Level.INFO, + () -> + MessageFormat.format( + "Removed property {0} from channel {1}", propertyName, channel.getName())); + } + } + if (!modifiedChannels.isEmpty()) { + channelRepository.indexAll(modifiedChannels); + } + return removedCount; + } + private void propagateRenameToChannels( String oldPropertyName, Property updated, List existingChannels) { if (existingChannels.isEmpty()) return; diff --git a/src/main/java/org/phoebus/channelfinder/web/v0/api/IProperty.java b/src/main/java/org/phoebus/channelfinder/web/v0/api/IProperty.java index e5757774..0a502bbe 100644 --- a/src/main/java/org/phoebus/channelfinder/web/v0/api/IProperty.java +++ b/src/main/java/org/phoebus/channelfinder/web/v0/api/IProperty.java @@ -6,6 +6,7 @@ import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.responses.ApiResponses; +import java.util.List; import org.phoebus.channelfinder.web.v0.dto.PropertyDto; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; @@ -234,7 +235,7 @@ PropertyDto update( content = @Content(schema = @Schema(implementation = ResponseStatusException.class))) }) @DeleteMapping("/{propertyName}") - void remove(@PathVariable("propertyName") String propertyName); + void removeBatch(@PathVariable("propertyName") String propertyName); @Operation( summary = "Delete property from a channel", @@ -263,11 +264,31 @@ void removeSingle( @PathVariable("propertyName") String propertyName, @PathVariable("channelName") String channelName); - /** - * Checks if 1. the property name is not null and matches the name in the body 2. the property - * owner is not null or empty 3. all the listed channels exist and have the property with a non - * null and non empty value - * - * @param property validate property - */ + @Operation( + summary = "Delete property from multiple channels", + description = + "Remove the specified property from a list of channels. Best-effort processing: non-existent channels are ignored, duplicates are deduplicated. Returns count of channels where property was actually removed.", + operationId = "deletePropertyFromChannels", + tags = {"Property"}) + @ApiResponses( + value = { + @ApiResponse( + responseCode = "200", + description = "Number of channels where property was removed"), + @ApiResponse( + responseCode = "401", + description = "Unauthorized", + content = @Content(schema = @Schema(implementation = ResponseStatusException.class))), + @ApiResponse( + responseCode = "404", + description = "Property not found", + content = @Content(schema = @Schema(implementation = ResponseStatusException.class))), + @ApiResponse( + responseCode = "500", + description = "Error while trying to remove property from channels", + content = @Content(schema = @Schema(implementation = ResponseStatusException.class))) + }) + @DeleteMapping("/{propertyName}/channels") + long removeBatch( + @PathVariable("propertyName") String propertyName, @RequestBody List channelNames); } diff --git a/src/main/java/org/phoebus/channelfinder/web/v0/controller/PropertyController.java b/src/main/java/org/phoebus/channelfinder/web/v0/controller/PropertyController.java index 8bb15cb9..0ea4f8f8 100644 --- a/src/main/java/org/phoebus/channelfinder/web/v0/controller/PropertyController.java +++ b/src/main/java/org/phoebus/channelfinder/web/v0/controller/PropertyController.java @@ -1,5 +1,6 @@ package org.phoebus.channelfinder.web.v0.controller; +import java.util.List; import org.phoebus.channelfinder.service.PropertyService; import org.phoebus.channelfinder.web.v0.api.IProperty; import org.phoebus.channelfinder.web.v0.dto.PropertyDto; @@ -58,12 +59,17 @@ public Iterable update(Iterable properties) { } @Override - public void remove(String propertyName) { - propertyService.remove(propertyName); + public void removeBatch(String propertyName) { + propertyService.removeBatch(propertyName); } @Override public void removeSingle(String propertyName, String channelName) { propertyService.removeSingle(propertyName, channelName); } + + @Override + public long removeBatch(String propertyName, List channelNames) { + return propertyService.removeBatch(propertyName, channelNames); + } } diff --git a/src/test/java/org/phoebus/channelfinder/PropertyControllerIT.java b/src/test/java/org/phoebus/channelfinder/PropertyControllerIT.java index aa6fea15..0cccef34 100644 --- a/src/test/java/org/phoebus/channelfinder/PropertyControllerIT.java +++ b/src/test/java/org/phoebus/channelfinder/PropertyControllerIT.java @@ -24,6 +24,7 @@ import org.phoebus.channelfinder.repository.PropertyRepository; import org.phoebus.channelfinder.web.v0.api.IProperty; import org.phoebus.channelfinder.web.v0.controller.PropertyController; +import org.phoebus.channelfinder.web.v0.mapper.PropertyMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; import org.springframework.security.test.context.support.WithMockUser; @@ -57,21 +58,24 @@ void setupAll() { // Helper operations to create and clean up the resources needed for successful // testing of the PropertyManager operations - private final Channel testChannel0 = new Channel("testChannel0", "testOwner"); - private final Channel testChannel1 = new Channel("testChannel1", "testOwner"); - private final Channel testChannelX = new Channel("testChannelX", "testOwner"); + private Channel testChannel0; + private Channel testChannel1; + private Channel testChannelX; - private final List testChannels = - Arrays.asList(testChannel0, testChannel1, testChannelX); + private List testChannels; @BeforeEach - public void setup() { + void setup() { + testChannel0 = new Channel("testChannel0", "testOwner"); + testChannel1 = new Channel("testChannel1", "testOwner"); + testChannelX = new Channel("testChannelX", "testOwner"); + testChannels = Arrays.asList(testChannel0, testChannel1, testChannelX); helper = new PropertyControllerHelper(propertyManager); channelRepository.indexAll(testChannels); } @AfterEach - public void cleanup() { + void cleanup() { MultiValueMap map = new LinkedMultiValueMap<>(); map.set("~name", "*"); @@ -89,7 +93,7 @@ void tearDown() throws IOException { /** list all properties */ @Test - void listXmlProperties() { + void listProperties() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty1 = new Property("testProperty1", "testOwner1"); testProperty1.setChannels( @@ -113,7 +117,7 @@ void listXmlProperties() { /** read a single property test the "withChannels" flag */ @Test - void readXmlProperty() { + void readProperty() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty1 = new Property("testProperty1", "testOwner"); testProperty1.setChannels( @@ -150,7 +154,7 @@ void readXmlProperty() { /** attempt to read a single non existent property */ @Test - void readNonExistingXmlProperty() { + void readNonExistingProperty() { // verify the property failed to be read, as expected Assertions.assertThrows( PropertyNotFoundException.class, () -> propertyManager.read("fakeProperty", false)); @@ -158,7 +162,7 @@ void readNonExistingXmlProperty() { /** attempt to read a single non existent property with channels */ @Test - void readNonExistingXmlProperty2() { + void readNonExistingProperty2() { // verify the property failed to be read, as expected Assertions.assertThrows( PropertyNotFoundException.class, () -> propertyManager.read("fakeProperty", true)); @@ -166,7 +170,7 @@ void readNonExistingXmlProperty2() { /** create a simple property */ @Test - void createXmlProperty() { + void createProperty() { Property testProperty0 = new Property("testProperty0", "testOwner"); // Create a simple property @@ -185,7 +189,7 @@ void createXmlProperty() { /** Rename a simple property using create */ @Test - void renameByCreateXmlProperty() { + void renameByCreateProperty() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty1 = new Property("testProperty1", "testOwner"); @@ -201,7 +205,7 @@ void renameByCreateXmlProperty() { /** create a single property with channels */ @Test - void createXmlProperty2() { + void createProperty2() { Property testProperty0WithChannels = new Property("testProperty0WithChannels", "testOwner"); testProperty0WithChannels.setChannels( Arrays.asList( @@ -257,7 +261,7 @@ void createXmlProperty2() { /** Rename a single property with channels using create */ @Test - void renameByCreateXmlProperty2() { + void renameByCreateProperty2() { Property testProperty0WithChannels = new Property("testProperty0WithChannels", "testOwner"); testProperty0WithChannels.setChannels( Arrays.asList( @@ -304,7 +308,7 @@ void renameByCreateXmlProperty2() { /** create multiple properties */ @Test - void createXmlProperties() { + void createProperties() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty1 = new Property("testProperty1", "testOwner"); Property testProperty2 = new Property("testProperty2", "testOwner"); @@ -388,7 +392,7 @@ void createXmlProperties() { /** create by overriding multiple properties */ @Test - void createXmlPropertiesWithOverride() { + void createPropertiesWithOverride() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty0WithChannels = new Property("testProperty0WithChannels", "testOwner"); testProperty0WithChannels.setChannels( @@ -454,7 +458,7 @@ void createXmlPropertiesWithOverride() { * add a single property to a single channel @Todo fix this test after addsingle method is fixed */ @Test - void addSingleXmlProperty() { + void addSingleProperty() { Property testProperty0 = new Property("testProperty0", "testOwner"); propertyRepository.index(testProperty0); testProperty0.setValue("value"); @@ -471,7 +475,7 @@ void addSingleXmlProperty() { /** update a property */ @Test - void updateXmlProperty() { + void updateProperty() { // A test property with only name and owner Property testProperty0 = new Property("testProperty0", "testOwner"); // A test property with name, owner, and a single test channel with a copy of the property with @@ -546,7 +550,7 @@ void updateXmlProperty() { /** update a property's name and owner and value on its channels */ @Test - void updateXmlPropertyOnChan() { + void updatePropertyOnChan() { // extra channel for this test Channel testChannelX = new Channel("testChannelX", "testOwner"); channelRepository.index(testChannelX); @@ -687,7 +691,7 @@ void updateXmlPropertyOnChan() { /** Rename a property using update */ @Test - void renameByUpdateXmlProperty() { + void renameByUpdateProperty() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty1 = new Property("testProperty1", "testOwner"); Property testProperty0WithChannels = new Property("testProperty0WithChannels", "testOwner"); @@ -1165,7 +1169,7 @@ void updateMultipleProperties() { /** update properties' names and values and attempt to change owners on their channels */ @Test - void updateMultipleXmlPropertiesOnChan() { + void updateMultiplePropertiesOnChan() { // extra channel for this test Channel testChannelX = new Channel("testChannelX", "testOwner"); channelRepository.index(testChannelX); @@ -1324,7 +1328,7 @@ void updateMultipleXmlPropertiesOnChan() { /** delete a single property */ @Test - void deleteXmlProperty() { + void deleteProperty() { Property testProperty0 = new Property("testProperty0", "testOwner"); Property testProperty0WithChannels = new Property("testProperty0WithChannels", "testOwner"); testProperty0WithChannels.setChannels( @@ -1342,12 +1346,12 @@ void deleteXmlProperty() { helper.apiCreate(testProperties); - propertyManager.remove(testProperty0.getName()); + propertyManager.removeBatch(testProperty0.getName()); // verify the property was deleted as expected Assertions.assertFalse( propertyRepository.existsById(testProperty0.getName()), "Failed to delete the property"); - propertyManager.remove(testProperty0WithChannels.getName()); + propertyManager.removeBatch(testProperty0WithChannels.getName()); MultiValueMap params = new LinkedMultiValueMap(); params.add("testProperty0WithChannels", "*"); // verify the property was deleted and removed from all associated channels @@ -1362,7 +1366,7 @@ void deleteXmlProperty() { /** delete a single property from a single channel */ @Test - void deleteXmlPropertyFromChannel() { + void deletePropertyFromChannel() { Property testProperty0WithChannels = new Property("testProperty0WithChannels", "testOwner"); testProperty0WithChannels.setChannels( Arrays.asList( @@ -1396,4 +1400,126 @@ void deleteXmlPropertyFromChannel() { }), "Failed to delete the property from channel"); } + + /** delete a property from multiple channels */ + @Test + void deletePropertyFromMultipleChannels() { + Property testProperty = new Property("batchTestProperty", "testOwner"); + testProperty.setChannels( + Arrays.asList( + new Channel( + testChannel0.getName(), + testChannel0.getOwner(), + Arrays.asList( + new Property(testProperty.getName(), testProperty.getOwner(), "value0")), + new ArrayList()), + new Channel( + testChannel1.getName(), + testChannel1.getOwner(), + Arrays.asList( + new Property(testProperty.getName(), testProperty.getOwner(), "value1")), + new ArrayList()))); + + propertyManager.create(testProperty.getName(), PropertyMapper.toDto(testProperty)); + + // Batch delete from both channels + long deletedCount = + propertyManager.removeBatch( + testProperty.getName(), Arrays.asList(testChannel0.getName(), testChannel1.getName())); + + // Verify 2 channels had the property removed + Assertions.assertEquals(2, deletedCount, "Failed to remove property from 2 channels"); + + // Verify the property still exists globally + Assertions.assertTrue( + propertyRepository.existsById(testProperty.getName()), + "Property should still exist globally"); + + // Verify the property was removed from all channels + MultiValueMap searchParams = new LinkedMultiValueMap(); + searchParams.add(testProperty.getName(), "*"); + Assertions.assertEquals( + 0, + channelRepository.search(searchParams).channels().size(), + "Failed to remove property from channels"); + } + + /** delete a property from multiple channels with mixed existing and non-existent channels */ + @Test + void deletePropertyFromMixedChannels() { + Property testProperty = new Property("batchTestProperty2", "testOwner"); + testProperty.setChannels( + Arrays.asList( + new Channel( + testChannel0.getName(), + testChannel0.getOwner(), + Arrays.asList( + new Property(testProperty.getName(), testProperty.getOwner(), "value")), + new ArrayList()))); + + propertyManager.create(testProperty.getName(), PropertyMapper.toDto(testProperty)); + + // Batch delete from existing and non-existent channels + long deletedCount = + propertyManager.removeBatch( + testProperty.getName(), + Arrays.asList(testChannel0.getName(), "nonExistentChannel", "anotherFakeChannel")); + + // Verify only 1 channel (testChannel0) had the property removed + Assertions.assertEquals( + 1, deletedCount, "Should only count the property removed from existing channel"); + + // Verify the property still exists + Assertions.assertTrue( + propertyRepository.existsById(testProperty.getName()), "Property should still exist"); + } + + /** delete a property from multiple channels with duplicates */ + @Test + void deletePropertyFromChannelsWithDuplicates() { + Property testProperty = new Property("batchTestProperty3", "testOwner"); + testProperty.setChannels( + Arrays.asList( + new Channel( + testChannel0.getName(), + testChannel0.getOwner(), + Arrays.asList( + new Property(testProperty.getName(), testProperty.getOwner(), "value")), + new ArrayList()))); + + propertyManager.create(testProperty.getName(), PropertyMapper.toDto(testProperty)); + + // Batch delete with duplicate channel names + long deletedCount = + propertyManager.removeBatch( + testProperty.getName(), + Arrays.asList(testChannel0.getName(), testChannel0.getName(), testChannel0.getName())); + + // Verify only 1 channel processed (duplicates deduplicated) + Assertions.assertEquals( + 1, deletedCount, "Should deduplicate and only remove from channel once"); + + // Verify the property is gone from the channel + Assertions.assertTrue( + channelRepository.findById(testChannel0.getName()).get().getProperties().stream() + .noneMatch(p -> p.getName().equals(testProperty.getName())), + "Property should be removed from channel"); + } + + /** delete a property from empty channel list returns 0 */ + @Test + void deletePropertyFromEmptyChannelList() { + Property testProperty = new Property("batchTestProperty4", "testOwner"); + propertyManager.create(testProperty.getName(), PropertyMapper.toDto(testProperty)); + + // Batch delete with empty channel list + long deletedCount = propertyManager.removeBatch(testProperty.getName(), List.of()); + + // Verify 0 channels affected + Assertions.assertEquals(0, deletedCount, "Should return 0 when channel list is empty"); + + // Verify the property still exists + Assertions.assertTrue( + propertyRepository.existsById(testProperty.getName()), "Property should still exist"); + } } diff --git a/src/test/java/org/phoebus/channelfinder/PropertyRepositoryIT.java b/src/test/java/org/phoebus/channelfinder/PropertyRepositoryIT.java index 9f82ede2..3fc61b06 100644 --- a/src/test/java/org/phoebus/channelfinder/PropertyRepositoryIT.java +++ b/src/test/java/org/phoebus/channelfinder/PropertyRepositoryIT.java @@ -47,7 +47,7 @@ void tearDown() throws IOException { /** index a single property */ @Test - void indexXmlProperty() { + void indexProperty() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property createdProperty = propertyRepository.index(testProperty); @@ -57,7 +57,7 @@ void indexXmlProperty() { /** index multiple properties */ @Test - void indexXmlProperties() { + void indexProperties() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property testProperty1 = new Property(TEST_PROPERTY_NAME + 1, "testOwner1"); List testProperties = Arrays.asList(testProperty, testProperty1); @@ -71,7 +71,7 @@ void indexXmlProperties() { /** save a single property */ @Test - void saveXmlProperty() { + void saveProperty() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property updateTestProperty = new Property(TEST_PROPERTY_NAME, "updateTestOwner"); Property updateTestProperty1 = new Property(TEST_PROPERTY_NAME + 1, "updateTestOwner1"); @@ -95,7 +95,7 @@ void saveXmlProperty() { /** save multiple properties */ @Test - void saveXmlProperties() { + void saveProperties() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property updateTestProperty = new Property(TEST_PROPERTY_NAME, "updateTestOwner"); Property testProperty1 = new Property(TEST_PROPERTY_NAME + 1, "testOwner1"); @@ -113,7 +113,7 @@ void saveXmlProperties() { /** find a single property */ @Test - void findXmlProperty() { + void findProperty() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Optional notFoundProperty = propertyRepository.findById(testProperty.getName()); @@ -164,7 +164,7 @@ void testPropertyExists() { /** find all properties */ @Test - void findAllXmlProperties() { + void findAllProperties() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property testProperty1 = new Property(TEST_PROPERTY_NAME + 1, "testOwner1"); List testProperties = Arrays.asList(testProperty, testProperty1); @@ -183,7 +183,7 @@ void findAllXmlProperties() { /** find all properties */ @Test - void countXmlProperties() { + void countProperties() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property testProperty1 = new Property(TEST_PROPERTY_NAME + 1, "testOwner1"); List testProperties = Arrays.asList(testProperty, testProperty1); @@ -204,7 +204,7 @@ void countXmlProperties() { /** find multiple properties */ @Test - void findXmlProperties() { + void findProperties() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Property testProperty1 = new Property(TEST_PROPERTY_NAME + 1, "testOwner1"); List testProperties = Arrays.asList(testProperty, testProperty1); @@ -233,7 +233,7 @@ void findXmlProperties() { /** delete a single property */ @Test - void deleteXmlProperty() { + void deleteProperty() { Property testProperty = new Property(TEST_PROPERTY_NAME, "testOwner"); Optional notFoundProperty = propertyRepository.findById(testProperty.getName()); Property createdProperty = propertyRepository.index(testProperty); diff --git a/src/test/java/org/phoebus/channelfinder/docker/ITUtilProperties.java b/src/test/java/org/phoebus/channelfinder/docker/ITUtilProperties.java index 989451c6..4bfbefbb 100644 --- a/src/test/java/org/phoebus/channelfinder/docker/ITUtilProperties.java +++ b/src/test/java/org/phoebus/channelfinder/docker/ITUtilProperties.java @@ -198,7 +198,7 @@ public static Property[] assertListProperties( assertTrue(actual.length <= expectedLessThanOrEqual); } if (expected != null && expected.length > 0) { - assertEqualsXmlProperties(actual, expected); + assertEqualsProperties(actual, expected); } } catch (Exception e) { fail(); @@ -386,7 +386,7 @@ public static Property[] assertCreateReplaceProperties( actual = ITUtil.MAPPER.readValue(response[1], Property[].class); } if (expected != null) { - assertEqualsXmlProperties(expected, actual); + assertEqualsProperties(expected, actual); } } catch (Exception e) { fail(); @@ -511,7 +511,7 @@ public static Property[] assertAddMultipleProperties( actual = ITUtil.MAPPER.readValue(response[1], Property[].class); } if (expected != null) { - assertEqualsXmlProperties(expected, actual); + assertEqualsProperties(expected, actual); } } catch (Exception e) { fail(); @@ -582,7 +582,7 @@ public static void assertRemoveProperty( * @param actual actual array of Property objects * @param expected expected arbitrary number of Property objects */ - static void assertEqualsXmlProperties(Property[] actual, Property... expected) { + static void assertEqualsProperties(Property[] actual, Property... expected) { if (expected != null) { assertNotNull(actual); assertEquals(expected.length, actual.length);