Skip to content

Commit

Permalink
[#57708] fixed nextcloud sync service
Browse files Browse the repository at this point in the history 
- use UrlBuilder.path to compare paths
- fixed test setup
  • Loading branch information
@Kharonus
Kharonus committed Sep 10, 2024
1 parent a8731ce commit 92277f6
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 153 deletions.
10 changes: 5 additions & 5 deletions modules/storages/app/services/storages/nextcloud_managed_folder_sync_service.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def prepare_remote_folders
remote_folders = remote_root_folder_map(@storage.group_folder).on_failure { return _1 }.result
info "Found #{remote_folders.count} remote folders"

ensure_root_folder_permissions(remote_folders["/#{@storage.group_folder}/"]["fileid"]).on_failure { return _1 }
ensure_root_folder_permissions(remote_folders["/#{@storage.group_folder}"].id).on_failure { return _1 }

ensure_folders_exist(remote_folders).on_success { hide_inactive_folders(remote_folders) }
end
Expand Down Expand Up @@ -174,8 +174,8 @@ def hide_inactive_folders(remote_folders)
info "Hiding folders related to inactive projects"
project_folder_ids = active_project_storages_scope.pluck(:project_folder_id).compact

remote_folders.except("/#{@storage.group_folder}/").each do |(path, attrs)|
folder_id = attrs["fileid"]
remote_folders.except("/#{@storage.group_folder}").each do |(path, file)|
folder_id = file.id

next if project_folder_ids.include?(folder_id)

Expand All @@ -201,7 +201,7 @@ def hide_inactive_folders(remote_folders)

def ensure_folders_exist(remote_folders)
info "Ensuring that automatically managed project folders exist and are correctly named."
id_folder_map = remote_folders.to_h { |folder, properties| [properties["fileid"], folder] }
id_folder_map = remote_folders.to_h { |path, file| [file.id, path] }

active_project_storages_scope.includes(:project).map do |project_storage|
unless id_folder_map.key?(project_storage.project_folder_id)
Expand All @@ -220,7 +220,7 @@ def ensure_folders_exist(remote_folders)
# @param current_path [String] current name of the remote project storage folder
# @return [ServiceResult, nil]
def rename_folder(project_storage, current_path)
return if current_path == project_storage.managed_project_folder_path
return if UrlBuilder.path(current_path) == UrlBuilder.path(project_storage.managed_project_folder_path)

name = project_storage.managed_project_folder_name
file_id = project_storage.project_folder_id
Expand Down
122 changes: 0 additions & 122 deletions modules/storages/spec/common/storages/peripherals/registry_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,128 +213,6 @@
end
end

describe "#file_ids_query" do
let(:nextcloud_subpath) { "" }
let(:url) { "https://example.com#{nextcloud_subpath}" }
let(:expected_request_body) do
<<~XML
<?xml version="1.0"?>
<d:propfind xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns" xmlns:nc="http://nextcloud.org/ns">
<d:prop>
<oc:fileid/>
</d:prop>
</d:propfind>
XML
end
let(:expected_response_body) do
<<~XML
<?xml version="1.0"?>
<d:multistatus
xmlns:d="DAV:"
xmlns:s="http://sabredav.org/ns"
xmlns:oc="http://owncloud.org/ns"
xmlns:nc="http://nextcloud.org/ns">
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>349</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/asd/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>783</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/Project%231/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>773</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/Project%20%232/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>381</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/Project%232/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>398</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/qwe/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>767</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
<d:response>
<d:href>#{nextcloud_subpath}/remote.php/dav/files/OpenProject/OpenProject/qweekk/</d:href>
<d:propstat>
<d:prop>
<oc:fileid>802</oc:fileid>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
</d:multistatus>
XML
end

before do
stub_request(:propfind, "#{url}/remote.php/dav/files/OpenProject/OpenProject").with(
body: expected_request_body,
headers: {
"Authorization" => "Basic T3BlblByb2plY3Q6T3BlblByb2plY3RTZWN1cmVQYXNzd29yZA==",
"Depth" => "1"
}
).to_return(status: 200, body: expected_response_body, headers: {})
end

shared_examples "a file_ids_query response" do
it "responds with a list of paths and attributes for each of them" do
result = registry.resolve("nextcloud.queries.file_ids")
.call(storage:, path: "OpenProject")
.result
expect(result).to eq({ "/OpenProject/" => { "fileid" => "349" },
"/OpenProject/Project #2/" => { "fileid" => "381" },
"/OpenProject/Project#1/" => { "fileid" => "773" },
"/OpenProject/Project#2/" => { "fileid" => "398" },
"/OpenProject/asd/" => { "fileid" => "783" },
"/OpenProject/qwe/" => { "fileid" => "767" },
"/OpenProject/qweekk/" => { "fileid" => "802" } })
end
end

it_behaves_like "a file_ids_query response"

context "when NC is deployed under subpath" do
let(:nexcloud_subpath) { "/subpath" }

it_behaves_like "a file_ids_query response"
end
end

describe "#delete_folder_command" do
let(:auth_strategy) { Storages::Peripherals::StorageInteraction::AuthenticationStrategies::BasicAuth.strategy }

Expand Down
70 changes: 44 additions & 26 deletions modules/storages/spec/services/storages/nextcloud_managed_folder_sync_service_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,14 @@ module Storages

shared_let(:remote_identities) do
[create(:remote_identity, user: admin, oauth_client: storage.oauth_client, origin_user_id: "admin"),
create(:remote_identity, user: multiple_projects_user, oauth_client: storage.oauth_client,
origin_user_id: "multiple_projects_user"),
create(:remote_identity, user: single_project_user, oauth_client: storage.oauth_client,
origin_user_id: "single_project_user")]
create(:remote_identity,
user: multiple_projects_user,
oauth_client: storage.oauth_client,
origin_user_id: "multiple_projects_user"),
create(:remote_identity,
user: single_project_user,
oauth_client: storage.oauth_client,
origin_user_id: "single_project_user")]
end

