Skip to content

Commit

Permalink
[#56997] files query for nextcloud returns bad location data (#16386)
Browse files Browse the repository at this point in the history
* [#56997] files query for nextcloud returns bad location data

- https://community.openproject.org/work_packages/56997

* [#56997] extend url builder for empty paths

- add override for origin user id in storage factory

# Conflicts:
#	modules/storages/spec/factories/storage_factory.rb
  • Loading branch information
Kharonus authored Aug 9, 2024
1 parent 291ad4b commit 0284a86
Show file tree
Hide file tree
Showing 14 changed files with 709 additions and 431 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,32 @@ def initialize(storage)
end

def call(auth_strategy:, folder:)
if auth_strategy.user.nil?
error_data = StorageErrorData.new(source: self)
return Util.error(:error, "Cannot execute query without user context.", error_data)
end
validate_input_data(auth_strategy, folder).on_failure { return _1 }

origin_user = Util.origin_user_id(caller: self.class, storage: @storage, auth_strategy:)
.on_failure { return _1 }
.result

@location_prefix = UrlBuilder.path(@storage.uri.path, "remote.php/dav/files", origin_user)
@location_prefix = CGI.unescape UrlBuilder.path(@storage.uri.path, "remote.php/dav/files", origin_user)

result = make_request(auth_strategy:, folder:, origin_user:)
storage_files(result)
end

private

def validate_input_data(auth_strategy, folder)
error_data = StorageErrorData.new(source: self.class)

if auth_strategy.user.nil?
Util.error(:error, "Cannot execute query without user context.", error_data)
elsif folder.is_a?(ParentFolder)
ServiceResult.success
else
Util.error(:error, "Folder input is not a ParentFolder object.", error_data)
end
end

def make_request(auth_strategy:, folder:, origin_user:)
Authentication[auth_strategy].call(storage: @storage,
http_options: Util.webdav_request_with_depth(1)) do |http|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ def initialize(storage)
def call(auth_strategy:, folder:)
with_tagged_logger do
info "Getting data on all files under folder '#{folder}' using #{auth_strategy.key}"
validate_input_data(folder).on_failure { return _1 }

Authentication[auth_strategy].call(storage: @storage) do |http|
response = handle_response(http.get(children_url_for(folder) + FIELDS), :value)

Expand All @@ -62,6 +64,16 @@ def call(auth_strategy:, folder:)

private

def validate_input_data(folder)
if folder.is_a?(ParentFolder)
ServiceResult.success
else
data = StorageErrorData.new(source: self.class)
log_message = "Folder input is not a ParentFolder object."
ServiceResult.failure(result: :error, errors: StorageError.new(code: :error, log_message:, data:))
end
end

# rubocop:disable Metrics/AbcSize
def handle_response(response, map_value)
case response
Expand Down Expand Up @@ -106,7 +118,7 @@ def empty_storage_files(path, parent_id)
StorageFile.new(
id: parent_id,
name: path.split("/").last,
location: path,
location: UrlBuilder.path(path),
permissions: %i[readable writeable]
),
forge_ancestors(path:)
Expand All @@ -122,7 +134,7 @@ def parent(parent_reference)
StorageFile.new(
id: parent_reference[:id],
name:,
location: Util.extract_location(parent_reference),
location: UrlBuilder.path(Util.extract_location(parent_reference)),
permissions: %i[readable writeable]
)
end
Expand All @@ -137,7 +149,7 @@ def forge_ancestors(parent_reference)
StorageFile.new(
id: Digest::SHA256.hexdigest(component),
name: component,
location: "/#{component}"
location: UrlBuilder.path(component)
)
end
end
Expand Down
4 changes: 3 additions & 1 deletion modules/storages/app/helpers/storages/url_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def url(uri, *path_fragments)
end

def path(*path_fragments)
URI.join(URI("https://drop.me"), *split_and_escape(path_fragments)).path
URI.join(URI("https://drop.me/"), *split_and_escape(path_fragments)).path
end

private
Expand All @@ -58,6 +58,8 @@ def split_and_escape(fragments)
.each { |f| ensure_unescaped_fragments(f) }
.map { |f| CGI.escapeURIComponent(f) }

return [] if single_fragments.empty?

single_fragments[..-2]
.map { |f| "#{f}/" }
.push(single_fragments.last)
Expand Down
10 changes: 1 addition & 9 deletions modules/storages/app/models/storages/storage_files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,5 @@
#++

module Storages
class StorageFiles
attr_reader :files, :parent, :ancestors

def initialize(files, parent, ancestors)
@files = files.freeze
@parent = parent.freeze
@ancestors = ancestors.freeze
end
end
StorageFiles = Data.define(:files, :parent, :ancestors)
end
Loading

0 comments on commit 0284a86

Please sign in to comment.