Merge pull request #11989 from opf/implementation/45875-change-api-response-of-storage-files-endpoint

[#45875] Change API response of storage files endpoint

frontend/src/app/core/state/storage-files/storage-files.model.ts

@@ -26,7 +26,12 @@
 // See COPYRIGHT and LICENSE files for more details.
 //++
 
-import { QueryEntity } from '@datorama/akita';
+import { IHalResourceLinks } from 'core-app/core/state/hal-resource';
-import { StoragesState } from 'core-app/core/state/storages/storages.store';
+import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model';
 
-export class StoragesQuery extends QueryEntity<StoragesState> {}
+export interface IStorageFiles {
+  files:IStorageFile[];
+  parent:IStorageFile;
+  _type:'StorageFiles';
+  _links:IHalResourceLinks;
+}

frontend/src/app/core/state/storage-files/storage-files.service.ts

@@ -27,52 +27,79 @@
 //++
 
 import { Injectable } from '@angular/core';
-import { Observable } from 'rxjs';
+import { combineLatest, Observable } from 'rxjs';
-import { map, take, tap } from 'rxjs/operators';
-
 import {
-  CollectionStore,
+  filter, map, take, tap,
-  ResourceCollectionService,
+} from 'rxjs/operators';
-} from 'core-app/core/state/resource-collection.service';
+
-import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model';
-import { IHALCollection } from 'core-app/core/apiv3/types/hal-collection.type';
 import { IHalResourceLink } from 'core-app/core/state/hal-resource';
 import { StorageFilesStore } from 'core-app/core/state/storage-files/storage-files.store';
-import { insertCollectionIntoState } from 'core-app/core/state/collection-store';
 import { IUploadLink } from 'core-app/core/state/storage-files/upload-link.model';
 import { IPrepareUploadLink } from 'core-app/core/state/storages/storage.model';
+import { IStorageFiles } from 'core-app/core/state/storage-files/storage-files.model';
+import { HttpClient } from '@angular/common/http';
+import { ID, QueryEntity } from '@datorama/akita';
+import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model';
+import isDefinedEntity from 'core-app/core/state/is-defined-entity';
 
 @Injectable()