shared_let(:non_member_role) { create(:non_member, permissions: ["read_files"]) }
Expand All @@ -69,12 +73,14 @@ module Storages
create(:project, :archived, name: "INACTIVE PROJECT", members: { multiple_projects_user => ordinary_role })
end
shared_let(:project) do
create(:project, name: "[Sample] Project Name / Ehüu ///",
members: { multiple_projects_user => ordinary_role, single_project_user => ordinary_role })
create(:project,
name: "[Sample] Project Name / Ehüu ///",
members: { multiple_projects_user => ordinary_role, single_project_user => ordinary_role })
end
shared_let(:renamed_project) do
create(:project, name: "Renamed Project #23",
members: { multiple_projects_user => ordinary_role })
create(:project,
name: "Renamed Project #23",
members: { multiple_projects_user => ordinary_role })
end

let!(:public_storage) { create(:project_storage, :as_automatically_managed, storage:, project: public_project) }
Expand All @@ -89,7 +95,7 @@ module Storages
storage:, project: renamed_project, project_folder_id: "9001")
end

let(:file_ids) { class_double(Peripherals::StorageInteraction::Nextcloud::FileIdsQuery) }
let(:file_path_to_id_map) { class_double(Peripherals::StorageInteraction::Nextcloud::FilePathToIdMapQuery) }
let(:group_users) { class_double(Peripherals::StorageInteraction::Nextcloud::GroupUsersQuery) }
let(:rename_file) { class_double(Peripherals::StorageInteraction::Nextcloud::RenameFileCommand) }
let(:set_permissions) { class_double(Peripherals::StorageInteraction::Nextcloud::SetPermissionsCommand) }
Expand All @@ -99,12 +105,14 @@ module Storages
let(:auth_strategy) { Peripherals::StorageInteraction::AuthenticationStrategies::Strategy.new(key: :basic_auth) }

let(:root_folder_id) { "root_folder_id" }
let(:file_ids_result) do
let(:file_path_to_id_map_result) do
inactive_storage_path = inactive_storage.managed_project_folder_path.chomp("/")

ServiceResult.success(
result: {
"/OpenProject/" => { "fileid" => root_folder_id },
inactive_storage.managed_project_folder_path => { "fileid" => inactive_storage.project_folder_id },
"/OpenProject/Another Name for this Project/" => { "fileid" => renamed_storage.project_folder_id }
"/OpenProject" => StorageFileId.new(root_folder_id),
inactive_storage_path => StorageFileId.new(inactive_storage.project_folder_id),
"/OpenProject/Another Name for this Project" => StorageFileId.new(renamed_storage.project_folder_id)
}
)
end
Expand Down Expand Up @@ -138,7 +146,7 @@ module Storages
let(:create_folder_result) { build_create_folder_result }

