Merge branch '19899-webdav-cache-control' into main. Closes #19899

Arvados-DCO-1.1-Signed-off-by: Stephen Smith <stephen@curii.com>

diff --git a/cypress/integration/collection.spec.js b/cypress/integration/collection.spec.js
index 01d7001f2fe55ca0975578379905a012b6ffc0e1..efde53e5e87f1762695cb8bfe6a6192222a4091e 100644 (file)
--- a/cypress/integration/collection.spec.js
+++ b/cypress/integration/collection.spec.js
@@ -698,7 +698,6 @@ describe('Collection panel tests', function () {
 
     it('automatically updates the collection UI contents without using the Refresh button', function () {
         const collName = `Test Collection ${Math.floor(Math.random() * 999999)}`;
-        const fileName = 'foobar'
 
         cy.createCollection(adminUser.token, {
             name: collName,
@@ -707,22 +706,35 @@ describe('Collection panel tests', function () {
 
         cy.getAll('@testCollection').then(function ([testCollection]) {
             cy.loginAs(activeUser);
+
+            const files = [
+                "foobar",
+                "anotherFile",
+                "",
+                "finalName",
+            ];
+
             cy.goToPath(`/collections/${testCollection.uuid}`);
             cy.get('[data-cy=collection-files-panel]').should('contain', 'This collection is empty');
-            cy.get('[data-cy=collection-files-panel]').should('not.contain', fileName);
+            cy.get('[data-cy=collection-files-panel]').should('not.contain', files[0]);
             cy.get('[data-cy=collection-info-panel]').should('contain', collName);
 
-            cy.updateCollection(adminUser.token, testCollection.uuid, {
-                name: `${collName + ' updated'}`,
-                manifest_text: `. 37b51d194a7513e45b56f6524f2d51f2+3 0:3:${fileName}\n`,
-            }).as('updatedCollection');
-            cy.getAll('@updatedCollection').then(function ([updatedCollection]) {
-                expect(updatedCollection.name).to.equal(`${collName + ' updated'}`);
-                cy.get('[data-cy=collection-info-panel]').should('contain', updatedCollection.name);
-                cy.get('[data-cy=collection-files-panel]').should('contain', fileName);
+            files.map((fileName, i, files) => {
+                cy.updateCollection(adminUser.token, testCollection.uuid, {
+                    name: `${collName + ' updated'}`,
+                    manifest_text: fileName ? `. 37b51d194a7513e45b56f6524f2d51f2+3 0:3:${fileName}\n` : "",
+                }).as('updatedCollection');
+                cy.getAll('@updatedCollection').then(function ([updatedCollection]) {
+                    expect(updatedCollection.name).to.equal(`${collName + ' updated'}`);
+                    cy.get('[data-cy=collection-info-panel]').should('contain', updatedCollection.name);
+                    fileName
+                        ? cy.get('[data-cy=collection-files-panel]').should('contain', fileName)
+                        : cy.get('[data-cy=collection-files-panel]').should('not.contain', files[i-1]);;
+                });
             });
+
         });
-    })
+    });
 
     it('makes a copy of an existing collection', function() {
         const collName = `Test Collection ${Math.floor(Math.random() * 999999)}`;

diff --git a/src/common/webdav.test.ts b/src/common/webdav.test.ts
index 2ab106fcd3cb90a8710de2c0c5662cb8b53b78c5..0a3b5170f64d34cf817a81f55bafa1b6d41c3aa2 100644 (file)
--- a/src/common/webdav.test.ts
+++ b/src/common/webdav.test.ts
@@ -2,7 +2,6 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-import { customEncodeURI } from "./url";
 import { WebDAV } from "./webdav";
 
 describe('WebDAV', () => {
@@ -14,34 +13,36 @@ describe('WebDAV', () => {
         const request = await promise;
         expect(open).toHaveBeenCalledWith('PROPFIND', 'http://foo.com/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Authorization', 'Basic');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('allows to modify defaults after instantiation', async () => {
         const { open, load, setRequestHeader, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://foo.com/';
-        webdav.defaults.headers = { Authorization: 'Basic' };
+        const webdav = new WebDAV({ baseURL: 'http://foo.com/' }, createRequest);
+        webdav.setAuthorization('Basic');
         const promise = webdav.propfind('foo');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('PROPFIND', 'http://foo.com/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Authorization', 'Basic');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('PROPFIND', async () => {
-        const { open, load, createRequest } = mockCreateRequest();
+        const { open, load, setRequestHeader, createRequest } = mockCreateRequest();
         const webdav = new WebDAV(undefined, createRequest);
         const promise = webdav.propfind('foo');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('PROPFIND', 'foo');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('PUT', async () => {
-        const { open, send, load, progress, createRequest } = mockCreateRequest();
+        const { open, send, load, progress, setRequestHeader, createRequest } = mockCreateRequest();
         const webdav = new WebDAV(undefined, createRequest);
         const promise = webdav.put('foo', 'Test data');
         progress();
@@ -49,88 +50,90 @@ describe('WebDAV', () => {
         const request = await promise;
         expect(open).toHaveBeenCalledWith('PUT', 'foo');
         expect(send).toHaveBeenCalledWith('Test data');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('COPY', async () => {
         const { open, setRequestHeader, load, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://base';
+        const webdav = new WebDAV({ baseURL: 'http://base' }, createRequest);
         const promise = webdav.copy('foo', 'foo-copy');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('COPY', 'http://base/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Destination', 'http://base/foo-copy');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('COPY - adds baseURL with trailing slash to Destination header', async () => {
         const { open, setRequestHeader, load, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://base';
+        const webdav = new WebDAV({ baseURL: 'http://base' }, createRequest);
         const promise = webdav.copy('foo', 'foo-copy');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('COPY', 'http://base/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Destination', 'http://base/foo-copy');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('COPY - adds baseURL without trailing slash to Destination header', async () => {
         const { open, setRequestHeader, load, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://base';
+        const webdav = new WebDAV({ baseURL: 'http://base' }, createRequest);
         const promise = webdav.copy('foo', 'foo-copy');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('COPY', 'http://base/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Destination', 'http://base/foo-copy');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('MOVE', async () => {
         const { open, setRequestHeader, load, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://base';
+        const webdav = new WebDAV({ baseURL: 'http://base' }, createRequest);
         const promise = webdav.move('foo', 'foo-moved');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('MOVE', 'http://base/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Destination', 'http://base/foo-moved');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('MOVE - adds baseURL with trailing slash to Destination header', async () => {
         const { open, setRequestHeader, load, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://base';
+        const webdav = new WebDAV({ baseURL: 'http://base' }, createRequest);
         const promise = webdav.move('foo', 'foo-moved');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('MOVE', 'http://base/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Destination', 'http://base/foo-moved');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('MOVE - adds baseURL without trailing slash to Destination header', async () => {
         const { open, setRequestHeader, load, createRequest } = mockCreateRequest();
-        const webdav = new WebDAV(undefined, createRequest);
-        webdav.defaults.baseURL = 'http://base';
+        const webdav = new WebDAV({ baseURL: 'http://base' }, createRequest);
         const promise = webdav.move('foo', 'foo-moved');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('MOVE', 'http://base/foo');
         expect(setRequestHeader).toHaveBeenCalledWith('Destination', 'http://base/foo-moved');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 
     it('DELETE', async () => {
-        const { open, load, createRequest } = mockCreateRequest();
+        const { open, load, setRequestHeader, createRequest } = mockCreateRequest();
         const webdav = new WebDAV(undefined, createRequest);
         const promise = webdav.delete('foo');
         load();
         const request = await promise;
         expect(open).toHaveBeenCalledWith('DELETE', 'foo');
+        expect(setRequestHeader).toHaveBeenCalledWith('Cache-Control', 'must-revalidate');
         expect(request).toBeInstanceOf(XMLHttpRequest);
     });
 });

diff --git a/src/common/webdav.ts b/src/common/webdav.ts
index d4f904ae9832461abd777e5c8f6a112c49d14e65..bb8a68bdd221e1733d42f0f64b8c61e434863304 100644 (file)
--- a/src/common/webdav.ts
+++ b/src/common/webdav.ts
@@ -6,17 +6,29 @@ import { customEncodeURI } from "./url";
 
 export class WebDAV {
 
-    defaults: WebDAVDefaults = {
+    private defaults: WebDAVDefaults = {
         baseURL: '',
-        headers: {},
+        headers: {
+            'Cache-Control': 'must-revalidate'
+        },
     };
 
     constructor(config?: Partial<WebDAVDefaults>, private createRequest = () => new XMLHttpRequest()) {
         if (config) {
-            this.defaults = { ...this.defaults, ...config };
+            this.defaults = {
+                ...this.defaults,
+                ...config,
+                headers: {
+                    ...this.defaults.headers,
+                    ...config.headers
+                },
+            };
         }
     }
 
+    getBaseUrl = (): string => this.defaults.baseURL;
+    setAuthorization = (token?) => this.defaults.headers.Authorization = token;
+
     propfind = (url: string, config: WebDAVRequestConfig = {}) =>
         this.request({
             ...config, url,

diff --git a/src/components/collection-panel-files/collection-panel-files.tsx b/src/components/collection-panel-files/collection-panel-files.tsx
index 2ba29d44801435ccc258ebfd0fea533186567bd7..08944d40a93519b59770ea52df546f2720d68ad8 100644 (file)
--- a/src/components/collection-panel-files/collection-panel-files.tsx
+++ b/src/components/collection-panel-files/collection-panel-files.tsx
@@ -233,11 +233,12 @@ export const CollectionPanelFiles = withStyles(styles)(connect((state: RootState
     const { classes, onItemMenuOpen, onUploadDataClick, isWritable, dispatch, collectionPanelFiles, collectionPanel } = props;
     const { apiToken, config } = props.auth;
 
-    const webdavClient = new WebDAV();
-    webdavClient.defaults.baseURL = config.keepWebServiceUrl;
-    webdavClient.defaults.headers = {
-        Authorization: `Bearer ${apiToken}`
-    };
+    const webdavClient = new WebDAV({
+        baseURL: config.keepWebServiceUrl,
+        headers: {
+            Authorization: `Bearer ${apiToken}`
+        },
+    });
 
     const webDAVRequestConfig: WebDAVRequestConfig = {
         headers: {

diff --git a/src/services/collection-service/collection-service.ts b/src/services/collection-service/collection-service.ts
index d08e7899568ea857807c8d302b39980bc2082098..1a03d8da3a1f536108b3707239714ec1eea73785 100644 (file)
--- a/src/services/collection-service/collection-service.ts
+++ b/src/services/collection-service/collection-service.ts
@@ -93,9 +93,9 @@ export class CollectionService extends TrashableResourceService<CollectionResour
     }
 
     extendFileURL = (file: CollectionDirectory | CollectionFile) => {
-        const baseUrl = this.webdavClient.defaults.baseURL.endsWith('/')
-            ? this.webdavClient.defaults.baseURL.slice(0, -1)
-            : this.webdavClient.defaults.baseURL;
+        const baseUrl = this.webdavClient.getBaseUrl().endsWith('/')
+            ? this.webdavClient.getBaseUrl().slice(0, -1)
+            : this.webdavClient.getBaseUrl();
         const apiToken = this.authService.getApiToken();
         const encodedApiToken = apiToken ? encodeURI(apiToken) : '';
         const userApiToken = `/t=${encodedApiToken}/`;

diff --git a/src/services/services.ts b/src/services/services.ts
index 2afb843f6c7760303512fd240e763ccaaa588d5e..4e4a682ebe065ed7247695c1ba760df5d0833f06 100644 (file)
--- a/src/services/services.ts
+++ b/src/services/services.ts
@@ -39,14 +39,12 @@ export function setAuthorizationHeader(services: ServiceRepository, token: strin
     services.apiClient.defaults.headers.common = {
         Authorization: `Bearer ${token}`
     };
-    services.webdavClient.defaults.headers = {
-        Authorization: `Bearer ${token}`
-    };
+    services.webdavClient.setAuthorization(`Bearer ${token}`);
 }
 
 export function removeAuthorizationHeader(services: ServiceRepository) {
     delete services.apiClient.defaults.headers.common;
-    delete services.webdavClient.defaults.headers.common;
+    services.webdavClient.setAuthorization(undefined);
 }
 
 export const createServices = (config: Config, actions: ApiActions, useApiClient?: AxiosInstance) => {
@@ -57,8 +55,9 @@ export const createServices = (config: Config, actions: ApiActions, useApiClient
     const apiClient = useApiClient || Axios.create({ headers: {} });
     apiClient.defaults.baseURL = config.baseUrl;
 
-    const webdavClient = new WebDAV();
-    webdavClient.defaults.baseURL = config.keepWebServiceUrl;
+    const webdavClient = new WebDAV({
+        baseURL: config.keepWebServiceUrl
+    });
 
     const apiClientAuthorizationService = new ApiClientAuthorizationService(apiClient, actions);
     const authorizedKeysService = new AuthorizedKeysService(apiClient, actions);

diff --git a/tools/arvados_config.yml b/tools/arvados_config.yml
index cfa1a3a975f2be07ff493db3c9c7bfa718f64bbd..1ef77b86ce8aa6b71e58ea83e91c9eec31bcf32f 100644 (file)
--- a/tools/arvados_config.yml
+++ b/tools/arvados_config.yml
@@ -16,8 +16,6 @@ Clusters:
       ForwardSlashNameSubstitution: /
       ManagedProperties:
         original_owner_uuid: {Function: original_owner, Protected: true}
-      WebDAVCache:
-        TTL: 0s
     Login:
       TrustPrivateNetworks: true
       PAM: