From 4d62fd27f61867efc8f5858a6f4f0aeaf287a1fa Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 2 Dec 2024 16:39:28 +0100 Subject: [PATCH 1/2] [#59346] add log warning to unexpected content - https://community.openproject.org/wp/59346 - added tagged logging to nextcloud and onedrive validator - log file name, file id and location as a warning --- .../nextcloud_connection_validator.rb | 48 ++++++++++--------- .../one_drive_connection_validator.rb | 38 +++++++++------ .../app/common/storages/tagged_logging.rb | 2 +- 3 files changed, 50 insertions(+), 38 deletions(-) diff --git a/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb b/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb index 371d24b18ccf..e66a7f32753e 100644 --- a/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb +++ b/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb @@ -31,6 +31,7 @@ module Storages module Peripherals class NextcloudConnectionValidator + include TaggedLogging include Dry::Monads[:maybe] using ServiceResultRefinements @@ -40,13 +41,15 @@ def initialize(storage:) end def validate - maybe_is_not_configured - .or { has_base_configuration_error? } - .or { has_ampf_configuration_error? } - .value_or(ConnectionValidation.new(type: :healthy, - error_code: :none, - timestamp: Time.current, - description: nil)) + with_tagged_logger do + maybe_is_not_configured + .or { has_base_configuration_error? } + .or { has_ampf_configuration_error? } + .value_or(ConnectionValidation.new(type: :healthy, + error_code: :none, + timestamp: Time.current, + description: nil)) + end end private @@ -55,7 +58,7 @@ def has_base_configuration_error? host_url_not_found .or { missing_dependencies } .or { version_mismatch } - .or { with_unexpected_content } + # .or { with_unexpected_content } .or { capabilities_request_failed_with_unknown_error } end @@ -194,6 +197,11 @@ def with_unexpected_content unexpected_files = files.result.files.reject { |file| expected_folder_ids.include?(file.id) } return None() if unexpected_files.empty? + file_representation = unexpected_files.map do |file| + "Name: #{file.name}, ID: #{file.id}, Location: #{file.location}" + end + warn "Unexpected files/folder found in group folder:\n\t#{file_representation.join("\n\t")}" + Some( ConnectionValidation.new( type: :warning, @@ -209,13 +217,11 @@ def with_unexpected_content def capabilities_request_failed_with_unknown_error return None() if capabilities.success? - Rails.logger.error( - "Connection validation failed with unknown error:\n\t" \ - "storage: ##{@storage.id} #{@storage.name}\n\t" \ - "request: Nextcloud capabilities\n\t" \ - "status: #{capabilities.result}\n\t" \ - "response: #{capabilities.error_payload}" - ) + error "Connection validation failed with unknown error:\n\t" \ + "storage: ##{@storage.id} #{@storage.name}\n\t" \ + "request: Nextcloud capabilities\n\t" \ + "status: #{capabilities.result}\n\t" \ + "response: #{capabilities.error_payload}" Some(ConnectionValidation.new(type: :error, error_code: :err_unknown, @@ -226,13 +232,11 @@ def capabilities_request_failed_with_unknown_error def files_request_failed_with_unknown_error return None() if files.success? - Rails.logger.error( - "Connection validation failed with unknown error:\n\t" \ - "storage: ##{@storage.id} #{@storage.name}\n\t" \ - "request: Group folder content\n\t" \ - "status: #{files.result}\n\t" \ - "response: #{files.error_payload}" - ) + error "Connection validation failed with unknown error:\n\t" \ + "storage: ##{@storage.id} #{@storage.name}\n\t" \ + "request: Group folder content\n\t" \ + "status: #{files.result}\n\t" \ + "response: #{files.error_payload}" Some(ConnectionValidation.new(type: :error, error_code: :err_unknown, diff --git a/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb b/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb index 911f8280f51c..5c62084e37d7 100644 --- a/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb +++ b/modules/storages/app/common/storages/peripherals/one_drive_connection_validator.rb @@ -31,6 +31,7 @@ module Storages module Peripherals class OneDriveConnectionValidator + include TaggedLogging include Dry::Monads[:maybe] using ServiceResultRefinements @@ -40,17 +41,19 @@ def initialize(storage:) end def validate - maybe_is_not_configured - .or { tenant_id_wrong } - .or { client_id_wrong } - .or { client_secret_wrong } - .or { drive_id_wrong } - .or { request_failed_with_unknown_error } - .or { drive_with_unexpected_content } - .value_or(ConnectionValidation.new(type: :healthy, - error_code: :none, - timestamp: Time.current, - description: nil)) + with_tagged_logger do + maybe_is_not_configured + .or { tenant_id_wrong } + .or { client_id_wrong } + .or { client_secret_wrong } + .or { drive_id_wrong } + .or { request_failed_with_unknown_error } + .or { drive_with_unexpected_content } + .value_or(ConnectionValidation.new(type: :healthy, + error_code: :none, + timestamp: Time.current, + description: nil)) + end end private @@ -146,6 +149,11 @@ def drive_with_unexpected_content unexpected_files = query.result.files.reject { |file| expected_folder_ids.include?(file.id) } return None() if unexpected_files.empty? + file_representation = unexpected_files.map do |file| + "Name: #{file.name}, ID: #{file.id}, Location: #{file.location}" + end + warn "Unexpected files/folder found in group folder:\n\t#{file_representation.join("\n\t")}" + Some(ConnectionValidation.new(type: :warning, error_code: :wrn_unexpected_content, timestamp: Time.current, @@ -157,10 +165,10 @@ def drive_with_unexpected_content def request_failed_with_unknown_error return None() if query.success? - Rails.logger.error("Connection validation failed with unknown error:\n\t" \ - "storage: ##{@storage.id} #{@storage.name}\n\t" \ - "status: #{query.result}\n\t" \ - "response: #{query.error_payload}") + error "Connection validation failed with unknown error:\n\t" \ + "storage: ##{@storage.id} #{@storage.name}\n\t" \ + "status: #{query.result}\n\t" \ + "response: #{query.error_payload}" Some(ConnectionValidation.new(type: :error, error_code: :err_unknown, diff --git a/modules/storages/app/common/storages/tagged_logging.rb b/modules/storages/app/common/storages/tagged_logging.rb index 28d18c4884fd..2a5e353435e9 100644 --- a/modules/storages/app/common/storages/tagged_logging.rb +++ b/modules/storages/app/common/storages/tagged_logging.rb @@ -30,7 +30,7 @@ module Storages module TaggedLogging - delegate :info, :error, to: :logger + delegate :info, :warn, :error, to: :logger # @param tag [Class, String, Array] the tag or list of tags to annotate the logs with # @yield [Logger] From a31ec450a6f56b7550612d19756ee01d3a1e4a84 Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Mon, 2 Dec 2024 16:53:56 +0100 Subject: [PATCH 2/2] [#59346] reenable validator check --- .../storages/peripherals/nextcloud_connection_validator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb b/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb index e66a7f32753e..399f871d21a3 100644 --- a/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb +++ b/modules/storages/app/common/storages/peripherals/nextcloud_connection_validator.rb @@ -58,7 +58,7 @@ def has_base_configuration_error? host_url_not_found .or { missing_dependencies } .or { version_mismatch } - # .or { with_unexpected_content } + .or { with_unexpected_content } .or { capabilities_request_failed_with_unknown_error } end