-export class StorageFilesResourceService extends ResourceCollectionService<IStorageFile> {
+export class StorageFilesResourceService {
-  protected createStore():CollectionStore<IStorageFile> {
+  private readonly store:StorageFilesStore = new StorageFilesStore();
-    return new StorageFilesStore();
+
-  }
+  private readonly query = new QueryEntity(this.store);
+
+  constructor(private readonly httpClient:HttpClient) {}
 
-  files(link:IHalResourceLink):Observable<IStorageFile[]> {
+  files(link:IHalResourceLink):Observable<IStorageFiles> {
-    if (this.collectionExists(link.href)) {
+    const value = this.store.getValue().files[link.href];
-      return this.collection(link.href);
+    if (value !== undefined) {
+      return combineLatest([this.lookupMany(value.files), this.lookup(value.parent)])
+        .pipe(
+          map(([files, parent]):IStorageFiles => ({
+            files, parent, _type: 'StorageFiles', _links: { self: link },
+          })),
+          take(1),
+        );
     }
 
-    return this.http
+    return this.httpClient
-      .get<IHALCollection<IStorageFile>>(link.href)
+      .get<IStorageFiles>(link.href)
-      .pipe(
+      .pipe(tap((storageFiles) => this.insert(storageFiles, link.href)));
-        tap((collection) => {
-          insertCollectionIntoState(this.store, collection, link.href);
-        }),
-        map((collection) => collection._embedded.elements),
-        take(1),
-      );
   }
 
   uploadLink(link:IPrepareUploadLink):Observable<IUploadLink> {
-    return this.http.request<IUploadLink>(link.method, link.href, { body: link.payload });
+    return this.httpClient.request<IUploadLink>(link.method, link.href, { body: link.payload });
   }
 
   reset():void {
     this.store.reset();
   }
 
-  protected basePath():string {
+  private lookup(id:ID):Observable<IStorageFile> {
-    return this.apiV3Service.storages.files.path;
+    return this
+      .query
+      .selectEntity(id)
+      .pipe(filter(isDefinedEntity));
+  }
+
+  private lookupMany(ids:ID[]):Observable<IStorageFile[]> {
+    return this.query.selectMany(ids);
+  }
+
+  private insert(storageFiles:IStorageFiles, link:string):void {
+    this.store.upsertMany([...storageFiles.files, storageFiles.parent]);
+
+    const fileIds = storageFiles.files.map((file) => file.id);
+    const parentId = storageFiles.parent.id;
+
+    this.store.update(({ files }) => ({
+      files: {
+        ...files,
+        [link]: {
+          files: fileIds,
+          parent: parentId,
+        },
+      },
+    }));
   }
 }

frontend/src/app/core/state/storage-files/storage-files.store.ts

@@ -26,18 +26,26 @@
 // See COPYRIGHT and LICENSE files for more details.
 //++
 
-import { EntityStore, StoreConfig } from '@datorama/akita';
+import {
-import { CollectionState, createInitialCollectionState } from 'core-app/core/state/collection-store';
+  EntityState, EntityStore, ID, StoreConfig,
+} from '@datorama/akita';
 import { IStorageFile } from 'core-app/core/state/storage-files/storage-file.model';
 
-export interface StorageFilesState extends CollectionState<IStorageFile> {}
+export interface IStorageFilesResponse {
+  files:ID[];
+  parent:ID;
+}
+
+export interface IStorageFilesState extends EntityState<IStorageFile> {
+  files:Record<string, IStorageFilesResponse>;
+}
 
 @StoreConfig({
   name: 'storage-files',
   resettable: true,
 })
-export class StorageFilesStore extends EntityStore<StorageFilesState> {
+export class StorageFilesStore extends EntityStore<IStorageFilesState> {
   constructor() {
-    super(createInitialCollectionState());
+    super({ files: {} });
   }
 }

frontend/src/app/shared/components/storages/file-picker-base-modal.component.ts/file-picker-base-modal.component.ts

@@ -96,8 +96,8 @@ export abstract class FilePickerBaseModalComponent extends OpModalComponent impl
 
     this.storageFilesResourceService
       .files(makeFilesCollectionLink(this.storageLink, '/'))
-      .subscribe((files) => {
+      .subscribe((storageFiles) => {
-        const root = files.find((file) => file.name === '/');
+        const root = storageFiles.parent;
         if (root === undefined) {
           throw new Error('Collection does not contain a root directory!');
         }
@@ -112,7 +112,7 @@ export abstract class FilePickerBaseModalComponent extends OpModalComponent impl
           }],
         );
 
-        this.storageFiles$.next(files.filter((file) => file.name !== '/'));
+        this.storageFiles$.next(storageFiles.files);
         this.loading$.next(false);
       });
   }
@@ -157,7 +157,7 @@ export abstract class FilePickerBaseModalComponent extends OpModalComponent impl
 
     this.loadingSubscription = this.storageFilesResourceService
       .files(makeFilesCollectionLink(this.storageLink, directory.location))
-      .pipe(map((files) => files.filter((file) => file.name !== this.currentDirectory.name)))
+      .pipe(map((storageFiles) => storageFiles.files.filter((file) => file.name !== this.currentDirectory.name)))
       .subscribe((files) => {
         this.storageFiles$.next(files);
         this.loading$.next(false);

modules/storages/app/common/storages/peripherals/storage_interaction/nextcloud/files_query.rb

@@ -106,9 +106,12 @@ module Storages::Peripherals::StorageInteraction::Nextcloud
               .xpath('//d:response')
               .to_a
 
-        a.map do |file_element|
+        parent, *files =
-          storage_file(file_element)
+          a.map do |file_element|
-        end
+            storage_file(file_element)
+          end
+
+        ::Storages::StorageFiles.new(files, parent)
       end
     end
 

modules/storages/app/models/storages/storage_files.rb

@@ -26,7 +26,11 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-module API::V3::StorageFiles
+class Storages::StorageFiles
-  class StorageFileCollectionRepresenter < ::API::Decorators::UnpaginatedCollection
+  attr_reader :files, :parent
+
+  def initialize(files, parent = nil)
+    @files = files
+    @parent = parent
   end
 end

modules/storages/lib/api/v3/storage_files/storage_files_api.rb

@@ -38,9 +38,8 @@ module API::V3::StorageFiles
         (files_query(@storage, current_user) >> execute_files_query(params[:parent]))
           .match(
             on_success: ->(files) do
-              API::V3::StorageFiles::StorageFileCollectionRepresenter.new(
+              API::V3::StorageFiles::StorageFilesRepresenter.new(
                 files,
-                self_link: api_v3_paths.storage_files(@storage.id),
                 current_user:
               )
             end,

modules/storages/lib/api/v3/storage_files/storage_files_representer.rb

@@ -0,0 +1,49 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module API::V3::StorageFiles
+  class StorageFilesRepresenter < ::API::Decorators::Single
+    link :self do
+      { href: "#{::API::V3::URN_PREFIX}storages:storage_files:no_link_provided" }
+    end
+
+    collection :files,
+               getter: ->(*) do
+                 represented.files.map { |file| API::V3::StorageFiles::StorageFileRepresenter.new(file, current_user:) }
+               end,
+               exec_context: :decorator
+
+    property :parent,
+             getter: ->(*) { API::V3::StorageFiles::StorageFileRepresenter.new(represented.parent, current_user:) },
+             exec_context: :decorator
+
+    def _type
+      'StorageFiles'
+    end
+  end
+end

modules/storages/spec/common/peripherals/storage_requests_spec.rb

@@ -204,7 +204,8 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
             on_success: ->(query) do
               result = query.call(nil)
               expect(result).to be_success
-              expect(result.result.size).to eq(5)
+              expect(result.result.files.size).to eq(4)
+              expect(result.result.parent).not_to be_nil
             end,
             on_failure: ->(error) do
               raise "Files query could not be created: #{error}"
@@ -219,9 +220,9 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
             on_success: ->(query) do
               result = query.call(nil)
               expect(result).to be_success
-              expect(result.result[1].name).to eq('Folder1')
+              expect(result.result.files[0].name).to eq('Folder1')
-              expect(result.result[1].mime_type).to eq('application/x-op-directory')
+              expect(result.result.files[0].mime_type).to eq('application/x-op-directory')
-              expect(result.result[1].id).to eq('11')
+              expect(result.result.files[0].id).to eq('11')
             end,
             on_failure: ->(error) do
               raise "Files query could not be created: #{error}"
@@ -237,13 +238,13 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
               result = query.call(nil)
               expect(result).to be_success
 
-              expect(result.result[1].mime_type).to eq('application/x-op-directory')
+              expect(result.result.files[0].mime_type).to eq('application/x-op-directory')
-              expect(result.result[1].permissions).to include(:readable)
+              expect(result.result.files[0].permissions).to include(:readable)
-              expect(result.result[1].permissions).to include(:writeable)
+              expect(result.result.files[0].permissions).to include(:writeable)
 
-              expect(result.result[2].mime_type).to eq('application/x-op-directory')
+              expect(result.result.files[1].mime_type).to eq('application/x-op-directory')
-              expect(result.result[2].permissions).to include(:readable)
+              expect(result.result.files[1].permissions).to include(:readable)
-              expect(result.result[2].permissions).not_to include(:writeable)
+              expect(result.result.files[1].permissions).not_to include(:writeable)
             end,
             on_failure: ->(error) do
               raise "Files query could not be created: #{error}"
@@ -259,13 +260,13 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
               result = query.call(nil)
               expect(result).to be_success
 
-              expect(result.result[3].mime_type).to eq('text/markdown')
+              expect(result.result.files[2].mime_type).to eq('text/markdown')
-              expect(result.result[3].permissions).to include(:readable)
+              expect(result.result.files[2].permissions).to include(:readable)
-              expect(result.result[3].permissions).to include(:writeable)
+              expect(result.result.files[2].permissions).to include(:writeable)
 
-              expect(result.result[4].mime_type).to eq('application/pdf')
+              expect(result.result.files[3].mime_type).to eq('application/pdf')
-              expect(result.result[4].permissions).to include(:readable)
+              expect(result.result.files[3].permissions).to include(:readable)
-              expect(result.result[4].permissions).not_to include(:writeable)
+              expect(result.result.files[3].permissions).not_to include(:writeable)
             end,
             on_failure: ->(error) do
               raise "Files query could not be created: #{error}"
@@ -280,9 +281,9 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
             on_success: ->(query) do
               result = query.call(nil)
               expect(result).to be_success
-              expect(result.result[3].name).to eq('README.md')
+              expect(result.result.files[2].name).to eq('README.md')
-              expect(result.result[3].mime_type).to eq('text/markdown')
+              expect(result.result.files[2].mime_type).to eq('text/markdown')
-              expect(result.result[3].id).to eq('12')
+              expect(result.result.files[2].id).to eq('12')
             end,
             on_failure: ->(error) do
               raise "Files query could not be created: #{error}"
@@ -299,7 +300,7 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
             .match(
               on_success: ->(query) {
                 result = query.call(parent)
-                expect(result.result[3].location).to eq('/Photos/Birds/README.md')
+                expect(result.result.files[2].location).to eq('/Photos/Birds/README.md')
               },
               on_failure: ->(error) { raise "Files query could not be created: #{error}" }
             )
@@ -317,7 +318,7 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
             .match(
               on_success: ->(query) {
                 result = query.call(nil)
-                expect(result.result[3].location).to eq('/README.md')
+                expect(result.result.files[2].location).to eq('/README.md')
               },
               on_failure: ->(error) { raise "Files query could not be created: #{error}" }
             )
@@ -336,7 +337,7 @@ describe Storages::Peripherals::StorageRequests, webmock: true do
             .match(
               on_success: ->(query) {
                 result = query.call(parent)
-                expect(result.result[3].location).to eq('/Photos/Birds/README.md')
+                expect(result.result.files[2].location).to eq('/Photos/Birds/README.md')
               },
               on_failure: ->(error) { raise "Files query could not be created: #{error}" }
             )

modules/storages/spec/lib/api/v3/storage_files/storage_files_representer_spec.rb

@@ -0,0 +1,88 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require 'spec_helper'
+
+describe API::V3::StorageFiles::StorageFilesRepresenter do
+  let(:user) { build_stubbed(:user) }
+  let(:created_at) { DateTime.now }
+  let(:last_modified_at) { DateTime.now }
+
+  let(:parent) do
+    Storages::StorageFile.new(
+      23,
+      '/',
+      2048,
+      'application/x-op-directory',
+      created_at,
+      last_modified_at,
+      'admin',
+      'admin',
+      '/',
+      %i[readable writeable]
+    )
+  end
+
+  let(:file) do
+    Storages::StorageFile.new(
+      42,
+      'readme.md',
+      4096,
+      'text/plain',
+      created_at,
+      last_modified_at,
+      'admin',
+      'admin',
+      '/readme.md',
+      %i[readable writeable]
+    )
+  end
+
+  let(:files) do
+    Storages::StorageFiles.new([file], parent)
+  end
+
+  let(:representer) { described_class.new(files, current_user: user) }
+
+  subject { representer.to_json }
+
+  describe 'properties' do
+    it_behaves_like 'property', :_type do
+      let(:value) { representer._type }
+    end
+
+    it_behaves_like 'collection', :files do
+      let(:value) { files.files }
+      let(:element_decorator) { API::V3::StorageFiles::StorageFileRepresenter }
+    end
+
+    it_behaves_like 'property', :parent do
+      let(:value) { API::V3::StorageFiles::StorageFileRepresenter.new(files.parent, current_user: user) }
+    end
+  end
+end

modules/storages/spec/requests/api/v3/storages/storage_files_spec.rb

@@ -62,20 +62,24 @@ describe 'API v3 storage files', content_type: :json, webmock: true do
   describe 'GET /api/v3/storages/:storage_id/files' do
     let(:path) { api_v3_paths.storage_files(storage.id) }
 
-    let(:files) do
+    let(:response) do
-      [
+      Storages::StorageFiles.new(
-        Storages::StorageFile.new(1, 'new_younglings.md', 4096, 'text/markdown', DateTime.now, DateTime.now,
+        [
-                                  'Obi-Wan Kenobi', 'Obi-Wan Kenobi', '/', %i[readable]),
+          Storages::StorageFile.new(1, 'new_younglings.md', 4096, 'text/markdown', DateTime.now, DateTime.now,
-        Storages::StorageFile.new(2, 'holocron_inventory.md', 4096, 'text/markdown', DateTime.now, DateTime.now,
+                                    'Obi-Wan Kenobi', 'Obi-Wan Kenobi', '/', %i[readable]),
+          Storages::StorageFile.new(2, 'holocron_inventory.md', 4096, 'text/markdown', DateTime.now, DateTime.now,
+                                    'Obi-Wan Kenobi', 'Obi-Wan Kenobi', '/', %i[readable writeable])
+        ],
+        Storages::StorageFile.new(32, '/', 4096 * 2, 'application/x-op-directory', DateTime.now, DateTime.now,
                                   'Obi-Wan Kenobi', 'Obi-Wan Kenobi', '/', %i[readable writeable])
-      ]
+      )
     end
 
     describe 'with successful response' do
       before do
         storage_requests = instance_double(Storages::Peripherals::StorageRequests)
         files_query = Proc.new do
-          ServiceResult.success(result: files)
+          ServiceResult.success(result: response)
         end
         allow(storage_requests).to receive(:files_query).and_return(ServiceResult.success(result: files_query))
         allow(Storages::Peripherals::StorageRequests).to receive(:new).and_return(storage_requests)
@@ -83,14 +87,16 @@ describe 'API v3 storage files', content_type: :json, webmock: true do
 
       subject { last_response.body }
 
-      it { is_expected.to be_json_eql(files.length.to_json).at_path('count') }
+      it { is_expected.to be_json_eql(response.files[0].id.to_json).at_path('files/0/id') }
-      it { is_expected.to be_json_eql(files[0].id.to_json).at_path('_embedded/elements/0/id') }
+      it { is_expected.to be_json_eql(response.files[0].name.to_json).at_path('files/0/name') }
-      it { is_expected.to be_json_eql(files[0].name.to_json).at_path('_embedded/elements/0/name') }
+      it { is_expected.to be_json_eql(response.files[1].id.to_json).at_path('files/1/id') }
-      it { is_expected.to be_json_eql(files[1].id.to_json).at_path('_embedded/elements/1/id') }
+      it { is_expected.to be_json_eql(response.files[1].name.to_json).at_path('files/1/name') }
-      it { is_expected.to be_json_eql(files[1].name.to_json).at_path('_embedded/elements/1/name') }
 
-      it { is_expected.to be_json_eql(files[0].permissions.to_json).at_path('_embedded/elements/0/permissions') }
+      it { is_expected.to be_json_eql(response.files[0].permissions.to_json).at_path('files/0/permissions') }
-      it { is_expected.to be_json_eql(files[1].permissions.to_json).at_path('_embedded/elements/1/permissions') }
+      it { is_expected.to be_json_eql(response.files[1].permissions.to_json).at_path('files/1/permissions') }
+
+      it { is_expected.to be_json_eql(response.parent.id.to_json).at_path('parent/id') }
+      it { is_expected.to be_json_eql(response.parent.name.to_json).at_path('parent/name') }
     end
 
     describe 'with files query creation failed' do

spec/lib/api/v3/support/collection_examples.rb

@@ -0,0 +1,37 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+shared_examples_for 'collection' do |name|
+  it "has the #{name} property" do
+    represented_elements = value.map { |v| element_decorator.new(v, current_user: user) }
+
+    expect(subject)
+      .to be_json_eql(represented_elements.to_json)
+            .at_path(name.to_s)
+  end
+end