before do
Peripherals::Registry.stub("nextcloud.queries.file_ids", file_ids)
Peripherals::Registry.stub("nextcloud.queries.file_path_to_id_map", file_path_to_id_map)
Peripherals::Registry.stub("nextcloud.queries.group_users", group_users)
Peripherals::Registry.stub("nextcloud.commands.add_user_to_group", add_user)
Peripherals::Registry.stub("nextcloud.commands.create_folder", create_folder)
Expand All @@ -148,7 +156,10 @@ module Storages
Peripherals::Registry.stub("nextcloud.authentication.userless", -> { auth_strategy })

# We arent using ParentFolder nor AuthStrategies on FileIds
allow(file_ids).to receive(:call).with(storage:, path: storage.group).and_return(file_ids_result)
folder = Peripherals::ParentFolder.new(storage.group)
allow(file_path_to_id_map).to receive(:call).with(storage:, auth_strategy:, folder:, depth: 1)
.and_return(file_path_to_id_map_result)

# Setting the Group Permissions
allow(set_permissions).to receive(:call).with(storage:, auth_strategy:, input_data: root_permission_input)
.and_return(root_permissions_result)
Expand Down Expand Up @@ -191,11 +202,11 @@ module Storages
end

context "when a project is renamed" do
let(:file_ids_result) do
let(:file_path_to_id_map_result) do
ServiceResult.success(
result: {
"/OpenProject/" => { "fileid" => root_folder_id },
"/OpenProject/OBVIOUSLY NON RENAMED/" => { "fileid" => renamed_storage.project_folder_id }
"/OpenProject" => StorageFileId.new(root_folder_id),
"/OpenProject/OBVIOUSLY NON RENAMED" => StorageFileId.new(renamed_storage.project_folder_id)
}
)
end
Expand All @@ -221,7 +232,9 @@ module Storages
end

context "with a public project" do
let(:file_ids_result) { ServiceResult.success(result: { "/OpenProject/" => { "fileid" => root_folder_id } }) }
let(:file_path_to_id_map_result) do
ServiceResult.success(result: { "/OpenProject" => StorageFileId.new(root_folder_id) })
end

before { ProjectStorage.where.not(id: public_storage.id).delete_all }

Expand Down Expand Up @@ -260,8 +273,10 @@ module Storages
let(:error_prefix) { "services.errors.models.nextcloud_sync_service" }

context "when the initial fetch of remote folders fails" do
let(:file_ids_result) do
errors = storage_error(:unauthorized, "error body", Peripherals::StorageInteraction::Nextcloud::FileIdsQuery)
let(:file_path_to_id_map_result) do
errors = storage_error(:unauthorized,
"error body",
Peripherals::StorageInteraction::Nextcloud::FilePathToIdMapQuery)
ServiceResult.failure(result: :unauthorized, errors:)
end

Expand Down Expand Up @@ -350,7 +365,8 @@ module Storages
end

it "interrupts the flow" do
commands = [file_ids, set_permissions, group_users, add_user, create_folder, remove_user, rename_file]
commands = [file_path_to_id_map, set_permissions, group_users, add_user, create_folder, remove_user,
rename_file]
service.call(storage)
expect(commands).to all(have_received(:call).at_least(:once))
end
Expand All @@ -367,10 +383,12 @@ def storage_error(code, data, source)

def build_create_folder_result
{
public_storage.managed_project_folder_name => ServiceResult.success(result:
StorageFile.new(id: "public_id", name: public_storage.managed_project_folder_name)),
project_storage.managed_project_folder_name => ServiceResult.success(result:
StorageFile.new(id: "normal_project_id", name: project_storage.managed_project_folder_name))
public_storage.managed_project_folder_name =>
ServiceResult.success(result: StorageFile.new(id: "public_id",
name: public_storage.managed_project_folder_name)),
project_storage.managed_project_folder_name =>
ServiceResult.success(result: StorageFile.new(id: "normal_project_id",
name: project_storage.managed_project_folder_name))
}
end

Expand Down

0 comments on commit 92277f6

Please sign in to comment.