Skip to content

chore: remove unused pruneCollectionsFileStrict method #253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
detail-app wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

chore: remove unused pruneCollectionsFileStrict method #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions src/main/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,27 +67,6 @@ public void pruneCollectionFileStrict(
pruneFileStrict(List.of(collectionName), sourceUrl, previousFileRecord);
}

/**
* Strictly prunes a file from every specified collection before deleting its local ingestion state.
*
* <p>Deferring local cleanup until every vector deletion succeeds preserves the marker needed to retry
* a partially completed prior-format marker prune.</p>
*
* @param collectionNames target Qdrant collection names
* @param sourceUrl authoritative URL key for file markers and vectors
* @param previousFileRecord previous file marker record, or {@code null} if unavailable
* @throws IOException when local marker or parsed-chunk cleanup fails
* @throws IllegalArgumentException when no collection names are provided or any name is blank
*/
public void pruneCollectionsFileStrict(
List<String> collectionNames, String sourceUrl, FileIngestionRecord previousFileRecord) throws IOException {
Objects.requireNonNull(collectionNames, "collectionNames");
if (collectionNames.isEmpty()) {
throw new IllegalArgumentException("At least one collection name is required for file pruning");
}
pruneFileStrict(List.copyOf(collectionNames), sourceUrl, previousFileRecord);
}

/**
* Removes obsolete local chunk state after a complete same-collection replacement was stored.
*
Expand Down
56 changes: 0 additions & 56 deletions ...est/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
package com.williamcallahan.javachat.service.ingestion;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand All @@ -25,12 +22,10 @@
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.InOrder;

/** Verifies strict multi-collection vector pruning preserves retryable local ingestion state. */
class IngestedFilePruneServiceTest {
private static final int LEGACY_CHUNK_HASH_PREFIX_LENGTH = 12;
private static final String BOOKS_COLLECTION_NAME = "books-collection";
private static final String DOCS_COLLECTION_NAME = "docs-collection";
private static final String SOURCE_URL = "https://docs.example.com/reference/page.html";
private static final String REPLACEMENT_CHUNK_HASH =
Expand All @@ -43,57 +38,6 @@ class IngestedFilePruneServiceTest {
private static final String MARKERLESS_STALE_CHUNK_HASH =
"fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210";

@Test
void deletesEveryCollectionBeforeLocalIngestionState() throws IOException {
HybridVectorService hybridVectorService = mock(HybridVectorService.class);
LocalStoreService localStoreService = mock(LocalStoreService.class);
FileIngestionMarkerStore fileIngestionMarkerStore = mock(FileIngestionMarkerStore.class);
IngestedFilePruneService pruneService = new IngestedFilePruneService(
hybridVectorService, localStoreService, fileIngestionMarkerStore, mock(ContentHasher.class));
FileIngestionRecord priorIngestionRecord = new FileIngestionRecord(
123L,
456L,
"fingerprint",
"extraction-v1",
BOOKS_COLLECTION_NAME,
List.of("first-hash", "second-hash"));

pruneService.pruneCollectionsFileStrict(
List.of(BOOKS_COLLECTION_NAME, DOCS_COLLECTION_NAME), SOURCE_URL, priorIngestionRecord);

InOrder pruneOrder = inOrder(hybridVectorService, localStoreService, fileIngestionMarkerStore);
pruneOrder.verify(hybridVectorService).deleteByUrl(BOOKS_COLLECTION_NAME, SOURCE_URL);
pruneOrder.verify(hybridVectorService).deleteByUrl(DOCS_COLLECTION_NAME, SOURCE_URL);
pruneOrder.verify(localStoreService).deleteChunkIngestionMarkers(priorIngestionRecord.chunkHashes());
pruneOrder.verify(localStoreService).deleteParsedChunksForUrl(SOURCE_URL);
pruneOrder.verify(fileIngestionMarkerStore).deleteFileIngestionRecord(SOURCE_URL);
}

@Test
void retainsLocalIngestionStateWhenACollectionDeleteFails() throws IOException {
HybridVectorService hybridVectorService = mock(HybridVectorService.class);
LocalStoreService localStoreService = mock(LocalStoreService.class);
FileIngestionMarkerStore fileIngestionMarkerStore = mock(FileIngestionMarkerStore.class);
IngestedFilePruneService pruneService = new IngestedFilePruneService(
hybridVectorService, localStoreService, fileIngestionMarkerStore, mock(ContentHasher.class));
FileIngestionRecord priorIngestionRecord = new FileIngestionRecord(
123L, 456L, "fingerprint", "extraction-v1", BOOKS_COLLECTION_NAME, List.of("first-hash"));
doThrow(new IllegalStateException("Qdrant delete failed"))
.when(hybridVectorService)
.deleteByUrl(DOCS_COLLECTION_NAME, SOURCE_URL);

assertThrows(
IllegalStateException.class,
() -> pruneService.pruneCollectionsFileStrict(
List.of(BOOKS_COLLECTION_NAME, DOCS_COLLECTION_NAME), SOURCE_URL, priorIngestionRecord));

verify(hybridVectorService).deleteByUrl(BOOKS_COLLECTION_NAME, SOURCE_URL);
verify(hybridVectorService).deleteByUrl(DOCS_COLLECTION_NAME, SOURCE_URL);
verify(localStoreService, never()).deleteChunkIngestionMarkers(anyList());
verify(localStoreService, never()).deleteParsedChunksForUrl(anyString());
verify(fileIngestionMarkerStore, never()).deleteFileIngestionRecord(anyString());
}

@Test
void removesLegacyStaleParsedChunkWhenFullHashesShareStoredPrefix(@TempDir Path parsedChunkDirectory)
throws IOException {
Expand Down