From 3d8cc4cd563245f857e003d2de9dba6e10ae9c4b Mon Sep 17 00:00:00 2001 From: shroffk Date: Mon, 4 May 2026 13:25:04 -0400 Subject: [PATCH 1/7] Basic support for batch delete/remove #213 --- .../repository/ChannelRepository.java | 35 +++++++++++++++++-- .../channelfinder/service/ChannelService.java | 32 +++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java index 29689d2a..23a54a39 100644 --- a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java +++ b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java @@ -293,7 +293,7 @@ public Iterable saveAll(Iterable channels) { } BulkResponse result; try { - result = client.bulk(br.refresh(Refresh.True).build()); + result = client.bulk(br.refresh(Refresh.WaitFor).build()); // Log errors, if any if (result.errors()) { logger.log(Level.SEVERE, TextUtil.BULK_HAD_ERRORS); @@ -806,8 +806,39 @@ public Scroll scroll(String scrollId, MultiValueMap searchParame @Override public void deleteAllById(Iterable ids) { - // TODO Auto-generated method stub + List idList = + StreamSupport.stream(ids.spliterator(), false) + .filter(id -> id != null && !id.isBlank()) + .map(String::valueOf) + .distinct() + .toList(); + + for (int i = 0; i < idList.size(); i += chunkSize) { + List chunk = idList.stream().skip(i).limit(chunkSize).toList(); + BulkRequest.Builder br = new BulkRequest.Builder(); + for (String id : chunk) { + br.operations(op -> op.delete(del -> del.index(esService.getES_CHANNEL_INDEX()).id(id))); + } + br.refresh(Refresh.True); + try { + BulkResponse result = client.bulk(br.build()); + if (result.errors()) { + String message = MessageFormat.format(TextUtil.FAILED_TO_DELETE_CHANNEL, chunk); + logger.log(Level.SEVERE, TextUtil.BULK_HAD_ERRORS); + for (BulkResponseItem item : result.items()) { + if (item.error() != null) { + logger.log(Level.SEVERE, () -> item.error().reason()); + } + } + throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, message, null); + } + } catch (IOException e) { + String message = MessageFormat.format(TextUtil.FAILED_TO_DELETE_CHANNEL, chunk); + logger.log(Level.SEVERE, message, e); + throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, message, e); + } + } } @PreDestroy diff --git a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java index c3b874fc..9005a12c 100644 --- a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java +++ b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java @@ -2,6 +2,7 @@ import com.google.common.collect.Lists; import java.text.MessageFormat; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -179,6 +180,37 @@ public void remove(String channelName) { channelRepository.deleteById(channelName); } + public long remove(Iterable channelNames) { + requireRole(ROLES.CF_CHANNEL, "channels batch"); + + List distinctChannelNames = + StreamSupport.stream(channelNames.spliterator(), false) + .filter(name -> name != null && !name.isBlank()) + .collect(Collectors.toCollection(LinkedHashSet::new)) + .stream() + .toList(); + + if (distinctChannelNames.isEmpty()) { + return 0; + } + + Map existingChannels = + channelRepository.findAllById(distinctChannelNames).stream() + .collect(Collectors.toMap(Channel::getName, c -> c)); + + for (String channelName : distinctChannelNames) { + Channel existing = existingChannels.get(channelName); + if (existing == null) { + throw new ChannelNotFoundException(channelName); + } + requireOwner(existing); + audit.log(Level.INFO, () -> MessageFormat.format(TextUtil.DELETE_CHANNEL, channelName)); + } + + channelRepository.deleteAllById(distinctChannelNames); + return distinctChannelNames.size(); + } + private Map findExistingChannels(List channels) { return channelRepository.findAllById(channels.stream().map(Channel::getName).toList()).stream() .collect(Collectors.toMap(Channel::getName, c -> c)); From 2c75e60368db0de0b33c38e4d7d0c57d8f8b4c11 Mon Sep 17 00:00:00 2001 From: shroffk Date: Mon, 4 May 2026 13:26:27 -0400 Subject: [PATCH 2/7] Now add the new batch delete to the REST api #213 --- .../web/v0/controller/ChannelController.java | 5 +++ src/main/resources/application.properties | 2 +- .../service/ChannelServiceTest.java | 36 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/phoebus/channelfinder/web/v0/controller/ChannelController.java b/src/main/java/org/phoebus/channelfinder/web/v0/controller/ChannelController.java index ab094ad1..3fa52a00 100644 --- a/src/main/java/org/phoebus/channelfinder/web/v0/controller/ChannelController.java +++ b/src/main/java/org/phoebus/channelfinder/web/v0/controller/ChannelController.java @@ -65,4 +65,9 @@ public Iterable update(Iterable channels) { public void remove(String channelName) { channelService.remove(channelName); } + + @Override + public long remove(List channelNames) { + return channelService.remove(channelNames); + } } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 1002887a..bde6e83d 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -111,7 +111,7 @@ elasticsearch.create.indices=true ############################## Repository ################################# # Repository chunk size, how many channels to submit to elastic at once -repository.chunk.size = 10000 +repository.chunk.size = 5000 ############################## CORS ############################### # Comma-separated list of allowed origins (supports wildcards). Default: allow all. diff --git a/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java b/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java index 5d49034d..55aa53da 100644 --- a/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java +++ b/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java @@ -1,9 +1,14 @@ package org.phoebus.channelfinder.service; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.List; @@ -16,6 +21,7 @@ import org.phoebus.channelfinder.entity.Channel; import org.phoebus.channelfinder.entity.Property; import org.phoebus.channelfinder.entity.Tag; +import org.phoebus.channelfinder.exceptions.ChannelNotFoundException; import org.phoebus.channelfinder.exceptions.ChannelValidationException; import org.phoebus.channelfinder.exceptions.PropertyNotFoundException; import org.phoebus.channelfinder.exceptions.TagNotFoundException; @@ -138,4 +144,34 @@ void createChannel_validChannelWithTagAndProperty_noException() { assertDoesNotThrow(() -> channelService.create("ch", channel)); } + + @Test + void removeMultipleChannels_validChannels_returnsDeletedCount() { + when(authorizationService.isAuthorizedOwner(any(), any(Channel.class))).thenReturn(true); + when(channelRepository.findAllById(any())) + .thenReturn( + List.of( + new Channel("ch1", "owner"), + new Channel("ch2", "owner"), + new Channel("ch3", "owner"))); + + long deleted = channelService.remove(List.of("ch1", "ch2", "ch3")); + + assertEquals(3L, deleted); + verify(channelRepository, times(1)).deleteAllById(any()); + verify(channelRepository, never()).deleteById(anyString()); + } + + @Test + void removeMultipleChannels_whenOneMissing_throwsAndStopsFurtherDeletes() { + when(authorizationService.isAuthorizedOwner(any(), any(Channel.class))).thenReturn(true); + when(channelRepository.findAllById(any())).thenReturn(List.of(new Channel("ch1", "owner"))); + + assertThrows( + ChannelNotFoundException.class, + () -> channelService.remove(List.of("ch1", "missing", "ch3"))); + + verify(channelRepository, never()).deleteById(anyString()); + verify(channelRepository, never()).deleteAllById(any()); + } } From f789fdfb0f15c9f876e02ec7860752302fcfa475 Mon Sep 17 00:00:00 2001 From: shroffk Date: Thu, 7 May 2026 15:36:01 -0400 Subject: [PATCH 3/7] make the deleteAll best effort --- .../repository/ChannelRepository.java | 83 ++++++++++++++----- .../channelfinder/service/ChannelService.java | 22 +++-- .../channelfinder/web/v0/api/IChannel.java | 20 +++++ 3 files changed, 92 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java index 23a54a39..b9740fe7 100644 --- a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java +++ b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java @@ -428,16 +428,33 @@ public Iterable findAll() { public List findAllById(Iterable channelIds) { try { List ids = - StreamSupport.stream(channelIds.spliterator(), false).collect(Collectors.toList()); + StreamSupport.stream(channelIds.spliterator(), false) + .filter(id -> id != null && !id.isBlank()) + .collect(Collectors.toCollection(LinkedHashSet::new)) + .stream() + .toList(); + + if (ids.isEmpty()) { + return Collections.emptyList(); + } - SearchRequest.Builder searchBuilder = - new SearchRequest.Builder() - .index(esService.getES_CHANNEL_INDEX()) - .query(IdsQuery.of(q -> q.values(ids))._toQuery()) - .size(esService.getES_QUERY_SIZE()) - .sort(SortOptions.of(s -> s.field(FieldSort.of(f -> f.field("name"))))); - SearchResponse response = client.search(searchBuilder.build(), Channel.class); - return response.hits().hits().stream().map(Hit::source).collect(Collectors.toList()); + int lookupBatchSize = Math.max(1, Math.min(chunkSize, esService.getES_QUERY_SIZE())); + List result = new ArrayList<>(); + + for (int i = 0; i < ids.size(); i += lookupBatchSize) { + List chunk = ids.subList(i, Math.min(i + lookupBatchSize, ids.size())); + SearchRequest.Builder searchBuilder = + new SearchRequest.Builder() + .index(esService.getES_CHANNEL_INDEX()) + .query(IdsQuery.of(q -> q.values(chunk))._toQuery()) + .size(chunk.size()) + .sort(SortOptions.of(s -> s.field(FieldSort.of(f -> f.field("name"))))); + SearchResponse response = client.search(searchBuilder.build(), Channel.class); + result.addAll( + response.hits().hits().stream().map(Hit::source).collect(Collectors.toList())); + } + + return result; } catch (ElasticsearchException | IOException e) { logger.log(Level.SEVERE, TextUtil.FAILED_TO_FIND_ALL_CHANNELS, e); throw new ResponseStatusException( @@ -812,9 +829,25 @@ public void deleteAllById(Iterable ids) { .map(String::valueOf) .distinct() .toList(); + deleteAllByIdBestEffort(idList); + } + + public long deleteAllByIdBestEffort(Iterable ids) { + List idList = + StreamSupport.stream(ids.spliterator(), false) + .filter(id -> id != null && !id.isBlank()) + .map(String::valueOf) + .distinct() + .toList(); + + if (idList.isEmpty()) { + return 0; + } + + long deletedCount = 0; for (int i = 0; i < idList.size(); i += chunkSize) { - List chunk = idList.stream().skip(i).limit(chunkSize).toList(); + List chunk = idList.subList(i, Math.min(i + chunkSize, idList.size())); BulkRequest.Builder br = new BulkRequest.Builder(); for (String id : chunk) { br.operations(op -> op.delete(del -> del.index(esService.getES_CHANNEL_INDEX()).id(id))); @@ -823,22 +856,30 @@ public void deleteAllById(Iterable ids) { try { BulkResponse result = client.bulk(br.build()); - if (result.errors()) { - String message = MessageFormat.format(TextUtil.FAILED_TO_DELETE_CHANNEL, chunk); - logger.log(Level.SEVERE, TextUtil.BULK_HAD_ERRORS); - for (BulkResponseItem item : result.items()) { - if (item.error() != null) { - logger.log(Level.SEVERE, () -> item.error().reason()); - } + for (BulkResponseItem item : result.items()) { + if (item.error() != null) { + logger.log( + Level.SEVERE, + () -> + MessageFormat.format( + "Failed to delete channel id {0}: {1}", item.id(), item.error().reason())); + continue; + } + if (Integer.valueOf(200).equals(item.status())) { + deletedCount++; } - throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, message, null); } } catch (IOException e) { - String message = MessageFormat.format(TextUtil.FAILED_TO_DELETE_CHANNEL, chunk); - logger.log(Level.SEVERE, message, e); - throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, message, e); + logger.log( + Level.SEVERE, + MessageFormat.format( + "Bulk delete failed for chunk starting at index {0} with size {1}", + i, chunk.size()), + e); } } + + return deletedCount; } @PreDestroy diff --git a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java index 9005a12c..c9d1e625 100644 --- a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java +++ b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java @@ -194,21 +194,19 @@ public long remove(Iterable channelNames) { return 0; } - Map existingChannels = - channelRepository.findAllById(distinctChannelNames).stream() - .collect(Collectors.toMap(Channel::getName, c -> c)); - - for (String channelName : distinctChannelNames) { - Channel existing = existingChannels.get(channelName); - if (existing == null) { - throw new ChannelNotFoundException(channelName); - } + List existingChannels = channelRepository.findAllById(distinctChannelNames); + + for (Channel existing : existingChannels) { requireOwner(existing); - audit.log(Level.INFO, () -> MessageFormat.format(TextUtil.DELETE_CHANNEL, channelName)); + audit.log(Level.INFO, () -> MessageFormat.format(TextUtil.DELETE_CHANNEL, existing.getName())); + } + + if (existingChannels.isEmpty()) { + return 0; } - channelRepository.deleteAllById(distinctChannelNames); - return distinctChannelNames.size(); + return channelRepository.deleteAllByIdBestEffort( + existingChannels.stream().map(Channel::getName).toList()); } private Map findExistingChannels(List channels) { diff --git a/src/main/java/org/phoebus/channelfinder/web/v0/api/IChannel.java b/src/main/java/org/phoebus/channelfinder/web/v0/api/IChannel.java index 4295d710..0fa494b3 100644 --- a/src/main/java/org/phoebus/channelfinder/web/v0/api/IChannel.java +++ b/src/main/java/org/phoebus/channelfinder/web/v0/api/IChannel.java @@ -269,4 +269,24 @@ long queryCount( }) @DeleteMapping("/{channelName}") void remove(@PathVariable("channelName") String channelName); + + @Operation( + summary = "Delete multiple channels", + description = "Delete multiple channel instances identified by a request-body list of names.", + operationId = "deleteChannels", + tags = {"Channel"}) + @ApiResponses( + value = { + @ApiResponse(responseCode = "200", description = "Number of channels deleted"), + @ApiResponse( + responseCode = "401", + description = "Unauthorized", + content = @Content(schema = @Schema(implementation = ResponseStatusException.class))), + @ApiResponse( + responseCode = "500", + description = "Error while trying to delete channels", + content = @Content(schema = @Schema(implementation = ResponseStatusException.class))) + }) + @DeleteMapping + long remove(@RequestBody List channelNames); } From 223e7e70b3d14287eccf64eb6703b5e7f5a90d54 Mon Sep 17 00:00:00 2001 From: shroffk Date: Thu, 7 May 2026 15:45:56 -0400 Subject: [PATCH 4/7] fix the unite test to match the "best effort delete" behvaiour --- .../channelfinder/service/ChannelService.java | 3 ++- src/main/resources/application.properties | 2 +- .../service/ChannelServiceTest.java | 19 +++++++++++-------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java index c9d1e625..c1128ee4 100644 --- a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java +++ b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java @@ -198,7 +198,8 @@ public long remove(Iterable channelNames) { for (Channel existing : existingChannels) { requireOwner(existing); - audit.log(Level.INFO, () -> MessageFormat.format(TextUtil.DELETE_CHANNEL, existing.getName())); + audit.log( + Level.INFO, () -> MessageFormat.format(TextUtil.DELETE_CHANNEL, existing.getName())); } if (existingChannels.isEmpty()) { diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index bde6e83d..1002887a 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -111,7 +111,7 @@ elasticsearch.create.indices=true ############################## Repository ################################# # Repository chunk size, how many channels to submit to elastic at once -repository.chunk.size = 5000 +repository.chunk.size = 10000 ############################## CORS ############################### # Comma-separated list of allowed origins (supports wildcards). Default: allow all. diff --git a/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java b/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java index 55aa53da..e74c7b28 100644 --- a/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java +++ b/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java @@ -21,7 +21,6 @@ import org.phoebus.channelfinder.entity.Channel; import org.phoebus.channelfinder.entity.Property; import org.phoebus.channelfinder.entity.Tag; -import org.phoebus.channelfinder.exceptions.ChannelNotFoundException; import org.phoebus.channelfinder.exceptions.ChannelValidationException; import org.phoebus.channelfinder.exceptions.PropertyNotFoundException; import org.phoebus.channelfinder.exceptions.TagNotFoundException; @@ -154,24 +153,28 @@ void removeMultipleChannels_validChannels_returnsDeletedCount() { new Channel("ch1", "owner"), new Channel("ch2", "owner"), new Channel("ch3", "owner"))); + when(channelRepository.deleteAllByIdBestEffort(any())).thenReturn(3L); long deleted = channelService.remove(List.of("ch1", "ch2", "ch3")); assertEquals(3L, deleted); - verify(channelRepository, times(1)).deleteAllById(any()); + verify(channelRepository, times(1)).deleteAllByIdBestEffort(any()); + verify(channelRepository, never()).deleteAllById(any()); verify(channelRepository, never()).deleteById(anyString()); } @Test - void removeMultipleChannels_whenOneMissing_throwsAndStopsFurtherDeletes() { + void removeMultipleChannels_whenOneMissing_deletesExistingAndReturnsDeletedCount() { when(authorizationService.isAuthorizedOwner(any(), any(Channel.class))).thenReturn(true); - when(channelRepository.findAllById(any())).thenReturn(List.of(new Channel("ch1", "owner"))); + when(channelRepository.findAllById(any())) + .thenReturn(List.of(new Channel("ch1", "owner"), new Channel("ch3", "owner"))); + when(channelRepository.deleteAllByIdBestEffort(eq(List.of("ch1", "ch3")))).thenReturn(2L); - assertThrows( - ChannelNotFoundException.class, - () -> channelService.remove(List.of("ch1", "missing", "ch3"))); + long deleted = channelService.remove(List.of("ch1", "missing", "ch3")); - verify(channelRepository, never()).deleteById(anyString()); + assertEquals(2L, deleted); + verify(channelRepository, times(1)).deleteAllByIdBestEffort(eq(List.of("ch1", "ch3"))); verify(channelRepository, never()).deleteAllById(any()); + verify(channelRepository, never()).deleteById(anyString()); } } From b89365dc9b869c03f3a9b591e9bfaa64ac78bd9b Mon Sep 17 00:00:00 2001 From: shroffk Date: Thu, 7 May 2026 16:30:50 -0400 Subject: [PATCH 5/7] Update ChannelFinderChannelsIT.java --- .../phoebus/channelfinder/docker/ChannelFinderChannelsIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/phoebus/channelfinder/docker/ChannelFinderChannelsIT.java b/src/test/java/org/phoebus/channelfinder/docker/ChannelFinderChannelsIT.java index 829ae555..e84398da 100644 --- a/src/test/java/org/phoebus/channelfinder/docker/ChannelFinderChannelsIT.java +++ b/src/test/java/org/phoebus/channelfinder/docker/ChannelFinderChannelsIT.java @@ -585,9 +585,9 @@ void handleChannelsCreateUpdateCheck() { + "]"; ITUtilChannels.assertCreateReplaceMultipleChannels( - AuthorizationChoice.ADMIN, "", json_multiple, HttpURLConnection.HTTP_INTERNAL_ERROR); + AuthorizationChoice.ADMIN, "", json_multiple, HttpURLConnection.HTTP_BAD_REQUEST); - ITUtilChannels.assertUpdateChannels("", json_multiple, HttpURLConnection.HTTP_INTERNAL_ERROR); + ITUtilChannels.assertUpdateChannels("", json_multiple, HttpURLConnection.HTTP_BAD_REQUEST); json_multiple = "[" From 412ffecb33a26d3a33190bf6093bc4731b8691d0 Mon Sep 17 00:00:00 2001 From: shroffk Date: Mon, 11 May 2026 11:05:06 -0400 Subject: [PATCH 6/7] reduce code duplications, utility method channelnames in requests --- .../repository/ChannelRepository.java | 39 ++++++++++--------- .../channelfinder/service/ChannelService.java | 1 - 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java index b9740fe7..d320746a 100644 --- a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java +++ b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java @@ -427,12 +427,7 @@ public Iterable findAll() { @Override public List findAllById(Iterable channelIds) { try { - List ids = - StreamSupport.stream(channelIds.spliterator(), false) - .filter(id -> id != null && !id.isBlank()) - .collect(Collectors.toCollection(LinkedHashSet::new)) - .stream() - .toList(); + List ids = normalizeIds(channelIds); if (ids.isEmpty()) { return Collections.emptyList(); @@ -822,23 +817,13 @@ public Scroll scroll(String scrollId, MultiValueMap searchParame } @Override + @SuppressWarnings("unchecked") public void deleteAllById(Iterable ids) { - List idList = - StreamSupport.stream(ids.spliterator(), false) - .filter(id -> id != null && !id.isBlank()) - .map(String::valueOf) - .distinct() - .toList(); - deleteAllByIdBestEffort(idList); + deleteAllByIdBestEffort((Iterable) ids); } public long deleteAllByIdBestEffort(Iterable ids) { - List idList = - StreamSupport.stream(ids.spliterator(), false) - .filter(id -> id != null && !id.isBlank()) - .map(String::valueOf) - .distinct() - .toList(); + List idList = normalizeIds(ids); if (idList.isEmpty()) { return 0; @@ -882,6 +867,22 @@ public long deleteAllByIdBestEffort(Iterable ids) { return deletedCount; } + /** + * Normalizes channel IDs by dropping null/blank values and removing duplicates while preserving + * encounter order. + * + * @param ids raw channel IDs from request/repository callers + * @return distinct, non-blank channel IDs in encounter order + */ + private static List normalizeIds(Iterable ids) { + // TODO: Consider rejecting blank/whitespace-only IDs with 400 at the API boundary. + return StreamSupport.stream(ids.spliterator(), false) + .filter(id -> id != null && !id.isBlank()) + .collect(Collectors.toCollection(LinkedHashSet::new)) + .stream() + .toList(); + } + @PreDestroy public void shutdownExecutor() { executor.shutdown(); diff --git a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java index c1128ee4..565a6fd5 100644 --- a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java +++ b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java @@ -185,7 +185,6 @@ public long remove(Iterable channelNames) { List distinctChannelNames = StreamSupport.stream(channelNames.spliterator(), false) - .filter(name -> name != null && !name.isBlank()) .collect(Collectors.toCollection(LinkedHashSet::new)) .stream() .toList(); From 3f77a739e9df14ad6d57b7e05efeaed7133cef0a Mon Sep 17 00:00:00 2001 From: shroffk Date: Mon, 11 May 2026 11:42:15 -0400 Subject: [PATCH 7/7] remove the duplicate unnecessary channel normalization --- .../channelfinder/service/ChannelService.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java index 565a6fd5..da9aeefc 100644 --- a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java +++ b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java @@ -182,18 +182,7 @@ public void remove(String channelName) { public long remove(Iterable channelNames) { requireRole(ROLES.CF_CHANNEL, "channels batch"); - - List distinctChannelNames = - StreamSupport.stream(channelNames.spliterator(), false) - .collect(Collectors.toCollection(LinkedHashSet::new)) - .stream() - .toList(); - - if (distinctChannelNames.isEmpty()) { - return 0; - } - - List existingChannels = channelRepository.findAllById(distinctChannelNames); + List existingChannels = channelRepository.findAllById(channelNames); for (Channel existing : existingChannels) { requireOwner(existing);