[ARVADOS-WORKBENCH2] created: 2.3.0-227-gc1c27e7f

Git user git at public.arvados.org
Mon Apr 4 20:06:56 UTC 2022


        at  c1c27e7fe3cbe068e939994e20b26a78348597c6 (commit)


commit c1c27e7fe3cbe068e939994e20b26a78348597c6
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Apr 4 17:03:05 2022 -0300

    18966: Changes back collectionService.get() to use the GET HTTP method.
    
    To avoid getting the manifest text, we hardcode the default list of selected
    fields instead of using the LIST request. This makes the code detect 404s
    correctly.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/src/models/collection.ts b/src/models/collection.ts
index 3effe672..9a5d2ee2 100644
--- a/src/models/collection.ts
+++ b/src/models/collection.ts
@@ -28,6 +28,40 @@ export interface CollectionResource extends TrashableResource, ResourceWithPrope
     fileSizeTotal: number;
 }
 
+// We exclude 'manifestText' and 'unsignedManifestText' from the default
+export const defaultCollectionSelectedFields = [
+    'name',
+    'description',
+    'portableDataHash',
+    'replicationDesired',
+    'replicationConfirmed',
+    'replicationConfirmedAt',
+    'storageClassesDesired',
+    'storageClassesConfirmed',
+    'storageClassesConfirmedAt',
+    'currentVersionUuid',
+    'version',
+    'preserveVersion',
+    'fileCount',
+    'fileSizeTotal',
+    // ResourceWithProperties field
+    'properties',
+    // TrashableResource fields
+    'trashAt',
+    'deleteAt',
+    'isTrashed',
+    // Resource fields
+    'uuid',
+    'ownerUuid',
+    'createdAt',
+    'modifiedByClientUuid',
+    'modifiedByUserUuid',
+    'modifiedAt',
+    'href',
+    'kind',
+    'etag',
+];
+
 export const getCollectionUrl = (uuid: string) => {
     return `/collections/${uuid}`;
 };
diff --git a/src/services/collection-service/collection-service.test.ts b/src/services/collection-service/collection-service.test.ts
index b759fd1a..61068369 100644
--- a/src/services/collection-service/collection-service.test.ts
+++ b/src/services/collection-service/collection-service.test.ts
@@ -4,7 +4,8 @@
 
 import axios, { AxiosInstance } from 'axios';
 import MockAdapter from 'axios-mock-adapter';
-import { CollectionResource } from 'models/collection';
+import { snakeCase } from 'lodash';
+import { CollectionResource, defaultCollectionSelectedFields } from 'models/collection';
 import { AuthService } from '../auth-service/auth-service';
 import { CollectionService } from './collection-service';
 
