diff --git a/app/models/health_report.rb b/app/models/health_report.rb new file mode 100644 index 00000000000..d5a7f816e84 --- /dev/null +++ b/app/models/health_report.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class HealthReport < ApplicationRecord + belongs_to :subject, polymorphic: true + + serialize :results, coder: HealthReport::ResultGroup + + def healthy? = results.all?(&:success?) + + def unhealthy? = results.any?(&:failure?) + + def warning? = results.any?(&:warning?) + + def group(key) + results.find { |group| group.key == key } + end + + def tally + results.reduce({}) do |tally, group| + tally.merge(group.tally) { |_, v1, v2| v1 + v2 } + end + end +end diff --git a/app/models/health_report/result.rb b/app/models/health_report/result.rb new file mode 100644 index 00000000000..9a94523cbb2 --- /dev/null +++ b/app/models/health_report/result.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class HealthReport + class Result + class << self + def skipped(key) + new(key:, state: :skipped, code: nil, context: nil) + end + + def success(key) + new(key:, state: :success, code: nil, context: nil) + end + + def failure(key, code, context) + new(key:, state: :failure, code:, context:) + end + + def warning(key, code, context) + new(key:, state: :warning, code:, context:) + end + + # Used for deserialization + def load(parsed_json) + new( + key: parsed_json.fetch("key"), + state: parsed_json.fetch("state"), + code: parsed_json.fetch("code"), + context: parsed_json.fetch("context") + ) + end + end + + attr_reader :key, :state, :code, :context + + def initialize(key:, state:, code:, context:) + @key = key + @state = state.to_sym + @code = code + @context = context + end + + def success? = state == :success + + def failure? = state == :failure + + def warning? = state == :warning + + def skipped? = state == :skipped + + def to_h + { key:, state:, code:, context: } + end + end +end diff --git a/modules/storages/app/common/storages/adapters/connection_validators/check_result.rb b/app/models/health_report/result_group.rb similarity index 50% rename from modules/storages/app/common/storages/adapters/connection_validators/check_result.rb rename to app/models/health_report/result_group.rb index e375efcb1e3..ebfca71aa97 100644 --- a/modules/storages/app/common/storages/adapters/connection_validators/check_result.rb +++ b/app/models/health_report/result_group.rb @@ -23,53 +23,60 @@ # # 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. +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. # # See COPYRIGHT and LICENSE files for more details. #++ -module Storages - module Adapters - module ConnectionValidators - CheckResult = Data.define(:key, :state, :code, :timestamp, :context) do - private_class_method :new +class HealthReport + class ResultGroup + class << self + # Used for serialization in health report + # Note: Because we deserialize from jsonb, we don't expect a string + # but already parsed json + def load(parsed_json) + Array(parsed_json).map { |h| new(key: h.fetch("key"), results: h.fetch("results").map { |r| Result.load(r) }) } + end - def self.skipped(key) - new(key:, state: :skipped, code: nil, timestamp: nil, context: nil) - end - - def self.failure(key, code, context) - new(key:, state: :failure, code:, timestamp: Time.zone.now, context:) - end - - def self.success(key) - new(key:, state: :success, code: nil, timestamp: Time.zone.now, context: nil) - end - - def self.warning(key, code, context) - new(key:, state: :warning, code:, timestamp: Time.zone.now, context:) - end - - def success? = state == :success - - def failure? = state == :failure - - def warning? = state == :warning - - def skipped? = state == :skipped - - def humanize_title(group) = I18n.t("storages.health.checks.#{group}.#{key}") - - def humanize_error_message - return nil if code.nil? - - I18n.t("storages.health.connection_validation.#{code}", **context) - end - - def to_h - { state: state.to_s, code:, context:, timestamp: timestamp&.iso8601 } + # Used for serialization in health report + # Note: Because we serialize into jsonb, we don't return a string (JSON.dump) + # but return a hash/array directly. + def dump(value) + if value.is_a?(Array) + value.map(&:to_h) + else + value.to_h end end end + + attr_reader :key, :results + + def initialize(key:, results: []) + @key = key + @results = results + end + + def success? = results.all?(&:success?) + + def non_failure? = results.none?(&:failure?) + + def failure? = results.any?(&:failure?) + + def warning? = results.any?(&:warning?) + + def result_for(key) + results.find { |r| r.key == key } + end + + alias [] result_for + + def tally + results.map(&:state).tally + end + + def to_h + { key:, results: results.map(&:to_h) } + end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index a609ac516cf..2b891390ae9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3547,6 +3547,16 @@ en: gui_validation_error: "1 error" gui_validation_error_plural: "%{count} errors" + health_report: + checks: + failures: + one: "%{count} check failed" + other: "%{count} checks failed" + success: All checks passed + warnings: + one: "%{count} check returned a warning" + other: "%{count} checks returned a warning" + homescreen: additional: projects: "Newest visible projects in this instance." diff --git a/db/migrate/20260428133700_create_health_reports.rb b/db/migrate/20260428133700_create_health_reports.rb new file mode 100644 index 00000000000..d0f11358640 --- /dev/null +++ b/db/migrate/20260428133700_create_health_reports.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class CreateHealthReports < ActiveRecord::Migration[8.1] + def change + create_table :health_reports do |t| + t.belongs_to :subject, null: false, polymorphic: true, index: true + t.jsonb :results, null: false, default: {} + t.timestamps null: false + end + end +end diff --git a/modules/storages/app/common/storages/adapters/connection_validators/base_connection_validator.rb b/modules/storages/app/common/storages/adapters/connection_validators/base_connection_validator.rb index bd2030a1d05..b26a9f4004d 100644 --- a/modules/storages/app/common/storages/adapters/connection_validators/base_connection_validator.rb +++ b/modules/storages/app/common/storages/adapters/connection_validators/base_connection_validator.rb @@ -34,11 +34,11 @@ module Storages class BaseConnectionValidator class << self def validation_groups - @validation_groups ||= {} + @validation_groups ||= [] end def register_group(klass, precondition: ->(*) { true }) - validation_groups[klass.key] = { klass:, precondition: } + validation_groups << { klass:, precondition: } end end @@ -47,15 +47,14 @@ module Storages end def call - validation_groups.each_with_object(ValidatorResult.new) do |(key, group_metadata), result| - if group_metadata[:precondition].call(@storage, result) - result.add_group_result(key, group_metadata[:klass].call(@storage)) + health_report = @storage.health_reports.build + validation_groups.each do |group_metadata| + if group_metadata[:precondition].call(@storage, health_report) + health_report.results << group_metadata[:klass].call(@storage) end end - end - def report_cache_key - "#{@storage}_storage_#{@storage.id}_health_status_report" + health_report end private diff --git a/modules/storages/app/common/storages/adapters/connection_validators/base_validator_group.rb b/modules/storages/app/common/storages/adapters/connection_validators/base_validator_group.rb index e56f1a467c7..1cf350dd89a 100644 --- a/modules/storages/app/common/storages/adapters/connection_validators/base_validator_group.rb +++ b/modules/storages/app/common/storages/adapters/connection_validators/base_validator_group.rb @@ -42,7 +42,8 @@ module Storages def initialize(storage) @storage = storage - @results = ValidationGroupResult.new(self.class.key) + @group = HealthReport::ResultGroup.new(key: self.class.key) + @pending_checks = [] end def call @@ -50,7 +51,9 @@ module Storages validate end - @results + @pending_checks.each { @group.results << HealthReport::Result.skipped(it) } + + @group end private @@ -58,24 +61,25 @@ module Storages def validate = raise SubclassResponsibilityError def register_checks(*keys) - keys.each { @results.register_check(it) } + @pending_checks.concat(keys) end - def update_result(...) - @results.update_result(...) + def add_result(key, result) + @group.results << result + @pending_checks.delete(key) end def pass_check(key) - update_result(key, CheckResult.success(key)) + add_result(key, HealthReport::Result.success(key)) end def fail_check(key, code, context: nil) - update_result(key, CheckResult.failure(key, code, context)) + add_result(key, HealthReport::Result.failure(key, code, context)) throw :interrupted end def warn_check(key, code, context: nil, halt_validation: false) - update_result(key, CheckResult.warning(key, code, context)) + add_result(key, HealthReport::Result.warning(key, code, context)) throw :interrupted if halt_validation end end diff --git a/modules/storages/app/common/storages/adapters/connection_validators/validation_group_result.rb b/modules/storages/app/common/storages/adapters/connection_validators/validation_group_result.rb deleted file mode 100644 index 06aadf9fd91..00000000000 --- a/modules/storages/app/common/storages/adapters/connection_validators/validation_group_result.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) 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 Storages - module Adapters - module ConnectionValidators - class ValidationGroupResult - delegate :[], :each_pair, to: :@results - - attr_reader :key - - def initialize(key) - @key = key - @results = {} - end - - def success? = @results.values.all?(&:success?) - - def non_failure? = @results.values.none?(&:failure?) - - def failure? = @results.values.any?(&:failure?) - - def warning? = @results.values.any?(&:warning?) - - def tally - @results.values.map(&:state).tally - end - - def register_checks(keys) - Array(keys).each { register_check(it) } - end - - def register_check(key) - warn("Overriding already defined check") if @results.key?(key) - - @results[key] = CheckResult.skipped(key) - end - - def update_result(key, value) - raise(ArgumentError, "Check #{key} not registered.") unless @results.key?(key) - - @results[key] = value - end - - def timestamp - @results.values.filter_map(&:timestamp).max - end - - def humanize_title = I18n.t("storages.health.checks.#{key}.header") - - def humanize_summary - case tally - in { failure: 1.. } - I18n.t("storages.health.checks.failures", count: tally[:failure]) - in { warning: 1.. } - I18n.t("storages.health.checks.warnings", count: tally[:warning]) - else - I18n.t("storages.health.checks.success") - end - end - - def to_h - @results.transform_values(&:to_h) - end - end - end - end -end diff --git a/modules/storages/app/common/storages/adapters/connection_validators/validator_result.rb b/modules/storages/app/common/storages/adapters/connection_validators/validator_result.rb deleted file mode 100644 index 526ebb0ebbb..00000000000 --- a/modules/storages/app/common/storages/adapters/connection_validators/validator_result.rb +++ /dev/null @@ -1,86 +0,0 @@ -# frozen_string_literal: true - -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) 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 Storages - module Adapters - module ConnectionValidators - class ValidatorResult - private attr_reader :group_results - - delegate :each_pair, :empty?, to: :group_results - - def initialize - @group_results = {} - end - - def healthy? = group_results.values.all?(&:success?) - - def unhealthy? = group_results.values.any?(&:failure?) - - def warning? = group_results.values.any?(&:warning?) - - def group(key) = group_results.fetch(key) - - alias_method :fetch, :group - - def add_group_result(key, result) - Kernel.warn "Overwriting #{key} results" if group_results.key?(key) - - group_results[key] = result - end - - def tally - group_results.reduce({}) do |tally, (_, group)| - tally.merge(group.tally) { |_, v1, v2| v1 + v2 } - end - end - - def latest_timestamp - group_results.values.filter_map(&:timestamp).max - end - - def humanize_summary - case tally - in { failure: 1.. } - I18n.t("storages.health.checks.failures", count: tally[:failure]) - in { warning: 1.. } - I18n.t("storages.health.checks.warnings", count: tally[:warning]) - else - I18n.t("storages.health.checks.success") - end - end - - def to_h - group_results.transform_values(&:to_h) - end - end - end - end -end diff --git a/modules/storages/app/components/storages/admin/health/check_result_component.html.erb b/modules/storages/app/components/storages/admin/health/check_result_component.html.erb index 16ae000b0a8..b6c39d345b4 100644 --- a/modules/storages/app/components/storages/admin/health/check_result_component.html.erb +++ b/modules/storages/app/components/storages/admin/health/check_result_component.html.erb @@ -31,23 +31,23 @@ See COPYRIGHT and LICENSE files for more details. flex_layout do |cell| cell.with_row do flex_layout(justify_content: :space_between, classes: "flex-wrap") do |row| - row.with_column(flex_layout: true, classes: "flex-wrap") do |text| - text.with_column(mr: 2) do - render(Primer::Beta::Text.new(font_weight: :bold)) { data[:text] } + row.with_column(flex_layout: true, classes: "flex-wrap") do |line| + line.with_column(mr: 2) do + render(Primer::Beta::Text.new(font_weight: :bold)) { text } end - text.with_column(mr: 2) do - render(Primer::Beta::Text.new(font_size: :small, color: data[:status_color])) { data[:status_text] } + line.with_column(mr: 2) do + render(Primer::Beta::Text.new(font_size: :small, color: status_color)) { status_text } end - if data[:error_code].present? - text.with_column do - render(Primer::Beta::Label.new(scheme: data[:status_color])) { data[:error_code] } + if error_code.present? + line.with_column do + render(Primer::Beta::Label.new(scheme: status_color)) { error_code } end end end - if data[:error_code].present? + if error_code.present? row.with_column do - helpers.static_link_to(href: data[:docs_href], + helpers.static_link_to(href: docs_href, label: I18n.t(:label_more_information), underline: true) end @@ -55,10 +55,10 @@ See COPYRIGHT and LICENSE files for more details. end end - if data[:error_text].present? + if error_text.present? cell.with_row(mt: 1) do render(Primer::Beta::Text.new(test_selector: "op-storages--health-status-check-information")) do - data[:error_text] + error_text end end end diff --git a/modules/storages/app/components/storages/admin/health/check_result_component.rb b/modules/storages/app/components/storages/admin/health/check_result_component.rb index f1134ce3521..5d7077a4841 100644 --- a/modules/storages/app/components/storages/admin/health/check_result_component.rb +++ b/modules/storages/app/components/storages/admin/health/check_result_component.rb @@ -41,17 +41,16 @@ module Storages private - def data - @data ||= { - text: model.humanize_title(@group), - status_color:, - status_text:, - error_code:, - error_text: model.humanize_error_message, - docs_href: ::OpenProject::Static::Links.url_for(:storage_docs, :health_status) - } + def text = I18n.t("storages.health.checks.#{@group}.#{model.key}") + + def error_text + return nil if model.code.nil? + + I18n.t("storages.health.connection_validation.#{model.code}", **model.context&.symbolize_keys) end + def docs_href = ::OpenProject::Static::Links.url_for(:storage_docs, :health_status) + def error_code if model.failure? "ERR_#{model.code.upcase}" diff --git a/modules/storages/app/components/storages/admin/health/health_report_component.html.erb b/modules/storages/app/components/storages/admin/health/health_report_component.html.erb index 93481d86aeb..f19f34e8245 100644 --- a/modules/storages/app/components/storages/admin/health/health_report_component.html.erb +++ b/modules/storages/app/components/storages/admin/health/health_report_component.html.erb @@ -48,7 +48,7 @@ See COPYRIGHT and LICENSE files for more details. flex_layout do |report_container| report_container.with_row do concat(render(Primer::Beta::Octicon.new(mr: 2, **summary_icon(@report.tally)))) - concat(render(Primer::Beta::Text.new(font_weight: :bold)) { @report.humanize_summary }) + concat(render(Primer::Beta::Text.new(font_weight: :bold)) { humanize_summary(@report.tally) }) end report_container.with_row(mt: 2) do @@ -63,25 +63,25 @@ See COPYRIGHT and LICENSE files for more details. end end - @report.each_pair do |_key, group_result| + @report.results.each do |result_group| report_container.with_row(mt: 3) do render(Primer::Beta::BorderBox.new(test_selector: "op-storages--health-report-group")) do |box| box.with_header do flex_layout(justify_content: :space_between, classes: "flex-wrap") do |header| header.with_column do - render(Primer::Beta::Text.new(font_weight: :bold)) { group_result.humanize_title } + render(Primer::Beta::Text.new(font_weight: :bold)) { I18n.t("storages.health.checks.#{result_group.key}.header") } end header.with_column do - concat(render(Primer::Beta::Octicon.new(mr: 2, **summary_icon(group_result.tally)))) - concat(render(Primer::Beta::Text.new) { group_result.humanize_summary }) + concat(render(Primer::Beta::Octicon.new(mr: 2, **summary_icon(result_group.tally)))) + concat(render(Primer::Beta::Text.new) { humanize_summary(result_group.tally) }) end end end - group_result.each_pair do |_key, value| + result_group.results.each do |value| box.with_row do - render(Storages::Admin::Health::CheckResultComponent.new(group: group_result.key, result: value)) + render(Storages::Admin::Health::CheckResultComponent.new(group: result_group.key, result: value)) end end end diff --git a/modules/storages/app/components/storages/admin/health/health_report_component.rb b/modules/storages/app/components/storages/admin/health/health_report_component.rb index 015df6d0563..b7e9ab84ff7 100644 --- a/modules/storages/app/components/storages/admin/health/health_report_component.rb +++ b/modules/storages/app/components/storages/admin/health/health_report_component.rb @@ -52,6 +52,17 @@ module Storages { icon: :"check-circle", color: :success } end end + + def humanize_summary(check_tally) + case check_tally + in { failure: 1.. } + I18n.t("health_report.checks.failures", count: check_tally[:failure]) + in { warning: 1.. } + I18n.t("health_report.checks.warnings", count: check_tally[:warning]) + else + I18n.t("health_report.checks.success") + end + end end end end diff --git a/modules/storages/app/components/storages/admin/side_panel/health_status_component.rb b/modules/storages/app/components/storages/admin/side_panel/health_status_component.rb index 7346d67360e..63dd0cb1795 100644 --- a/modules/storages/app/components/storages/admin/side_panel/health_status_component.rb +++ b/modules/storages/app/components/storages/admin/side_panel/health_status_component.rb @@ -39,8 +39,7 @@ module Storages private def report - validator = Adapters::Registry.resolve("#{model}.validators.connection").new(model) - Rails.cache.read(validator.report_cache_key) + model.health_reports.order(created_at: :asc).last end end end diff --git a/modules/storages/app/components/storages/admin/side_panel/validation_result_component.rb b/modules/storages/app/components/storages/admin/side_panel/validation_result_component.rb index 0861b4c26dc..291033b62d6 100644 --- a/modules/storages/app/components/storages/admin/side_panel/validation_result_component.rb +++ b/modules/storages/app/components/storages/admin/side_panel/validation_result_component.rb @@ -49,16 +49,16 @@ module Storages { icon: :alert, icon_color: :danger, - text: I18n.t("storages.health.checks.failures", count: tally[:failure]) + text: I18n.t("health_report.checks.failures", count: tally[:failure]) } in { warning: 1.. } { icon: :alert, icon_color: :attention, - text: I18n.t("storages.health.checks.warnings", count: tally[:warning]) + text: I18n.t("health_report.checks.warnings", count: tally[:warning]) } else - { icon: :"check-circle", icon_color: :success, text: I18n.t("storages.health.checks.success") } + { icon: :"check-circle", icon_color: :success, text: I18n.t("health_report.checks.success") } end end @@ -71,7 +71,7 @@ module Storages I18n.t("storages.health.summary.warning") end - "#{text} #{I18n.t('storages.health.checked', datetime: helpers.format_time(@result.latest_timestamp))}" + "#{text} #{I18n.t('storages.health.checked', datetime: helpers.format_time(@result.created_at))}" end end end diff --git a/modules/storages/app/controllers/storages/admin/health_status_controller.rb b/modules/storages/app/controllers/storages/admin/health_status_controller.rb index 47c38968e66..90f9a52f385 100644 --- a/modules/storages/app/controllers/storages/admin/health_status_controller.rb +++ b/modules/storages/app/controllers/storages/admin/health_status_controller.rb @@ -45,12 +45,12 @@ module Storages end def show - @report = Rails.cache.read(validator.report_cache_key) + @report = @storage.health_reports.order(created_at: :asc).last respond_to do |format| format.html format.text do - timestamp = (@report&.latest_timestamp || Time.zone.now).iso8601 + timestamp = (@report&.created_at || Time.zone.now).iso8601 filename = "#{@storage.name.underscore}_health_report_#{timestamp}.txt" send_data text_report(timestamp), filename:, type: "text/plain", disposition: :attachment end @@ -58,13 +58,13 @@ module Storages end def create - create_and_cache_report + create_and_store_report redirect_to admin_settings_storage_health_status_report_path(@storage), status: :see_other end def create_health_status_report - report = create_and_cache_report + report = create_and_store_report update_via_turbo_stream(component: SidePanel::ValidationResultComponent.new(storage: @storage, result: report)) respond_to_with_turbo_streams @@ -77,18 +77,18 @@ module Storages storage: @storage.name, storage_type: @storage.to_s, configuration: @storage.non_confidential_configuration, - ran_at: timestamp - }.merge(@report.to_h).to_yaml(stringify_names: true) + ran_at: timestamp, + results: @report ? @report.results.map(&:to_h) : [] + }.to_yaml(stringify_names: true) end def find_storage @storage = ::Storages::Storage.visible.find(params[:storage_id]) end - def create_and_cache_report + def create_and_store_report report = validator.call - Rails.cache.write(validator.report_cache_key, report, expires_in: 6.hours) - + report.save! report end diff --git a/modules/storages/app/models/storages/storage.rb b/modules/storages/app/models/storages/storage.rb index 5eb02f46c08..e7ecbef4efa 100644 --- a/modules/storages/app/models/storages/storage.rb +++ b/modules/storages/app/models/storages/storage.rb @@ -52,6 +52,7 @@ module Storages has_one :oauth_client, as: :integration, dependent: :destroy has_one :oauth_application, class_name: "::Doorkeeper::Application", as: :integration, dependent: :destroy has_many :remote_identities, as: :integration, dependent: :destroy + has_many :health_reports, as: :subject, dependent: :delete_all validates :host, uniqueness: { allow_nil: true } validates :name, uniqueness: { case_sensitive: false } diff --git a/modules/storages/app/views/storages/admin/health_status/show.html.erb b/modules/storages/app/views/storages/admin/health_status/show.html.erb index e1e3e1e6191..e40350ad320 100644 --- a/modules/storages/app/views/storages/admin/health_status/show.html.erb +++ b/modules/storages/app/views/storages/admin/health_status/show.html.erb @@ -37,7 +37,7 @@ See COPYRIGHT and LICENSE files for more details. header.with_title { page_title } header.with_description do if @report.present? - I18n.t("storages.health.checked", datetime: format_time(@report.latest_timestamp)) + I18n.t("storages.health.checked", datetime: format_time(@report.created_at)) else I18n.t("storages.health.no_report") end diff --git a/modules/storages/config/locales/en.yml b/modules/storages/config/locales/en.yml index eb7a546569e..02d6576168f 100644 --- a/modules/storages/config/locales/en.yml +++ b/modules/storages/config/locales/en.yml @@ -314,13 +314,6 @@ en: host_url_accessible: Host URL accessible storage_configured: Configuration complete tenant_id: Tenant ID - failures: - one: "%{count} check failed" - other: "%{count} checks failed" - success: All checks passed - warnings: - one: "%{count} check returned a warning" - other: "%{count} checks returned a warning" connection_validation: client_id_invalid: The configured OAuth 2 client id is invalid. Please check the configuration. client_secret_invalid: The configured OAuth 2 client secret is invalid. Please check the configuration. diff --git a/modules/storages/spec/common/storages/adapters/connection_validators/base_connection_validator_spec.rb b/modules/storages/spec/common/storages/adapters/connection_validators/base_connection_validator_spec.rb index 74064e03536..b8e98498440 100644 --- a/modules/storages/spec/common/storages/adapters/connection_validators/base_connection_validator_spec.rb +++ b/modules/storages/spec/common/storages/adapters/connection_validators/base_connection_validator_spec.rb @@ -47,16 +47,16 @@ module Storages after { TestValidator.reset_groups! } - it "returns a ValidationResult" do - expect(validator.call).to be_a(ValidatorResult) + it "returns a HealthReport" do + expect(validator.call).to be_a(HealthReport) end it "only runs a verification if the precondition evaluates as truthy" do test_group = class_spy(Providers::Nextcloud::Validators::StorageConfigurationValidator) TestValidator.register_group test_group, precondition: ->(_, _) { false } - result = validator.call - expect(result).to be_empty + report = validator.call + expect(report.results).to be_empty expect(test_group).not_to have_received(:call) end @@ -69,11 +69,11 @@ module Storages ).non_failure? end - results = TestValidator.new(create(:nextcloud_storage_with_local_connection)).call + report = TestValidator.new(create(:nextcloud_storage_with_local_connection)).call - expect(results).to be_warning - expect(results.group(Providers::Nextcloud::Validators::StorageConfigurationValidator.key)).to be_success - expect(results.group(Providers::Nextcloud::Validators::AuthenticationValidator.key)).to be_warning + expect(report).to be_warning + expect(report.group(Providers::Nextcloud::Validators::StorageConfigurationValidator.key)).to be_success + expect(report.group(Providers::Nextcloud::Validators::AuthenticationValidator.key)).to be_warning end end end diff --git a/modules/storages/spec/common/storages/adapters/providers/nextcloud/validators/storage_configuration_validator_spec.rb b/modules/storages/spec/common/storages/adapters/providers/nextcloud/validators/storage_configuration_validator_spec.rb index 10975ec5bd2..c39ed18696c 100644 --- a/modules/storages/spec/common/storages/adapters/providers/nextcloud/validators/storage_configuration_validator_spec.rb +++ b/modules/storages/spec/common/storages/adapters/providers/nextcloud/validators/storage_configuration_validator_spec.rb @@ -41,10 +41,10 @@ module Storages subject(:validator) { described_class.new(storage) } - it "returns a GroupValidationResult", vcr: "nextcloud/capabilities_success" do + it "returns a ResultGroup", vcr: "nextcloud/capabilities_success" do results = validator.call - expect(results).to be_a(ConnectionValidators::ValidationGroupResult) + expect(results).to be_a(HealthReport::ResultGroup) expect(results).to be_success end diff --git a/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/ampf_configuration_validator_spec.rb b/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/ampf_configuration_validator_spec.rb index 9036e78a17c..7569d1e3d3c 100644 --- a/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/ampf_configuration_validator_spec.rb +++ b/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/ampf_configuration_validator_spec.rb @@ -43,10 +43,10 @@ module Storages subject(:validator) { described_class.new(storage) } - it "returns a GroupValidationResult", vcr: "one_drive/validator_ampf_clean_run" do + it "returns a ResultGroup", vcr: "one_drive/validator_ampf_clean_run" do results = validator.call - expect(results).to be_a(ConnectionValidators::ValidationGroupResult) + expect(results).to be_a(HealthReport::ResultGroup) expect(results).to be_success end diff --git a/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/storage_configuration_validator_spec.rb b/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/storage_configuration_validator_spec.rb index ab3b0478e9e..16fe975bb47 100644 --- a/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/storage_configuration_validator_spec.rb +++ b/modules/storages/spec/common/storages/adapters/providers/one_drive/validators/storage_configuration_validator_spec.rb @@ -43,10 +43,10 @@ module Storages subject(:validator) { described_class.new(storage) } - it "returns a GroupValidationResult", vcr: "one_drive/files_query_userless" do + it "returns a ResultGroup", vcr: "one_drive/files_query_userless" do results = validator.call - expect(results).to be_a(ConnectionValidators::ValidationGroupResult) + expect(results).to be_a(HealthReport::ResultGroup) expect(results).to be_success end diff --git a/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/ampf_configuration_validator_spec.rb b/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/ampf_configuration_validator_spec.rb index 63719717e14..9067f370c93 100644 --- a/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/ampf_configuration_validator_spec.rb +++ b/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/ampf_configuration_validator_spec.rb @@ -47,10 +47,10 @@ module Storages subject(:validator) { described_class.new(storage) } - it "returns a GroupValidationResult", vcr: "sharepoint/validator_ampf_clean_run" do + it "returns a ResultGroup", vcr: "sharepoint/validator_ampf_clean_run" do results = validator.call - expect(results).to be_a(ConnectionValidators::ValidationGroupResult) + expect(results).to be_a(HealthReport::ResultGroup) expect(results).to be_success end diff --git a/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/storage_configuration_validator_spec.rb b/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/storage_configuration_validator_spec.rb index 11f13f30703..1af849738e2 100644 --- a/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/storage_configuration_validator_spec.rb +++ b/modules/storages/spec/common/storages/adapters/providers/sharepoint/validators/storage_configuration_validator_spec.rb @@ -43,10 +43,10 @@ module Storages subject(:validator) { described_class.new(storage) } describe "success", vcr: "sharepoint/files_query_userless" do - it "returns a GroupValidationResult" do + it "returns a ResultGroup" do results = validator.call - expect(results).to be_a(ConnectionValidators::ValidationGroupResult) + expect(results).to be_a(HealthReport::ResultGroup) expect(results).to be_success end end diff --git a/modules/storages/spec/components/storages/admin/health/check_result_component_spec.rb b/modules/storages/spec/components/storages/admin/health/check_result_component_spec.rb index d473b8da305..b34d239839e 100644 --- a/modules/storages/spec/components/storages/admin/health/check_result_component_spec.rb +++ b/modules/storages/spec/components/storages/admin/health/check_result_component_spec.rb @@ -40,7 +40,7 @@ RSpec.describe Storages::Admin::Health::CheckResultComponent, type: :component d end context "if check result is successful" do - let(:check_result) { Storages::Adapters::ConnectionValidators::CheckResult.success(:capabilities_request) } + let(:check_result) { HealthReport::Result.success(:capabilities_request) } it "renders the component" do expect(page).to have_text(I18n.t("storages.health.checks.#{group_key}.#{check_result.key}")) @@ -52,7 +52,7 @@ RSpec.describe Storages::Admin::Health::CheckResultComponent, type: :component d end context "if check result is skipped" do - let(:check_result) { Storages::Adapters::ConnectionValidators::CheckResult.skipped(:capabilities_request) } + let(:check_result) { HealthReport::Result.skipped(:capabilities_request) } it "renders the component" do expect(page).to have_text(I18n.t("storages.health.checks.#{group_key}.#{check_result.key}")) @@ -66,7 +66,7 @@ RSpec.describe Storages::Admin::Health::CheckResultComponent, type: :component d context "if check result is a warning" do let(:group_key) { :ampf_configuration } let(:check_result) do - Storages::Adapters::ConnectionValidators::CheckResult.warning(:drive_contents, :od_unexpected_content, nil) + HealthReport::Result.warning(:drive_contents, :od_unexpected_content, nil) end it "renders the component" do @@ -80,7 +80,7 @@ RSpec.describe Storages::Admin::Health::CheckResultComponent, type: :component d context "if check result is a failure" do let(:check_result) do - Storages::Adapters::ConnectionValidators::CheckResult.failure(:capabilities_request, :unknown_error, nil) + HealthReport::Result.failure(:capabilities_request, :unknown_error, nil) end it "renders the component" do diff --git a/modules/storages/spec/components/storages/admin/health/health_report_component_spec.rb b/modules/storages/spec/components/storages/admin/health/health_report_component_spec.rb index e66a8e4b099..dde82acd44f 100644 --- a/modules/storages/spec/components/storages/admin/health/health_report_component_spec.rb +++ b/modules/storages/spec/components/storages/admin/health/health_report_component_spec.rb @@ -87,23 +87,22 @@ RSpec.describe Storages::Admin::Health::HealthReportComponent, type: :component private def generate_test_group(group_key, checks) - group = Storages::Adapters::ConnectionValidators::ValidationGroupResult.new(group_key) + group = HealthReport::ResultGroup.new(key: group_key) checks.each_with_index do |check, idx| key = :"check_#{idx + 1}" result = case check when :success - Storages::Adapters::ConnectionValidators::CheckResult.success(key) + HealthReport::Result.success(key) when :warning - Storages::Adapters::ConnectionValidators::CheckResult.warning(key, :"#{key}_warning", nil) + HealthReport::Result.warning(key, :"#{key}_warning", nil) when :failure - Storages::Adapters::ConnectionValidators::CheckResult.failure(key, :"#{key}_failure", nil) + HealthReport::Result.failure(key, :"#{key}_failure", nil) else - Storages::Adapters::ConnectionValidators::CheckResult.skipped(key) + HealthReport::Result.skipped(key) end - group.register_check(key) - group.update_result(key, result) + group.results << result allow(I18n).to receive(:t).with("storages.health.checks.#{group_key}.#{key}").and_return(key.to_s.humanize) if result.code.present? allow(I18n).to receive(:t).with("storages.health.connection_validation.#{result.code}") @@ -116,11 +115,10 @@ RSpec.describe Storages::Admin::Health::HealthReportComponent, type: :component def generate_test_report(map) allow(I18n).to receive(:t).and_call_original - report = Storages::Adapters::ConnectionValidators::ValidatorResult.new + report = HealthReport.new map.each_pair do |key, values| - result = generate_test_group(key, values) - report.add_group_result(key, result) + report.results << generate_test_group(key, values) allow(I18n).to receive(:t).with("storages.health.checks.#{key}.header").and_return(key.to_s.humanize) end diff --git a/modules/storages/spec/controllers/storages/admin/health_status_controller_spec.rb b/modules/storages/spec/controllers/storages/admin/health_status_controller_spec.rb index 0179578ea8d..69efd9c15c9 100644 --- a/modules/storages/spec/controllers/storages/admin/health_status_controller_spec.rb +++ b/modules/storages/spec/controllers/storages/admin/health_status_controller_spec.rb @@ -32,7 +32,7 @@ require "spec_helper" RSpec.describe Storages::Admin::HealthStatusController do let(:user) { build_stubbed(:admin) } - let(:storage) { build_stubbed(:nextcloud_storage_configured) } + let(:storage) { create(:nextcloud_storage_configured) } let(:params) { { storage_id: storage.id } } before do @@ -72,8 +72,8 @@ RSpec.describe Storages::Admin::HealthStatusController do it "sends the text version of the report when requested" do # Creating an actual report result and caching it so we can test the rendering of the response validator = Storages::Adapters::Registry["nextcloud.validators.connection"].new(storage) - result = validator.call - Rails.cache.write validator.report_cache_key, result + report = validator.call + report.save! get :show, params: params.merge(format: :txt) @@ -84,36 +84,32 @@ RSpec.describe Storages::Admin::HealthStatusController do yaml = YAML.load(response.body) expect(yaml["storage"]).to eq storage.name expect(yaml["storage_type"]).to eq storage.to_s - expect(yaml.dig("base_configuration", "storage_configured", "state")).to eq("failure") + expect(yaml.dig("results", 0, "results", 0, "state")).to eq(:success) expect(yaml.dig("configuration", "host")).to eq(storage.host) end end describe "#create" do - let(:cache_key) { "my_cache_key" } - before do validator = instance_double(Storages::Adapters::Providers::Nextcloud::Validators::ConnectionValidator) - report = Storages::Adapters::ConnectionValidators::ValidatorResult.new + report = storage.health_reports.build allow(Storages::Adapters::Providers::Nextcloud::Validators::ConnectionValidator).to receive(:new).and_return(validator) - allow(validator).to receive_messages(call: report, report_cache_key: cache_key) + allow(validator).to receive_messages(call: report) end - it "creates and caches a health status report and redirects to show" do + it "creates a health report and redirects to show" do post :create, params: params expect(response.status).to redirect_to admin_settings_storage_health_status_report_path(storage) - expect(Rails.cache.read(cache_key)).to be_a(Storages::Adapters::ConnectionValidators::ValidatorResult) + expect(storage.reload.health_reports.count).to eq(1) end end describe "#create_health_status_report" do - let(:cache_key) { "my_cache_key" } - before do validator = instance_double(Storages::Adapters::Providers::Nextcloud::Validators::ConnectionValidator) - report = Storages::Adapters::ConnectionValidators::ValidatorResult.new + report = storage.health_reports.build allow(Storages::Adapters::Providers::Nextcloud::Validators::ConnectionValidator).to receive(:new).and_return(validator) - allow(validator).to receive_messages(call: report, report_cache_key: cache_key) + allow(validator).to receive_messages(call: report) end it "creates and caches a health status report and updates page via turbo stream" do