fix(personal): harden retrieval edge cases
This commit is contained in:
+15
-3
@@ -15,6 +15,7 @@ import org.springframework.stereotype.Service;
|
|||||||
|
|
||||||
import java.time.LocalDate;
|
import java.time.LocalDate;
|
||||||
import java.time.LocalDateTime;
|
import java.time.LocalDateTime;
|
||||||
|
import java.time.DateTimeException;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Comparator;
|
import java.util.Comparator;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
@@ -205,9 +206,7 @@ public class PersonalRetrievalService {
|
|||||||
|| request.queryText().trim().length() > 1000) {
|
|| request.queryText().trim().length() > 1000) {
|
||||||
throw new IllegalArgumentException("PERSONAL_SEARCH_QUERY_INVALID");
|
throw new IllegalArgumentException("PERSONAL_SEARCH_QUERY_INVALID");
|
||||||
}
|
}
|
||||||
if (request.dateFrom() != null && request.dateTo() != null && request.dateFrom().isAfter(request.dateTo())) {
|
validateDates(request.dateFrom(), request.dateTo());
|
||||||
throw new IllegalArgumentException("PERSONAL_SEARCH_DATE_RANGE_INVALID");
|
|
||||||
}
|
|
||||||
List<Long> itemIds = request.itemIds() == null ? List.of() : request.itemIds().stream().distinct().toList();
|
List<Long> itemIds = request.itemIds() == null ? List.of() : request.itemIds().stream().distinct().toList();
|
||||||
if (itemIds.size() > 100 || itemIds.stream().anyMatch(id -> id == null || id <= 0)) {
|
if (itemIds.size() > 100 || itemIds.stream().anyMatch(id -> id == null || id <= 0)) {
|
||||||
throw new IllegalArgumentException("PERSONAL_SEARCH_ITEM_SCOPE_INVALID");
|
throw new IllegalArgumentException("PERSONAL_SEARCH_ITEM_SCOPE_INVALID");
|
||||||
@@ -231,6 +230,19 @@ public class PersonalRetrievalService {
|
|||||||
args.addAll(itemIds);
|
args.addAll(itemIds);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static void validateDates(LocalDate dateFrom, LocalDate dateTo) {
|
||||||
|
if (dateFrom != null && dateTo != null && dateFrom.isAfter(dateTo)) {
|
||||||
|
throw new IllegalArgumentException("PERSONAL_SEARCH_DATE_INVALID");
|
||||||
|
}
|
||||||
|
if (dateTo != null) {
|
||||||
|
try {
|
||||||
|
dateTo.plusDays(1);
|
||||||
|
} catch (DateTimeException ex) {
|
||||||
|
throw new IllegalArgumentException("PERSONAL_SEARCH_DATE_INVALID");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static String excerpt(String content) {
|
private static String excerpt(String content) {
|
||||||
if (content == null) {
|
if (content == null) {
|
||||||
return "";
|
return "";
|
||||||
|
|||||||
+20
-4
@@ -15,6 +15,7 @@ import java.net.http.HttpClient;
|
|||||||
import java.net.http.HttpRequest;
|
import java.net.http.HttpRequest;
|
||||||
import java.net.http.HttpResponse;
|
import java.net.http.HttpResponse;
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
|
import java.time.DateTimeException;
|
||||||
import java.time.LocalDate;
|
import java.time.LocalDate;
|
||||||
import java.time.LocalDateTime;
|
import java.time.LocalDateTime;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
@@ -135,9 +136,7 @@ public class PersonalVectorStore {
|
|||||||
requireOwner(owner);
|
requireOwner(owner);
|
||||||
ArrayNode vector = parseVector(vectorJson);
|
ArrayNode vector = parseVector(vectorJson);
|
||||||
validateDimension(vector.size());
|
validateDimension(vector.size());
|
||||||
if (dateFrom != null && dateTo != null && dateFrom.isAfter(dateTo)) {
|
validateDates(dateFrom, dateTo);
|
||||||
throw new IllegalArgumentException("PERSONAL_VECTOR_DATE_RANGE_INVALID");
|
|
||||||
}
|
|
||||||
List<Long> scopedItems = itemIds == null ? List.of() : itemIds.stream().distinct().toList();
|
List<Long> scopedItems = itemIds == null ? List.of() : itemIds.stream().distinct().toList();
|
||||||
if (scopedItems.size() > 100 || scopedItems.stream().anyMatch(id -> id == null || id <= 0)) {
|
if (scopedItems.size() > 100 || scopedItems.stream().anyMatch(id -> id == null || id <= 0)) {
|
||||||
throw new IllegalArgumentException("PERSONAL_VECTOR_ITEM_SCOPE_INVALID");
|
throw new IllegalArgumentException("PERSONAL_VECTOR_ITEM_SCOPE_INVALID");
|
||||||
@@ -239,7 +238,7 @@ public class PersonalVectorStore {
|
|||||||
}
|
}
|
||||||
return new CollectionMetadata(size.asInt(), result.path("payload_schema"));
|
return new CollectionMetadata(size.asInt(), result.path("payload_schema"));
|
||||||
} catch (Exception ex) {
|
} catch (Exception ex) {
|
||||||
log.warn("event=personal_vector_transport_failed exception={}", ex.getClass().getSimpleName());
|
log.warn("event=personal_vector_collection_metadata_invalid exception={}", ex.getClass().getSimpleName());
|
||||||
throw unavailable();
|
throw unavailable();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -308,6 +307,19 @@ public class PersonalVectorStore {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static void validateDates(LocalDate dateFrom, LocalDate dateTo) {
|
||||||
|
if (dateFrom != null && dateTo != null && dateFrom.isAfter(dateTo)) {
|
||||||
|
throw new IllegalArgumentException("PERSONAL_SEARCH_DATE_INVALID");
|
||||||
|
}
|
||||||
|
if (dateTo != null) {
|
||||||
|
try {
|
||||||
|
dateTo.plusDays(1);
|
||||||
|
} catch (DateTimeException ex) {
|
||||||
|
throw new IllegalArgumentException("PERSONAL_SEARCH_DATE_INVALID");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private void requireOwner(PersonalOwner owner) {
|
private void requireOwner(PersonalOwner owner) {
|
||||||
if (owner == null || owner.tenantId() == null || owner.tenantId().isBlank() || owner.userId() <= 0) {
|
if (owner == null || owner.tenantId() == null || owner.tenantId().isBlank() || owner.userId() <= 0) {
|
||||||
log.warn("event=personal_vector_owner_invalid");
|
log.warn("event=personal_vector_owner_invalid");
|
||||||
@@ -330,6 +342,10 @@ public class PersonalVectorStore {
|
|||||||
return transport.send(new TransportRequest(method, path,
|
return transport.send(new TransportRequest(method, path,
|
||||||
body == null ? "" : objectMapper.writeValueAsString(body), Map.copyOf(headers)));
|
body == null ? "" : objectMapper.writeValueAsString(body), Map.copyOf(headers)));
|
||||||
} catch (Exception ex) {
|
} catch (Exception ex) {
|
||||||
|
if (ex instanceof InterruptedException) {
|
||||||
|
Thread.currentThread().interrupt();
|
||||||
|
}
|
||||||
|
log.warn("event=personal_vector_transport_failed exception={}", ex.getClass().getSimpleName());
|
||||||
throw unavailable();
|
throw unavailable();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+8
-1
@@ -59,8 +59,10 @@ class PersonalRetrievalServiceTest {
|
|||||||
PersonalOwner owner = new PersonalOwner("t", 1, null);
|
PersonalOwner owner = new PersonalOwner("t", 1, null);
|
||||||
assertTrue(service.search(owner, new PersonalSearchRequest("q", List.of(SearchScope.ENTERPRISE), null, null, null, 10)).isEmpty());
|
assertTrue(service.search(owner, new PersonalSearchRequest("q", List.of(SearchScope.ENTERPRISE), null, null, null, 10)).isEmpty());
|
||||||
assertThrows(IllegalArgumentException.class, () -> service.search(owner, new PersonalSearchRequest(" ", null, null, null, null, 10)));
|
assertThrows(IllegalArgumentException.class, () -> service.search(owner, new PersonalSearchRequest(" ", null, null, null, null, 10)));
|
||||||
assertThrows(IllegalArgumentException.class, () -> service.search(owner, new PersonalSearchRequest("q", null,
|
assertDateInvalid(() -> service.search(owner, new PersonalSearchRequest("q", null,
|
||||||
LocalDate.of(2026, 2, 1), LocalDate.of(2026, 1, 1), null, 10)));
|
LocalDate.of(2026, 2, 1), LocalDate.of(2026, 1, 1), null, 10)));
|
||||||
|
assertDateInvalid(() -> service.search(owner, new PersonalSearchRequest("q", null,
|
||||||
|
null, LocalDate.MAX, null, 10)));
|
||||||
verifyNoInteractions(jdbc);
|
verifyNoInteractions(jdbc);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -166,4 +168,9 @@ class PersonalRetrievalServiceTest {
|
|||||||
private SearchHitResponse hit(String id, String title) {
|
private SearchHitResponse hit(String id, String title) {
|
||||||
return new SearchHitResponse("PERSONAL", id, title, title + " excerpt", LocalDateTime.of(2026, 1, 1, 0, 0), 1);
|
return new SearchHitResponse("PERSONAL", id, title, title + " excerpt", LocalDateTime.of(2026, 1, 1, 0, 0), 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void assertDateInvalid(org.junit.jupiter.api.function.Executable executable) {
|
||||||
|
IllegalArgumentException error = assertThrows(IllegalArgumentException.class, executable);
|
||||||
|
assertEquals("PERSONAL_SEARCH_DATE_INVALID", error.getMessage());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+30
@@ -144,6 +144,36 @@ class PersonalVectorStoreTest {
|
|||||||
assertEquals(2, posts.get());
|
assertEquals(2, posts.get());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void rejectsOverflowingAndReversedDatesBeforeHttp() {
|
||||||
|
List<PersonalVectorStore.TransportRequest> seen = new ArrayList<>();
|
||||||
|
PersonalVectorStore store = fixture(seen, request -> ok("{}"));
|
||||||
|
PersonalOwner owner = new PersonalOwner("t", 1, null);
|
||||||
|
|
||||||
|
for (List<LocalDate> dates : List.of(
|
||||||
|
java.util.Arrays.asList(null, LocalDate.MAX),
|
||||||
|
List.of(LocalDate.of(2026, 2, 1), LocalDate.of(2026, 1, 1)))) {
|
||||||
|
IllegalArgumentException error = assertThrows(IllegalArgumentException.class,
|
||||||
|
() -> store.query(owner, "[1,2]", 5, dates.get(0), dates.get(1), List.of()));
|
||||||
|
assertEquals("PERSONAL_SEARCH_DATE_INVALID", error.getMessage());
|
||||||
|
}
|
||||||
|
assertTrue(seen.isEmpty());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void preservesThreadInterruptWhenTransportIsInterrupted() {
|
||||||
|
PersonalVectorStore store = PersonalVectorStore.forTest(properties(), mapper, request -> {
|
||||||
|
throw new InterruptedException("stop");
|
||||||
|
});
|
||||||
|
try {
|
||||||
|
assertTrue(store.query(new PersonalOwner("t", 1, null), "[1,2]", 5).isEmpty());
|
||||||
|
assertTrue(Thread.currentThread().isInterrupted());
|
||||||
|
} finally {
|
||||||
|
Thread.interrupted();
|
||||||
|
}
|
||||||
|
assertFalse(Thread.currentThread().isInterrupted());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void preservesConfiguredQdrantBasePathPrefix() {
|
void preservesConfiguredQdrantBasePathPrefix() {
|
||||||
PersonalKnowledgeProperties properties = properties();
|
PersonalKnowledgeProperties properties = properties();
|
||||||
|
|||||||
Reference in New Issue
Block a user