improvements to kuery validation

elastic · Oct 29, 2022 · 51fcde7 · 51fcde7
1 parent b446737
commit 51fcde7
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 35 deletions.
diff --git a/x-pack/plugins/apm/common/service_groups.test.ts b/x-pack/plugins/apm/common/service_groups.test.ts
@@ -39,16 +39,16 @@ describe('service_groups common utils', () => {
       const result = validateServiceGroupKuery(
         `service.name: testbeans* or agent.name: "nodejs"`
       );
-      expect(result).toHaveProperty('isValid', true);
-      expect(result).toHaveProperty('isParsingError', false);
-      expect(result).toHaveProperty('message', '');
+      expect(result).toHaveProperty('isValidFields', true);
+      expect(result).toHaveProperty('isValidSyntax', true);
+      expect(result).not.toHaveProperty('message');
     });
     it('should return validation error when unsupported fields are used', () => {
       const result = validateServiceGroupKuery(
         `service.name: testbeans* or agent.name: "nodejs" or transaction.type: request`
       );
-      expect(result).toHaveProperty('isValid', false);
-      expect(result).toHaveProperty('isParsingError', false);
+      expect(result).toHaveProperty('isValidFields', false);
+      expect(result).toHaveProperty('isValidSyntax', true);
       expect(result).toHaveProperty(
         'message',
         'Query filter for service group does not support fields [transaction.type]'
@@ -58,8 +58,8 @@ describe('service_groups common utils', () => {
       const result = validateServiceGroupKuery(
         `service.name: testbeans* or agent.name: "nod`
       );
-      expect(result).toHaveProperty('isValid', false);
-      expect(result).toHaveProperty('isParsingError', true);
+      expect(result).toHaveProperty('isValidFields', false);
+      expect(result).toHaveProperty('isValidSyntax', false);
       expect(result).toHaveProperty('message');
       expect(result).not.toBe('');
     });

diff --git a/x-pack/plugins/apm/common/service_groups.ts b/x-pack/plugins/apm/common/service_groups.ts
@@ -48,31 +48,35 @@ export function isSupportedField(fieldName: string) {
   );
 }
 
-export function validateServiceGroupKuery(kuery: string) {
+export function validateServiceGroupKuery(kuery: string): {
+  isValidFields: boolean;
+  isValidSyntax: boolean;
+  message?: string;
+} {
   try {
     const kueryFields = getKueryFields([fromKueryExpression(kuery)]);
     const unsupportedKueryFields = kueryFields.filter(
       (fieldName) => !isSupportedField(fieldName)
     );
-    if (unsupportedKueryFields.length > 0) {
-      return {
-        isValid: false,
-        isParsingError: false,
-        message: i18n.translate(
-          'xpack.apm.serviceGroups.invalidFields.message',
-          {
-            defaultMessage:
-              'Query filter for service group does not support fields [{unsupportedFieldNames}]',
-            values: {
-              unsupportedFieldNames: unsupportedKueryFields.join(', '),
-            },
-          }
-        ),
-      };
-    } else {
-      return { isValid: true, isParsingError: false, message: '' };
+    if (unsupportedKueryFields.length === 0) {
+      return { isValidFields: true, isValidSyntax: true };
     }
+    return {
+      isValidFields: false,
+      isValidSyntax: true,
+      message: i18n.translate('xpack.apm.serviceGroups.invalidFields.message', {
+        defaultMessage:
+          'Query filter for service group does not support fields [{unsupportedFieldNames}]',
+        values: {
+          unsupportedFieldNames: unsupportedKueryFields.join(', '),
+        },
+      }),
+    };
   } catch (error) {
-    return { isValid: false, isParsingError: true, message: error.message };
+    return {
+      isValidFields: false,
+      isValidSyntax: false,
+      message: error.message,
+    };
   }
 }
diff --git a/...k/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx b/...k/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx
@@ -82,13 +82,8 @@ export function SelectServices({
     if (!stagedKuery) {
       return;
     }
-    const { isValid, isParsingError, message } =
-      validateServiceGroupKuery(stagedKuery);
-    if (isValid || isParsingError) {
-      setKueryValidationMessage(undefined);
-    } else {
-      setKueryValidationMessage(message);
-    }
+    const { message } = validateServiceGroupKuery(stagedKuery);
+    setKueryValidationMessage(message);
   }, [stagedKuery]);
 
   const { start, end } = useMemo(

diff --git a/x-pack/plugins/apm/server/routes/service_groups/route.ts b/x-pack/plugins/apm/server/routes/service_groups/route.ts
@@ -124,8 +124,10 @@ const serviceGroupSaveRoute = createApmServerRoute({
     const {
       savedObjects: { client: savedObjectsClient },
     } = await context.core;
-    const { isValid, message } = validateServiceGroupKuery(params.body.kuery);
-    if (!isValid) {
+    const { isValidFields, isValidSyntax, message } = validateServiceGroupKuery(
+      params.body.kuery
+    );
+    if (!(isValidFields && isValidSyntax)) {
       throw Boom.badRequest(message);
     }