cleanup

src/plugins/data_view_management/public/components/edit_index_pattern/edit_index_pattern.tsx

@@ -93,7 +93,7 @@ export const EditIndexPattern = withRouter(
     const [showEditDialog, setShowEditDialog] = useState<boolean>(false);
     const [relationships, setRelationships] = useState<SavedObjectRelationWithTitle[]>([]);
     const [allowedTypes, setAllowedTypes] = useState<SavedObjectManagementTypeInfo[]>([]);
-    const [refreshCount, setRefreshCount] = useState<number>(0);
+    const [refreshCount, setRefreshCount] = useState<number>(0); // used for forcing rerender of field list
     const [isRefreshing, setIsRefreshing] = React.useState(false);
 
     const conflictFieldsUrl = useMemo(() => {
@@ -250,7 +250,7 @@ export const EditIndexPattern = withRouter(
           refreshIndexPatternClick={async () => {
             setIsRefreshing(true);
             await dataViews.refreshFields(indexPattern, false, true);
-            setRefreshCount(refreshCount + 1);
+            setRefreshCount(refreshCount + 1); // rerender field list
             setIsRefreshing(false);
           }}
           defaultIndex={defaultIndex}

src/plugins/data_views/common/utils.test.ts

@@ -8,6 +8,7 @@
 
 import { isFilterable } from '.';
 import type { DataViewField } from './fields';
+import { unwrapEtag } from './utils';
 
 const mockField = {
   name: 'foo',
@@ -16,38 +17,55 @@ const mockField = {
   type: 'string',
 } as DataViewField;
 
-describe('isFilterable', () => {
-  describe('types', () => {
-    it('should return true for filterable types', () => {
-      ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
-        expect(isFilterable({ ...mockField, type } as DataViewField)).toBe(true);
+describe('common utils', () => {
+  describe('isFilterable', () => {
+    describe('types', () => {
+      it('should return true for filterable types', () => {
+        ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
+          expect(isFilterable({ ...mockField, type } as DataViewField)).toBe(true);
+        });
       });
-    });
 
-    it('should return false for filterable types if the field is not searchable', () => {
-      ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
-        expect(isFilterable({ ...mockField, type, searchable: false } as DataViewField)).toBe(
-          false
-        );
+      it('should return false for filterable types if the field is not searchable', () => {
+        ['string', 'number', 'date', 'ip', 'boolean'].forEach((type) => {
+          expect(isFilterable({ ...mockField, type, searchable: false } as DataViewField)).toBe(
+            false
+          );
+        });
       });
-    });
 
-    it('should return false for un-filterable types', () => {
-      ['geo_point', 'geo_shape', 'attachment', 'murmur3', '_source', 'unknown', 'conflict'].forEach(
-        (type) => {
+      it('should return false for un-filterable types', () => {
+        [
+          'geo_point',
+          'geo_shape',
+          'attachment',
+          'murmur3',
+          '_source',
+          'unknown',
+          'conflict',
+        ].forEach((type) => {
           expect(isFilterable({ ...mockField, type } as DataViewField)).toBe(false);
-        }
-      );
+        });
+      });
     });
-  });
 
-  it('should return true for scripted fields', () => {
-    expect(isFilterable({ ...mockField, scripted: true, searchable: false } as DataViewField)).toBe(
-      true
-    );
+    it('should return true for scripted fields', () => {
+      expect(
+        isFilterable({ ...mockField, scripted: true, searchable: false } as DataViewField)
+      ).toBe(true);
+    });
+
+    it('should return true for the _id field', () => {
+      expect(isFilterable({ ...mockField, name: '_id' } as DataViewField)).toBe(true);
+    });
   });
 
-  it('should return true for the _id field', () => {
-    expect(isFilterable({ ...mockField, name: '_id' } as DataViewField)).toBe(true);
+  describe('unwrapEtag', () => {
+    it('should return the etag without quotes', () => {
+      expect(unwrapEtag('"foo"')).toBe('foo');
+    });
+    it('should return the etag without quotes and without gzip', () => {
+      expect(unwrapEtag('"foo-gzip"')).toBe('foo');
+    });
   });
 });

src/plugins/data_views/common/utils.ts

@@ -27,3 +27,11 @@ export async function findByName(client: PersistenceAPI, name: string) {
     return savedObjects ? savedObjects[0] : undefined;
   }
 }
+
+export function unwrapEtag(ifNoneMatch: string) {
+  let requestHash = (ifNoneMatch as string).slice(1, -1);
+  if (requestHash.indexOf('-') > -1) {
+    requestHash = requestHash.split('-')[0];
+  }
+  return requestHash;
+}

src/plugins/data_views/public/data_views/data_views_api_client.ts

@@ -37,6 +37,7 @@ export class DataViewsApiClient implements IDataViewsApiClient {
   ): Promise<HttpResponse<T> | undefined> {
     const asResponse = true;
     // circle back to this, will likely need changes to any code that loads fields
+    // setting to true skips automatic json parsing
     // const rawResponse = true;
     const rawResponse = false;
     const cacheOptions = forceRefresh ? { cache: 'no-cache' as RequestCache } : {};

src/plugins/data_views/server/rest_api_routes/internal/fields.ts

@@ -11,6 +11,7 @@ import { estypes } from '@elastic/elasticsearch';
 import { schema } from '@kbn/config-schema';
 import { IRouter, RequestHandler, StartServicesAccessor } from '@kbn/core/server';
 // import { FullValidationConfig } from '@kbn/core-http-server';
+import { unwrapEtag } from '../../../common/utils';
 import { IndexPatternsFetcher } from '../../fetcher';
 import type {
   DataViewsServerPluginStart,
@@ -144,7 +145,6 @@ const handler: (isRollupsEnabled: () => boolean) => RequestHandler<{}, IQuery, I
     try {
       const { fields, indices } = await indexPatterns.getFieldsForWildcard({
         pattern,
-        // todo should these be added elsewhere?
         metaFields: parsedMetaFields,
         type,
         rollupIndex,
@@ -162,15 +162,9 @@ const handler: (isRollupsEnabled: () => boolean) => RequestHandler<{}, IQuery, I
 
       const etag = calculateHash(Buffer.from(JSON.stringify(body)));
 
-      // todo are these needed?
-      const headers: Record<string, string> = {
-        'content-type': 'application/json',
-        // 'If-None-Match': '123456',
-        etag,
-        // Expires
-      };
+      const headers: Record<string, string> = { 'content-type': 'application/json', etag };
 
-      // todo consider running in parallel with the request
+      // todo examine how long this takes
       const cacheMaxAge = await uiSettings.get<number>('data_views:cache_max_age');
 
       if (cacheMaxAge) {
@@ -180,14 +174,11 @@ const handler: (isRollupsEnabled: () => boolean) => RequestHandler<{}, IQuery, I
         ] = `private, max-age=${cacheMaxAge}, stale-while-revalidate=${stale}`;
       }
 
-      // move to util
       const ifNoneMatch = request.headers['if-none-match'];
-      if (ifNoneMatch) {
-        let requestHash = (ifNoneMatch as string).slice(1, -1);
-        if (requestHash.indexOf('-') > -1) {
-          requestHash = requestHash.split('-')[0];
-        }
+      const ifNoneMatchString = Array.isArray(ifNoneMatch) ? ifNoneMatch[0] : ifNoneMatch;
 
+      if (ifNoneMatchString) {
+        const requestHash = unwrapEtag(ifNoneMatchString);
         if (etag === requestHash) {
           return response.notModified({ headers });
         }
@@ -225,14 +216,5 @@ export const registerFields = async (
   >,
   isRollupsEnabled: () => boolean
 ) => {
-  // handler
-  /* This seems to fail due to lack of custom headers on cache requests
-  router.versioned
-    .get({ path, access })
-    .addVersion(
-      { version, validate: { request: { query: querySchema }, response: validate.response } },
-      handler(isRollupsEnabled)
-    );
-    */
   router.get({ path, validate: { query: querySchema } }, handler(isRollupsEnabled));
 };