diff --git a/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java b/src/main/java/org/phoebus/channelfinder/repository/ChannelRepository.java index 29689d2a..d320746a 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); @@ -427,17 +427,29 @@ public Iterable findAll() { @Override public List findAllById(Iterable channelIds) { try { - List ids = - StreamSupport.stream(channelIds.spliterator(), false).collect(Collectors.toList()); + List ids = normalizeIds(channelIds); - 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()); + if (ids.isEmpty()) { + return Collections.emptyList(); + } + + 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( @@ -805,9 +817,70 @@ public Scroll scroll(String scrollId, MultiValueMap searchParame } @Override + @SuppressWarnings("unchecked") public void deleteAllById(Iterable ids) { - // TODO Auto-generated method stub + deleteAllByIdBestEffort((Iterable) ids); + } + + public long deleteAllByIdBestEffort(Iterable ids) { + List idList = normalizeIds(ids); + if (idList.isEmpty()) { + return 0; + } + + long deletedCount = 0; + + for (int i = 0; i < idList.size(); i += chunkSize) { + 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))); + } + br.refresh(Refresh.True); + + try { + BulkResponse result = client.bulk(br.build()); + 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++; + } + } + } catch (IOException 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; + } + + /** + * 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 diff --git a/src/main/java/org/phoebus/channelfinder/service/ChannelService.java b/src/main/java/org/phoebus/channelfinder/service/ChannelService.java index c3b874fc..da9aeefc 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,24 @@ public void remove(String channelName) { channelRepository.deleteById(channelName); } + public long remove(Iterable channelNames) { + requireRole(ROLES.CF_CHANNEL, "channels batch"); + List existingChannels = channelRepository.findAllById(channelNames); + + for (Channel existing : existingChannels) { + requireOwner(existing); + audit.log( + Level.INFO, () -> MessageFormat.format(TextUtil.DELETE_CHANNEL, existing.getName())); + } + + if (existingChannels.isEmpty()) { + return 0; + } + + return channelRepository.deleteAllByIdBestEffort( + existingChannels.stream().map(Channel::getName).toList()); + } + private Map findExistingChannels(List channels) { return channelRepository.findAllById(channels.stream().map(Channel::getName).toList()).stream() .collect(Collectors.toMap(Channel::getName, c -> c)); 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); } 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/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 = "[" diff --git a/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java b/src/test/java/org/phoebus/channelfinder/service/ChannelServiceTest.java index 5d49034d..e74c7b28 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; @@ -138,4 +143,38 @@ 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"))); + when(channelRepository.deleteAllByIdBestEffort(any())).thenReturn(3L); + + long deleted = channelService.remove(List.of("ch1", "ch2", "ch3")); + + assertEquals(3L, deleted); + verify(channelRepository, times(1)).deleteAllByIdBestEffort(any()); + verify(channelRepository, never()).deleteAllById(any()); + verify(channelRepository, never()).deleteById(anyString()); + } + + @Test + void removeMultipleChannels_whenOneMissing_deletesExistingAndReturnsDeletedCount() { + when(authorizationService.isAuthorizedOwner(any(), any(Channel.class))).thenReturn(true); + 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); + + long deleted = channelService.remove(List.of("ch1", "missing", "ch3")); + + assertEquals(2L, deleted); + verify(channelRepository, times(1)).deleteAllByIdBestEffort(eq(List.of("ch1", "ch3"))); + verify(channelRepository, never()).deleteAllById(any()); + verify(channelRepository, never()).deleteById(anyString()); + } }