feat(devices): search device logs by words, not patterns (CU-86agb21qt)

openframe-api-lib/src/main/java/com/openframe/api/dto/device/DeviceLogFilterCriteria.java

@@ -29,11 +29,6 @@ public class DeviceLogFilterCriteria {
      */
     private List<String> excludes;
 
-    /**
-     * Opt-in RE2 pattern, applied case-insensitively.
-     */
-    private String regex;
-
     /**
      * Inclusive lower bound; defaults to {@code to} minus the default lookback.
      */

openframe-api-lib/src/main/java/com/openframe/api/service/device/DeviceLogService.java

@@ -28,8 +28,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.regex.Pattern;
-import java.util.regex.PatternSyntaxException;
 
 import static java.util.stream.Collectors.joining;
 
@@ -55,8 +53,6 @@ public class DeviceLogService {
 
     private static final String AGENT_LOGS_JOB = "agent-logs";
     private static final String CASE_INSENSITIVE = "(?i)";
-    /** Lookaround, atomic groups and backreferences: Java compiles them, Loki's RE2 engine rejects them. */
-    private static final Pattern UNSUPPORTED_REGEX = Pattern.compile("\\(\\?[=!<>]|\\\\[1-9]");
     private static final long NANOS_PER_SECOND = 1_000_000_000L;
 
     private final LokiClient lokiClient;
@@ -124,9 +120,6 @@ static String buildQuery(String tenantDomain, String machineId, DeviceLogFilterC
         // Line filters before the metadata filter: the cheapest stage runs first
         appendTermFilters(query, " |~ ", criteria.getContains());
         appendTermFilters(query, " !~ ", criteria.getExcludes());
-        if (StringUtils.hasText(criteria.getRegex())) {
-            query.append(" |~ ").append(LogQl.quote(CASE_INSENSITIVE + criteria.getRegex()));
-        }
         return query.append(" | machine_id=").append(LogQl.quote(machineId)).toString();
     }
 
@@ -229,7 +222,6 @@ private static int pageSize(Integer requested) {
     private static void validateSearch(DeviceLogFilterCriteria criteria) {
         validateTerms(criteria.getContains(), "contains");
         validateTerms(criteria.getExcludes(), "excludes");
-        validateRegex(criteria.getRegex());
     }
 
     /**
@@ -255,26 +247,6 @@ private static void validateTerms(List<String> terms, String field) {
         }
     }
 
-    /**
-     * Rejected here rather than at Loki, so a mistyped pattern reads as a bad request instead of a failed query.
-     */
-    private static void validateRegex(String regex) {
-        if (!StringUtils.hasText(regex)) {
-            return;
-        }
-        if (regex.length() > MAX_SEARCH_LENGTH) {
-            throw new IllegalArgumentException("regex cannot exceed " + MAX_SEARCH_LENGTH + " characters");
-        }
-        if (UNSUPPORTED_REGEX.matcher(regex).find()) {
-            throw new IllegalArgumentException("regex cannot use lookaround, atomic groups or backreferences");
-        }
-        try {
-            Pattern.compile(regex);
-        } catch (PatternSyntaxException e) {
-            throw new IllegalArgumentException("regex is not valid: " + e.getDescription());
-        }
-    }
-
     private static void validateRange(Instant from, Instant to) {
         if (!from.isBefore(to)) {
             throw new IllegalArgumentException("'from' must be before 'to'");

openframe-api-lib/src/test/java/com/openframe/api/service/device/DeviceLogServiceIT.java

@@ -179,19 +179,14 @@ void matchesSearchTextLiterallySoItCannotWidenTheQuery() {
     }
 
     @Test
-    void narrowsWithSeveralTermsExclusionsAndRegex() {
+    void narrowsWithSeveralTermsAndExclusions() {
         List<DeviceLogEntry> allTerms = walk(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM)
                 .contains(List.of("connection", "failed")).build(), 500);
         List<DeviceLogEntry> withoutADecade = walk(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM)
                 .contains(List.of("heartbeat")).excludes(List.of("line-01")).build(), 500);
-        List<DeviceLogEntry> byRegex = walk(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM)
-                .regex("line-00[0-9] ").build(), 500);
-
         assertThat(allTerms).hasSize(ERROR_LINES);
         assertThat(withoutADecade).hasSize(DEVICE_LINES - ERROR_LINES - 10)
                 .extracting(DeviceLogEntry::getMessage).noneMatch(message -> message.contains("line-01"));
-        assertThat(byRegex).hasSize(10)
-                .extracting(DeviceLogEntry::getMessage).first().isEqualTo("line-009 heartbeat ok");
     }
 
     @Test

openframe-api-lib/src/test/java/com/openframe/api/service/device/DeviceLogServiceTest.java

@@ -248,7 +248,6 @@ void buildsOneLineFilterPerTermBeforeTheMetadataFilter() {
         DeviceLogFilterCriteria filter = DeviceLogFilterCriteria.builder()
                 .contains(List.of("connection", "failed"))
                 .excludes(List.of("heartbeat"))
-                .regex("fd=\\d+")
                 .from(FROM)
                 .to(TO)
                 .build();
@@ -257,27 +256,21 @@ void buildsOneLineFilterPerTermBeforeTheMetadataFilter() {
 
         verify(lokiClient).queryRange(
                 "{job=\"agent-logs\", tenant_domain=\"acme.openframe.ai\"}"
-                        + " |~ \"(?i)connection\" |~ \"(?i)failed\" !~ \"(?i)heartbeat\" |~ \"(?i)fd=\\\\d+\""
+                        + " |~ \"(?i)connection\" |~ \"(?i)failed\" !~ \"(?i)heartbeat\""
                         + " | machine_id=\"machine-1\"",
                 FROM_NANOS, TO_NANOS + 1, 101, LokiDirection.BACKWARD);
     }
 
     @Test
-    void rejectsTooManyTermsAndPatternsLokiCannotRun() {
+    void rejectsMoreThanFiveTerms() {
         assertThatThrownBy(() -> service.queryDeviceLogs(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM).to(TO)
                 .contains(List.of("a", "b", "c", "d", "e", "f")).build(), page(null, null)))
                 .isInstanceOf(IllegalArgumentException.class);
-        assertThatThrownBy(() -> service.queryDeviceLogs(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM).to(TO)
-                .regex("(?=lookahead)").build(), page(null, null)))
-                .isInstanceOf(IllegalArgumentException.class);
-        assertThatThrownBy(() -> service.queryDeviceLogs(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM).to(TO)
-                .regex("unclosed(").build(), page(null, null)))
-                .isInstanceOf(IllegalArgumentException.class);
         verifyNoInteractions(lokiClient);
     }
 
     @Test
-    void ignoresBlankTermsAndRejectsAtomicGroups() {
+    void ignoresBlankTerms() {
         service.queryDeviceLogs(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM).to(TO)
                 .contains(List.of("a", "", "  ", "b", "c", "d", "e")).build(), page(null, null));
 
@@ -287,11 +280,6 @@ void ignoresBlankTermsAndRejectsAtomicGroups() {
                         + " |~ \"(?i)a\" |~ \"(?i)b\" |~ \"(?i)c\" |~ \"(?i)d\" |~ \"(?i)e\""
                         + " | machine_id=\"machine-1\"",
                 FROM_NANOS, TO_NANOS + 1, 101, LokiDirection.BACKWARD);
-
-        // Java compiles an atomic group, Loki's RE2 engine does not
-        assertThatThrownBy(() -> service.queryDeviceLogs(MACHINE_ID, DeviceLogFilterCriteria.builder().from(FROM).to(TO)
-                .regex("(?>atomic)").build(), page(null, null)))
-                .isInstanceOf(IllegalArgumentException.class);
     }
 
     private static DeviceLogFilterCriteria window() {

openframe-api-service-core/src/main/java/com/openframe/api/dto/device/DeviceLogFilterInput.java

@@ -17,14 +17,11 @@ public class DeviceLogFilterInput {
 
     private List<DeviceLogLevel> levels;
 
-    @Size(max = 5, message = "contains cannot hold more than 5 terms")
-    private List<@Size(max = 256, message = "search terms cannot exceed 256 characters") String> contains;
+    @Size(max = 5, message = "cannot hold more than 5 terms")
+    private List<@Size(max = 256, message = "cannot exceed 256 characters") String> contains;
 
-    @Size(max = 5, message = "excludes cannot hold more than 5 terms")
-    private List<@Size(max = 256, message = "search terms cannot exceed 256 characters") String> excludes;
-
-    @Size(max = 256, message = "regex cannot exceed 256 characters")
-    private String regex;
+    @Size(max = 5, message = "cannot hold more than 5 terms")
+    private List<@Size(max = 256, message = "cannot exceed 256 characters") String> excludes;
 
     private Instant from;
 

openframe-api-service-core/src/main/java/com/openframe/api/mapper/GraphQLDeviceLogMapper.java

@@ -25,7 +25,6 @@ public DeviceLogFilterCriteria toFilterCriteria(DeviceLogFilterInput input) {
                 .levels(input.getLevels())
                 .contains(input.getContains())
                 .excludes(input.getExcludes())
-                .regex(input.getRegex())
                 .from(input.getFrom())
                 .to(input.getTo())
                 .build();

openframe-api-service-core/src/main/resources/schema/device-log.graphqls

@@ -20,7 +20,6 @@ input DeviceLogFilterInput {
     levels: [DeviceLogLevel!]  # Any of these levels; all levels when omitted
     contains: [String!]        # All of these must appear in the message; case-insensitive, matched literally. Max 5 terms of 256 characters
     excludes: [String!]        # None of these may appear in the message; same matching and limits
-    regex: String              # Opt-in RE2 pattern, case-insensitive, max 256 characters. Lookaround and backreferences are rejected
     from: Instant              # Inclusive; defaults to 7 days before `to`
     to: Instant                # Inclusive; defaults to now. The range may not exceed 30 days
 }

openframe-api-service-core/src/test/java/com/openframe/api/exception/GraphQLExceptionHandlerTest.java

@@ -1,66 +1,67 @@
 package com.openframe.api.exception;
 
-import com.openframe.core.exception.ErrorCode;
-import com.openframe.core.exception.NotFoundException;
+import com.openframe.api.dto.device.DeviceLogFilterInput;
 import graphql.GraphQLError;
 import graphql.execution.DataFetcherExceptionHandlerParameters;
+import graphql.execution.DataFetcherExceptionHandlerResult;
 import jakarta.validation.ConstraintViolationException;
 import jakarta.validation.Validation;
 import jakarta.validation.Validator;
-import jakarta.validation.constraints.NotBlank;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
 
 class GraphQLExceptionHandlerTest {
 
     private final GraphQLExceptionHandler handler = new GraphQLExceptionHandler();
 
     @Test
-    void constraintViolationIsAValidationErrorWithTheConstraintMessage() {
-        Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
-        ConstraintViolationException exception = new ConstraintViolationException(validator.validate(new Input("")));
+    void reportsBeanValidationFailuresAsBadRequestsWithTheViolationMessage() {
+        DeviceLogFilterInput tooManyTerms = DeviceLogFilterInput.builder()
+                .contains(List.of("a", "b", "c", "d", "e", "f"))
+                .build();
+        ConstraintViolationException exception = violationsOf(tooManyTerms);
 
         GraphQLError error = handle(exception);
 
-        assertEquals("name: name is required", error.getMessage());
-        assertEquals(ErrorCode.VALIDATION_ERROR.getCode(), error.getExtensions().get("code"));
-        assertEquals(400, error.getExtensions().get("httpStatus"));
-    }
-
-    @Test
-    void illegalArgumentKeepsItsMessageAsValidationError() {
-        GraphQLError error = handle(new IllegalArgumentException("Tag not found: t-1"));
-
-        assertEquals("Tag not found: t-1", error.getMessage());
-        assertEquals(ErrorCode.VALIDATION_ERROR.getCode(), error.getExtensions().get("code"));
+        assertThat(error.getExtensions()).containsEntry("code", "VALIDATION_ERROR").containsEntry("httpStatus", 400);
+        // The handler prefixes the offending field, so the message itself should not repeat it
+        assertThat(error.getMessage()).isEqualTo("contains: cannot hold more than 5 terms");
     }
 
     @Test
-    void typedNotFoundKeepsItsCode() {
-        GraphQLError error = handle(new NotFoundException(ErrorCode.ORGANIZATION_NOT_FOUND, "Organization not found"));
+    void reportsRejectedArgumentsAsBadRequests() {
+        // The handler maps IllegalArgumentException and IllegalStateException to VALIDATION_ERROR:
+        // services throw them for input they refuse, such as a time range over 30 days
+        GraphQLError error = handle(new IllegalStateException("range too wide"));
 
-        assertEquals("Organization not found", error.getMessage());
-        assertEquals("ORGANIZATION_NOT_FOUND", error.getExtensions().get("code"));
-        assertEquals(404, error.getExtensions().get("httpStatus"));
+        assertThat(error.getExtensions()).containsEntry("code", "VALIDATION_ERROR");
+        assertThat(error.getMessage()).isEqualTo("range too wide");
     }
 
     @Test
-    void unexpectedRuntimeExceptionDoesNotLeakItsMessage() {
-        GraphQLError error = handle(new RuntimeException("jdbc://secret-host is down"));
+    void keepsReportingUnexpectedFailuresAsInternalErrors() {
+        GraphQLError error = handle(new RuntimeException("boom"));
 
-        assertEquals("An unexpected error occurred. Please try again later.", error.getMessage());
-        assertEquals(ErrorCode.INTERNAL_ERROR.getCode(), error.getExtensions().get("code"));
+        assertThat(error.getExtensions()).containsEntry("code", "INTERNAL_ERROR").containsEntry("httpStatus", 500);
+        assertThat(error.getMessage()).doesNotContain("boom");
     }
 
     private GraphQLError handle(Throwable exception) {
-        DataFetcherExceptionHandlerParameters parameters = mock(DataFetcherExceptionHandlerParameters.class);
-        when(parameters.getException()).thenReturn(exception);
-        return handler.handleException(parameters).join().getErrors().getFirst();
+        DataFetcherExceptionHandlerParameters parameters = DataFetcherExceptionHandlerParameters
+                .newExceptionParameters()
+                .exception(exception)
+                .build();
+        DataFetcherExceptionHandlerResult result = handler.handleException(parameters).join();
+        return result.getErrors().get(0);
     }
 
-    private record Input(@NotBlank(message = "name is required") String name) {
+    private static ConstraintViolationException violationsOf(DeviceLogFilterInput input) {
+        try (var factory = Validation.buildDefaultValidatorFactory()) {
+            Validator validator = factory.getValidator();
+            return new ConstraintViolationException(validator.validate(input));
+        }
     }
 }