@@ -31,17 +32,16 @@ describe('collection-service', () => {
     });
 
     describe('get', () => {
-        it('should make a list request with uuid filtering', async () => {
+        it('should make a request with default selected fields', async () => {
             serverApi.get = jest.fn(() => Promise.resolve(
                 { data: { items: [{}] } }
             ));
             const uuid = 'zzzzz-4zz18-0123456789abcde'
             await collectionService.get(uuid);
             expect(serverApi.get).toHaveBeenCalledWith(
-                '/collections', {
+                `/collections/${uuid}`, {
                     params: {
-                        filters: `[["uuid","=","zzzzz-4zz18-0123456789abcde"]]`,
-                        include_old_versions: true,
+                        select: JSON.stringify(defaultCollectionSelectedFields.map(snakeCase)),
                     },
                 }
             );
@@ -54,10 +54,8 @@ describe('collection-service', () => {
             const uuid = 'zzzzz-4zz18-0123456789abcde'
             await collectionService.get(uuid, undefined, ['manifestText']);
             expect(serverApi.get).toHaveBeenCalledWith(
-                '/collections', {
+                `/collections/${uuid}`, {
                     params: {
-                        filters: `[["uuid","=","zzzzz-4zz18-0123456789abcde"]]`,
-                        include_old_versions: true,
                         select: `["manifest_text"]`
                     },
                 }
diff --git a/src/services/collection-service/collection-service.ts b/src/services/collection-service/collection-service.ts
index 0bddc82b..857828c2 100644
--- a/src/services/collection-service/collection-service.ts
+++ b/src/services/collection-service/collection-service.ts
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-import { CollectionResource } from "models/collection";
+import { CollectionResource, defaultCollectionSelectedFields } from "models/collection";
 import { AxiosInstance } from "axios";
 import { CollectionFile, CollectionDirectory } from "models/collection-file";
 import { WebDAV } from "common/webdav";
@@ -11,8 +11,6 @@ import { extractFilesData } from "./collection-service-files-response";
 import { TrashableResourceService } from "services/common-service/trashable-resource-service";
 import { ApiActions } from "services/api/api-actions";
 import { customEncodeURI } from "common/url";
-import { FilterBuilder } from "services/api/filter-builder";
-import { ListArguments } from "services/common-service/common-service";
 import { Session } from "models/session";
 
 export type UploadProgress = (fileId: number, loaded: number, total: number, currentTime: number) => void;
@@ -33,19 +31,8 @@ export class CollectionService extends TrashableResourceService<CollectionResour
 
     async get(uuid: string, showErrors?: boolean, select?: string[], session?: Session) {
         super.validateUuid(uuid);
-        // We use a filtered list request to avoid getting the manifest text
-        const filters = new FilterBuilder().addEqual('uuid', uuid).getFilters();
-        const listArgs: ListArguments = {filters, includeOldVersions: true};
-        if (select) {
-            listArgs.select = select;
-        }
-
-        if (!session) {
-            const lst = await super.list(listArgs, showErrors);
-            return lst.items[0];
-        } else {
-            return super.get(uuid, showErrors, select, session);
-        }
+        const selectParam = select || defaultCollectionSelectedFields;
+        return super.get(uuid, showErrors, selectParam, session);
     }
 
     create(data?: Partial<CollectionResource>) {
diff --git a/src/services/common-service/common-service.ts b/src/services/common-service/common-service.ts
index ddaf2ab0..f16a2024 100644
--- a/src/services/common-service/common-service.ts
+++ b/src/services/common-service/common-service.ts
@@ -117,7 +117,13 @@ export class CommonService<T> {
     get(uuid: string, showErrors?: boolean, select?: string[], session?: Session) {
         this.validateUuid(uuid);
 
-        const cfg: AxiosRequestConfig = {};
+        const cfg: AxiosRequestConfig = {
+            params: {
+                select: select
+                    ? `[${select.map(snakeCase).map(s => `"${s}"`).join(',')}]`
+                    : undefined
+            }
+        };
         if (session) {
             cfg.baseURL = session.baseUrl;
             cfg.headers = { 'Authorization': 'Bearer ' + session.token };
@@ -125,7 +131,7 @@ export class CommonService<T> {
 
         return CommonService.defaultResponse(
             this.serverApi
-                .get<T>(`/${this.resourceType}/${uuid}`, session ? cfg : undefined),
+                .get<T>(`/${this.resourceType}/${uuid}`, cfg),
             this.actions,
             true, // mapKeys
             showErrors

commit 272907ed21b66b72144237bf54a5347051f8693c
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Apr 4 12:15:16 2022 -0300

    18966: Expands test to expose the bug and cover other cases.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/cypress/integration/page-not-found.spec.js b/cypress/integration/page-not-found.spec.js
index 5d6a26b7..4df4135c 100644
--- a/cypress/integration/page-not-found.spec.js
+++ b/cypress/integration/page-not-found.spec.js
@@ -31,14 +31,22 @@ describe('Page not found tests', function() {
 
     it('shows not found popup', function() {
         // given
-        const notExistingUUID = 'zzzzz-tpzed-5o5tg0l9a57gxxx';
-
-        // when
-        cy.loginAs(adminUser);
-        cy.goToPath(`/projects/${notExistingUUID}`);
-
-        // then
-        cy.get('[data-cy=not-found-content]').should('exist');
-        cy.get('[data-cy=not-found-page]').should('not.exist');
+        [
+            '/projects/zzzzz-j7d0g-nonexistingproj',
+            '/projects/zzzzz-tpzed-nonexistinguser',
+            '/processes/zzzzz-xvhdp-nonexistingproc',
+            '/collections/zzzzz-4zz18-nonexistingcoll'
+        ].forEach(function(path) {
+            // Using de slower loginAs() method to avoid bumping into dialog
+            // dismissal issues that are not related to this test.
+            cy.loginAs(adminUser);
+
+            // when
+            cy.goToPath(path);
+
+            // then
+            cy.get('[data-cy=not-found-page]').should('not.exist');
+            cy.get('[data-cy=not-found-content]').should('exist');
+        });
     });
 })
\ No newline at end of file

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list