From 222db56b762865bc9c992b6dc0aa16327b074b5d Mon Sep 17 00:00:00 2001 From: cd Date: Thu, 17 Sep 2026 09:13:05 +0200 Subject: [PATCH 01/25] fix: Bug Owner Import add: Log Level rule_owner add: log run historie --- documentation/revision-history.md | 16 + inventory/group_vars/all.yml | 2 +- .../setAllActiveRuleOwnersRemoved.graphql | 11 +- .../files/sql/idempotent/fworch-texts.sql | 120 +++- roles/lib/files/FWO.Basics/GlobalConstants.cs | 1 + .../files/FWO.Config.Api/Data/ConfigData.cs | 3 + roles/lib/files/FWO.Data/Alert.cs | 1 + .../Enums/RuleOwnerMappingLogLevel.cs | 50 ++ roles/lib/files/FWO.Data/RuleOwner.cs | 3 + .../Events/UpdateRuleOwnerMappingEventArgs.cs | 13 + .../FWO.Services/RuleOwnerMappingLogger.cs | 82 +++ .../RuleOwnerMappingRunHistory.cs | 362 ++++++++++ .../UpdateRuleOwnerMappingBase.cs | 252 ++++++- .../UpdateRuleOwnerMappingCustomField.cs | 5 +- .../UpdateRuleOwnerMappingDisabled.cs | 11 +- .../UpdateRuleOwnerMappingIpBased.cs | 31 +- .../UpdateRuleOwnerMappingNameField.cs | 32 +- .../Jobs/UpdateRuleOwnerMappingJob.cs | 3 +- .../FWO.Test/OwnerMappingSourceHandlerTest.cs | 103 +++ .../FWO.Test/RuleOwnerMappingLogLevelTest.cs | 101 +++ .../RuleOwnerMappingRunHandlerTest.cs | 295 ++++++++ .../UpdateRuleOwnerMappingIncrementalTest.cs | 679 ++++++++++++++++++ .../FWO.Test/UpdateRuleOwnerMappingTests.cs | 54 +- .../HelpMonitoringRuleOwnerMapping.cshtml | 25 + .../Pages/Help/HelpMonitoringSidebar.cshtml | 3 + .../Pages/Help/HelpSettingsOwners.cshtml | 1 + .../Monitoring/MonitorRuleOwnerMapping.razor | 258 +++++++ .../FWO.UI/Pages/Settings/OwnerMapping.razor | 6 +- .../Pages/Settings/SettingsImport.razor | 2 +- .../Services/OwnerMappingSourceHandler.cs | 84 ++- .../Services/RuleOwnerMappingRunHandler.cs | 231 ++++++ .../FWO.UI/Shared/MonitoringLayout.razor | 5 + .../Shared/OwnerMappingSourceEditor.razor | 27 + 33 files changed, 2784 insertions(+), 88 deletions(-) create mode 100644 roles/lib/files/FWO.Data/Enums/RuleOwnerMappingLogLevel.cs create mode 100644 roles/lib/files/FWO.Services/RuleOwnerMappingLogger.cs create mode 100644 roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs create mode 100644 roles/tests-unit/files/FWO.Test/RuleOwnerMappingLogLevelTest.cs create mode 100644 roles/tests-unit/files/FWO.Test/RuleOwnerMappingRunHandlerTest.cs create mode 100644 roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs create mode 100644 roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringRuleOwnerMapping.cshtml create mode 100644 roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor create mode 100644 roles/ui/files/FWO.UI/Services/RuleOwnerMappingRunHandler.cs diff --git a/documentation/revision-history.md b/documentation/revision-history.md index 9558681e54..53aba4382c 100644 --- a/documentation/revision-history.md +++ b/documentation/revision-history.md @@ -708,3 +708,19 @@ Not supported any longer are: - report rules containing objects that cannot be assigned to a compliance network zone as `NOT ASSESSABLE` instead of compliant; real violations of the same rule remain decisive and visible - new import matrix format with path_to_root and path_to_internet, while old format is still supported - validation checks for matrix import + +## 9.5.2 +- rule owner mapping: fix an owner import blocking the incremental processing permanently - rebuilding + the mappings of a newly created owner collided with the partial unique index on rule_owner because the + on_conflict clause only covers the primary key, and the failure was swallowed and reported as success +- rule owner mapping: a failing import no longer holds up the imports behind it, and failures raise an + alert instead of only a log line +- rule owner mapping: a full reinitialize that matches no rule now removes the obsolete mappings and + reports the empty result as an alert, instead of aborting and leaving the previous state in place +- rule owner mapping: new setting for the log level of mapping issues, so installations with many + unmappable legacy rules no longer get one message per rule on every run +- rule owner mapping: the result of the last full reinitialize runs is kept in the config entry + ruleOwnerMappingRunHistory and shown on the new page monitoring/rule_owner_mapping, including which + rules were affected and whether a deliberate change, a pending import backlog or a real deviation + caused the difference +- rule owner mapping: new AlertCode RuleOwnerMapping (52) for every alert of this area diff --git a/inventory/group_vars/all.yml b/inventory/group_vars/all.yml index 828035ae41..ecdd228024 100644 --- a/inventory/group_vars/all.yml +++ b/inventory/group_vars/all.yml @@ -1,5 +1,5 @@ ### general settings -product_version: "9.5.1" +product_version: "9.5.2" # The oldest product version an upgrade may start from, enforced by # roles/common/tasks/validate-upgrade-source-version.yml before anything touches the # database. It exists because 9.5.0 removed the migration files below 8.0: diff --git a/roles/common/files/fwo-api-calls/owner/setAllActiveRuleOwnersRemoved.graphql b/roles/common/files/fwo-api-calls/owner/setAllActiveRuleOwnersRemoved.graphql index b5fd4b0df5..6c5a315423 100644 --- a/roles/common/files/fwo-api-calls/owner/setAllActiveRuleOwnersRemoved.graphql +++ b/roles/common/files/fwo-api-calls/owner/setAllActiveRuleOwnersRemoved.graphql @@ -1,8 +1,17 @@ +# returning delivers the state before the full reinitialize, which is what the run history diffs +# against the rebuilt mappings - no separate query needed, the where clause already selects exactly +# the mappings that were active. mutation setAllActiveRuleOwnersRemoved($controlId: bigint!) { update_rule_owner( where: { removed: { _is_null: true } }, _set: { removed: $controlId } ) { affected_rows + returning { + rule_id + owner_id + created + rule_metadata_id + } } -} \ No newline at end of file +} diff --git a/roles/database/files/sql/idempotent/fworch-texts.sql b/roles/database/files/sql/idempotent/fworch-texts.sql index 5dfd300d2e..2c388a9cdf 100644 --- a/roles/database/files/sql/idempotent/fworch-texts.sql +++ b/roles/database/files/sql/idempotent/fworch-texts.sql @@ -345,6 +345,14 @@ INSERT INTO txt VALUES ('CustomField', 'German', 'Custom-Feld'); INSERT INTO txt VALUES ('CustomField', 'English', 'Custom field'); INSERT INTO txt VALUES ('NameField', 'German', 'Namensfeld'); INSERT INTO txt VALUES ('NameField', 'English', 'Name field'); +INSERT INTO txt VALUES ('Error', 'German', 'Fehler'); +INSERT INTO txt VALUES ('Error', 'English', 'Error'); +INSERT INTO txt VALUES ('Warning', 'German', 'Warnung'); +INSERT INTO txt VALUES ('Warning', 'English', 'Warning'); +INSERT INTO txt VALUES ('Info', 'German', 'Info'); +INSERT INTO txt VALUES ('Info', 'English', 'Info'); +INSERT INTO txt VALUES ('Debug', 'German', 'Debug'); +INSERT INTO txt VALUES ('Debug', 'English', 'Debug'); INSERT INTO txt VALUES ('Manual', 'German', 'Manuell'); INSERT INTO txt VALUES ('Manual', 'English', 'Manual'); INSERT INTO txt VALUES ('DemoData', 'German', 'Demo-Daten'); @@ -543,6 +551,10 @@ INSERT INTO txt VALUES ('whats_new_facts', 'German', '
  • Internal-Work-Regeländerungen laufen in die Genehmigungsphase statt direkt in die Planung; Email-Aktionen für Request Tasks können pro Task-Typ gebündelt werden, sodass eine Email alle passenden Request Tasks enthält.
  • Matrix Import nimmt und validiert neue Felder für Network Zone Tree, ohne sie bisher zu speichern.
  • Die automatische Eigentümerzuordnung von Regeln kann nun auf "Deaktiviert" gesetzt werden und ist nach einer Neuinstallation so voreingestellt. Damit lassen sich die übrigen Importer-Einstellungen speichern, ohne vorher eine Zuordnungsquelle einzurichten. Beim Umstellen auf "Deaktiviert" werden die bereits berechneten Zuordnungen entfernt.
    • +
  • Die Protokollierung der Eigentümerzuordnung lässt sich nun in fünf Stufen einstellen. Auf Installationen mit vielen Altregeln, die nie zugeordnet werden können, erzeugte bisher jeder Lauf eine Meldung pro Regel. Die Zusammenfassung jedes Laufs und fehlgeschlagene Importe werden unabhängig davon immer protokolliert.
    • +
  • Probleme der Eigentümerzuordnung erscheinen nun als Alarm unter Monitoring statt nur im Logfile: nicht verarbeitete Importe, eine Quelle die keine Regel mehr trifft, und Abweichungen zwischen laufender Aktualisierung und vollständiger Neuberechnung.
    • +
  • Die neue Seite Monitoring – Eigentümerzuordnung: Läufe zeigt, ob die laufende Aktualisierung denselben Stand erzeugt wie eine vollständige Neuberechnung, und listet die betroffenen Regeln samt Anlass auf.
    • +
  • Mehrere Fehler der laufenden Eigentümerzuordnung behoben: ein neu angelegter Eigentümer konnte die Verarbeitung dauerhaft blockieren, Fehlschläge wurden als Erfolg gemeldet, ein einzelner Fehler hielt alle nachfolgenden Importe auf, und eine Quelle ohne Treffer ließ veraltete Zuordnungen stehen.
  • Details: siehe Release Notes.
    • '); @@ -558,6 +570,10 @@ INSERT INTO txt VALUES ('whats_new_facts', 'English', '
  • Internal work rule changes are routed into the approval phase instead of directly into planning; request task email actions can be bundled by task type so that one email covers all matching request tasks.
  • Matrix import takes and validates new fields for Network Zone Tree, but does not store them yet.
  • The automatic owner mapping of rules can now be set to "Disabled" and a new installation starts with it. The remaining importer settings can therefore be saved without setting up a mapping source first. Switching to "Disabled" removes the mappings calculated so far.
    • +
  • Logging of the owner mapping can now be set to one of five levels. On installations with many legacy rules that can never be mapped, every run used to produce one message per rule. The summary of each run and failed imports are always logged regardless.
    • +
  • Problems of the owner mapping now appear as an alert under Monitoring instead of only in the log file: imports that could not be processed, a source that no longer matches any rule, and deviations between the running update and a full recalculation.
    • +
  • The new page Monitoring – Owner mapping runs shows whether the running update produces the same state as a full recalculation, and lists the affected rules together with what caused the run.
    • +
  • Several defects of the running owner mapping fixed: a newly created owner could block processing permanently, failures were reported as success, a single failure held up all following imports, and a source without any match left obsolete mappings in place.
  • Details: see release notes.
  • Hilfeseiten: Benutzerhandbuch
  • Einstellungen: Alle Einstellungen wie z.B. Sprache der Benutzeroberfläche oder @@ -2336,6 +2352,56 @@ INSERT INTO txt VALUES ('owner_mapping', 'German', 'Eigentümerzuor INSERT INTO txt VALUES ('owner_mapping', 'English', 'Owner Mapping'); INSERT INTO txt VALUES ('custom_field_Owner_key', 'German', 'Custom-Feld-Eigentümer-Schlüssel'); INSERT INTO txt VALUES ('custom_field_Owner_key', 'English', 'Custom Field Owner key'); +INSERT INTO txt VALUES ('ruleOwnerMappingLogLevel', 'German', 'Protokollierung Zuordnungsprobleme'); +INSERT INTO txt VALUES ('ruleOwnerMappingLogLevel', 'English', 'Logging of mapping issues'); +INSERT INTO txt VALUES ('rule_owner_mapping_runs', 'German', 'Eigentümerzuordnung: Läufe'); +INSERT INTO txt VALUES ('rule_owner_mapping_runs', 'English', 'Owner mapping runs'); +INSERT INTO txt VALUES ('rule_owner_mappings_total', 'German', 'Zuordnungen gesamt'); +INSERT INTO txt VALUES ('rule_owner_mappings_total', 'English', 'Mappings in total'); +INSERT INTO txt VALUES ('rule_owner_pending_imports','German', 'Offene Importe'); +INSERT INTO txt VALUES ('rule_owner_pending_imports','English', 'Pending imports'); +INSERT INTO txt VALUES ('rule_owner_run_InSync', 'German', 'Unverändert'); +INSERT INTO txt VALUES ('rule_owner_run_InSync', 'English', 'In sync'); +INSERT INTO txt VALUES ('rule_owner_run_Drift', 'German', 'Abweichung'); +INSERT INTO txt VALUES ('rule_owner_run_Drift', 'English', 'Deviation'); +INSERT INTO txt VALUES ('rule_owner_run_ImportsPending', 'German', 'Importe offen'); +INSERT INTO txt VALUES ('rule_owner_run_ImportsPending', 'English', 'Imports pending'); +INSERT INTO txt VALUES ('rule_owner_run_ChangeApplied', 'German', 'Änderung vorgenommen'); +INSERT INTO txt VALUES ('rule_owner_run_ChangeApplied', 'English', 'Change applied'); +INSERT INTO txt VALUES ('rule_owner_finding_Missing', 'German', 'Nicht angelegt'); +INSERT INTO txt VALUES ('rule_owner_finding_Missing', 'English', 'Not created'); +INSERT INTO txt VALUES ('rule_owner_finding_Superfluous', 'German', 'Nicht entfernt'); +INSERT INTO txt VALUES ('rule_owner_finding_Superfluous', 'English', 'Not removed'); +INSERT INTO txt VALUES ('rule_owner_change_added', 'German', 'Neu hinzugekommen'); +INSERT INTO txt VALUES ('rule_owner_change_added', 'English', 'Newly added'); +INSERT INTO txt VALUES ('rule_owner_change_removed', 'German', 'Weggefallen'); +INSERT INTO txt VALUES ('rule_owner_change_removed', 'English', 'No longer applies'); +INSERT INTO txt VALUES ('run', 'German', 'Lauf'); +INSERT INTO txt VALUES ('run', 'English', 'Run'); +INSERT INTO txt VALUES ('newer', 'German', 'Neuer'); +INSERT INTO txt VALUES ('newer', 'English', 'Newer'); +INSERT INTO txt VALUES ('older', 'German', 'Älter'); +INSERT INTO txt VALUES ('older', 'English', 'Older'); +INSERT INTO txt VALUES ('finding', 'German', 'Art der Abweichung'); +INSERT INTO txt VALUES ('finding', 'English', 'Type of deviation'); +INSERT INTO txt VALUES ('rule_metadata_id', 'German', 'Regel-Metadaten-ID'); +INSERT INTO txt VALUES ('rule_metadata_id', 'English', 'Rule metadata ID'); +INSERT INTO txt VALUES ('rule_owner_applied_changes', 'German', 'Geänderte Einstellung'); +INSERT INTO txt VALUES ('rule_owner_applied_changes', 'English', 'Changed setting'); +INSERT INTO txt VALUES ('rule_owner_last_clean_run', 'German', 'Zuletzt ohne Abweichung geprüft'); +INSERT INTO txt VALUES ('rule_owner_last_clean_run', 'English', 'Last verified without deviation'); +INSERT INTO txt VALUES ('rule_owner_current_InSync', 'German', 'Aktuell keine Abweichung'); +INSERT INTO txt VALUES ('rule_owner_current_InSync', 'English', 'Currently no deviation'); +INSERT INTO txt VALUES ('rule_owner_current_Drift', 'German', 'Die letzte Prüfung hat Abweichungen gefunden'); +INSERT INTO txt VALUES ('rule_owner_current_Drift', 'English', 'The last check found deviations'); +INSERT INTO txt VALUES ('rule_owner_current_ChangeApplied', 'German', 'Änderung angewendet, seither nicht geprüft'); +INSERT INTO txt VALUES ('rule_owner_current_ChangeApplied', 'English', 'Change applied, not verified since'); +INSERT INTO txt VALUES ('rule_owner_current_ImportsPending', 'German', 'Letzte Prüfung nicht aussagekräftig, es waren Importe offen'); +INSERT INTO txt VALUES ('rule_owner_current_ImportsPending', 'English', 'Last check inconclusive, imports were pending'); +INSERT INTO txt VALUES ('rule_owner_runs_with_findings', 'German', 'Aufgezeichnete Läufe'); +INSERT INTO txt VALUES ('rule_owner_runs_with_findings', 'English', 'Recorded runs'); +INSERT INTO txt VALUES ('removed', 'German', 'Entfernt'); +INSERT INTO txt VALUES ('removed', 'English', 'Removed'); INSERT INTO txt VALUES ('custom_field_ChangeID_key', 'German', 'Custom-Feld-Änderungs-ID-Schlüssel'); INSERT INTO txt VALUES ('custom_field_ChangeID_key', 'English', 'Custom field change-ID keys'); INSERT INTO txt VALUES ('custom_field_import_Settings', 'German', 'Custom-Feld-Einstellungen'); @@ -4151,8 +4217,8 @@ INSERT INTO txt VALUES ('U5501', 'German', 'Sind sie sicher, dass sie folgenden INSERT INTO txt VALUES ('U5501', 'English', 'Are you sure you want to delete state: '); INSERT INTO txt VALUES ('U5502', 'German', 'Sind sie sicher, dass sie folgende Aktion löschen wollen: '); INSERT INTO txt VALUES ('U5502', 'English', 'Are you sure you want to delete action: '); -INSERT INTO txt VALUES ('U5503', 'German', 'Import erfolgreich.'); -INSERT INTO txt VALUES ('U5503', 'English', 'Import successful.'); +INSERT INTO txt VALUES ('U5503', 'German', 'Eigentümerzuordnung neu berechnet.'); +INSERT INTO txt VALUES ('U5503', 'English', 'Owner mapping recalculated.'); INSERT INTO txt VALUES ('U5601', 'German', 'Sind sie sicher, dass sie das letzte Senden für folgende Benachrichtigung zurücksetzen wollen: '); INSERT INTO txt VALUES ('U5601', 'English', 'Are you sure you want to reset the last sending for following notification: '); @@ -4181,6 +4247,16 @@ INSERT INTO txt VALUES ('U7401', 'German', 'Archiv der Autodiscovery-Nachrichte INSERT INTO txt VALUES ('U7401', 'English', 'View the past autodiscovery messages'); INSERT INTO txt VALUES ('U7501', 'German', 'Archiv der Nachrichten der täglichen Checks'); INSERT INTO txt VALUES ('U7501', 'English', 'View the past daily check messages'); +INSERT INTO txt VALUES ('U7550', 'German', 'Ergebnis der letzten vollständigen Neuberechnungen der Eigentümerzuordnung. Ein vollständiger Neuaufbau berechnet alle Zuordnungen neu; weicht das Ergebnis vom gespeicherten Stand ab, obwohl kein Import offen war, hat die laufende Aktualisierung etwas übersehen. Aufgelistet werden nur Läufe mit Abweichung; wann zuletzt ohne Befund geprüft wurde, steht oben.'); +INSERT INTO txt VALUES ('U7550', 'English', 'Result of the last full recalculations of the owner mapping. A full rebuild recalculates every mapping; if the result differs from the stored state although no import was pending, the running update missed something. Only runs with a deviation are listed; when it was last verified without findings is shown above.'); +INSERT INTO txt VALUES ('U7551', 'German', 'Bisher wurde keine vollständige Neuberechnung aufgezeichnet. Sie wird unter Einstellungen - Eigentümer - Eigentümerzuordnung ausgelöst.'); +INSERT INTO txt VALUES ('U7551', 'English', 'No full recalculation has been recorded yet. It is triggered under Settings - Owners - Owner Mapping.'); +INSERT INTO txt VALUES ('U7552', 'German', 'Es wurden mehr Abweichungen gefunden, als hier aufgelistet sind. Die Anzahl oben ist vollständig, die Liste zeigt nur die ersten Einträge.'); +INSERT INTO txt VALUES ('U7552', 'English', 'More deviations were found than are listed here. The counts above are complete, the list only shows the first entries.'); +INSERT INTO txt VALUES ('U7553', 'German', 'Keine Abweichung: die laufende Aktualisierung hat denselben Stand erzeugt wie die vollständige Neuberechnung.'); +INSERT INTO txt VALUES ('U7553', 'English', 'No deviation: the running update produced the same state as the full recalculation.'); +INSERT INTO txt VALUES ('U7554', 'German', 'Bei einem Wechsel der Mapping-Quelle ändert sich jede Zuordnung. Die Einzelliste wird deshalb nicht gespeichert, die Zahlen oben sind vollständig.'); +INSERT INTO txt VALUES ('U7554', 'English', 'Switching the mapping source changes every mapping. The individual list is therefore not stored; the counts above are complete.'); INSERT INTO txt VALUES ('U8001', 'German', 'Sind sie sicher, dass sie Folgendes löschen wollen: '); INSERT INTO txt VALUES ('U8001', 'English', 'Are you sure you want to delete: '); @@ -4547,8 +4623,8 @@ INSERT INTO txt VALUES ('E5431', 'English', 'Uploaded File exceeds the allowed m INSERT INTO txt VALUES ('E5501', 'German', 'Die Regel kann nicht doppelt zugewiesen werden.'); INSERT INTO txt VALUES ('E5501', 'English', 'Rule cannot be assigned twice.'); -INSERT INTO txt VALUES ('E5502', 'German', 'Keine Regeln/Eigentümer gefunden oder falsches Schlüsselfeld.'); -INSERT INTO txt VALUES ('E5502', 'English', 'No Rules/Owner or wrong Key-Field.'); +INSERT INTO txt VALUES ('E5502', 'German', 'Die Neuberechnung der Eigentümerzuordnung ist fehlgeschlagen. Einzelheiten stehen im Middleware-Log.'); +INSERT INTO txt VALUES ('E5502', 'English', 'Recalculating the owner mapping failed. See the middleware log for details.'); INSERT INTO txt VALUES ('E5503', 'German', 'Fehler beim Import.'); INSERT INTO txt VALUES ('E5503', 'English', 'Import error.'); INSERT INTO txt VALUES ('E5504', 'German', 'Bitte eine Eigentümerzuordnungsquelle auswählen.'); @@ -7987,6 +8063,38 @@ INSERT INTO txt VALUES ('H7252', 'English', 'Sample data (defined by the ending '); INSERT INTO txt VALUES ('H7253', 'German', 'Die Ergebnisse der Prüfung des Import-Status der aktiven Managements sind hier protokolliert. Werden Anomalien wie überlange Import-Zeiten oder fehlende Imports festgestellt, werden einzelne Alarme ausgelöst, die unter Offenen Alarme analysiert und behandelt werden können. Hier wird lediglich die Anzahl der gefundenen Probleme protokolliert. +INSERT INTO txt VALUES ('H7261', 'German', 'Ein vollständiger Neuaufbau berechnet alle Zuordnungen von Grund auf und vergleicht das Ergebnis mit dem gespeicherten Stand. Der Balken oben sagt, wie die Zuordnung aktuell steht; er nimmt den zuletzt gelaufenen Neuaufbau. Darunter stehen die aufgezeichneten Läufe, zwischen denen mit Neuer und Älter gewechselt wird.'); +INSERT INTO txt VALUES ('H7261', 'English', 'A full rebuild recalculates every mapping from scratch and compares the result with the stored state. The banner at the top says how the mapping stands right now, taken from the rebuild that ran last. Below it are the recorded runs; use Newer and Older to move between them.'); +INSERT INTO txt VALUES ('H7262', 'German', 'Die vier Zustände: Unverändert – der Lauf fand keinen Unterschied, die laufende Aktualisierung ist auf dem richtigen Stand. Änderung angewendet – eine Einstellung wurde geändert, das Ergebnis weicht deshalb bewusst ab. Importe offen – es war noch etwas unverarbeitet, der Lauf kann nichts aussagen. Abweichung – Unterschied ohne offene Importe und ohne Änderung; nur dieser Zustand ist ein Problem.'); +INSERT INTO txt VALUES ('H7262', 'English', 'The four states: In sync – the run found no difference, the running update is up to date. Change applied – a setting was changed, so the result deliberately differs. Imports pending – something was still unprocessed, the run cannot judge. Deviation – a difference without pending imports and without a change; only this state is a problem.'); +INSERT INTO txt VALUES ('H7263', 'German', 'Aufgezeichnet werden Läufe mit Unterschied und Läufe, die eine geänderte Einstellung angewendet haben – letztere auch dann, wenn sie nichts bewirkt haben, denn genau das will man sehen. Ein Neuaufbau ohne Unterschied belegt keinen Platz, er frischt nur den Zeitstempel oben auf. Es werden bis zu zehn Läufe behalten.'); +INSERT INTO txt VALUES ('H7263', 'English', 'Recorded are runs with a difference and runs that applied a changed setting – the latter even when they had no effect, because that is exactly what you want to see. A rebuild without a difference takes no slot, it only refreshes the timestamp above. Up to ten runs are kept.'); +INSERT INTO txt VALUES ('H7264', 'German', 'In der Tabelle hängt die Bezeichnung vom Anlass ab. Bei einer Abweichung heißt es Nicht angelegt beziehungsweise Nicht entfernt – die laufende Aktualisierung hätte das tun müssen. Nach einer bewussten Änderung heißt dasselbe Neu hinzugekommen und Weggefallen, denn dort ist es das gewollte Ergebnis. Regel, Eigentümer und Metadaten sind als IDs angegeben; über Erstellt und Entfernt lässt sich die Zeile in der Tabelle rule_owner wiederfinden.'); +INSERT INTO txt VALUES ('H7264', 'English', 'In the table the wording depends on what caused the run. For a deviation it reads Not created or Not removed – the running update should have done that. After a deliberate change the same thing reads Newly added and No longer applies, because there it is the intended result. Rule, owner and metadata are given as IDs; Created and Removed let you find the row again in the rule_owner table.'); +INSERT INTO txt VALUES ('H7265', 'German', 'Beim Wechsel der Mapping-Quelle ändert sich jede Zuordnung. Die Einzelliste wird dann nicht gespeichert, weil sie nichts aussagen würde; die Zähler bleiben vollständig. Waren Importe offen, stehen deren Nummern dabei und der Lauf frischt den Zeitstempel oben nicht auf – er hat ja nichts nachgewiesen.'); +INSERT INTO txt VALUES ('H7265', 'English', 'Switching the mapping source changes every mapping. The individual list is then not stored because it would say nothing; the counts stay complete. If imports were pending, their numbers are listed and the run does not refresh the timestamp above – it proved nothing.'); +INSERT INTO txt VALUES ('H7276', 'German', 'Ein Alarm wird gemeldet, wenn ein Import nicht verarbeitet werden konnte, wenn wegen zu vieler offener Importe auf einen vollständigen Neuaufbau ausgewichen wurde, wenn die Mapping-Quelle überhaupt keine Regel mehr trifft, und bei einer echten Abweichung. Eine bewusste Änderung und offene Importe lösen keinen Alarm aus. Alarme stehen unter Monitoring – Alarme.'); +INSERT INTO txt VALUES ('H7276', 'English', 'An alert is raised when an import could not be processed, when too many pending imports forced a full rebuild, when the mapping source no longer matches any rule at all, and on a real deviation. A deliberate change and pending imports raise no alert. Alerts are listed under Monitoring – Alerts.'); +INSERT INTO txt VALUES ('H7266', 'German', 'Wie das Ergebnis dieses Laufs zu lesen ist. Nur Abweichung weist auf ein Problem hin; die anderen Zustände erklären den Unterschied von selbst.'); +INSERT INTO txt VALUES ('H7266', 'English', 'How the result of this run has to be read. Only Deviation points at a problem; the other states explain the difference themselves.'); +INSERT INTO txt VALUES ('H7267', 'German', 'Zeitpunkt, zu dem die vollständige Neuberechnung abgeschlossen und ihr Ergebnis festgehalten wurde.'); +INSERT INTO txt VALUES ('H7267', 'English', 'Point in time at which the full recalculation finished and its result was recorded.'); +INSERT INTO txt VALUES ('H7268', 'German', 'Nummer des Imports, unter dem die Neuberechnung verbucht wurde. Alle unten aufgeführten Zeilen sind in der Tabelle rule_owner über diese Nummer auffindbar.'); +INSERT INTO txt VALUES ('H7268', 'English', 'Number of the import the recalculation was recorded under. Every row listed below can be found in the rule_owner table by this number.'); +INSERT INTO txt VALUES ('H7269', 'German', 'Verfahren, nach dem die Zuordnungen berechnet wurden, also IP-basiert, Custom-Feld oder Namensfeld.'); +INSERT INTO txt VALUES ('H7269', 'English', 'Method the mappings were calculated with, that is IP based, custom field or name field.'); +INSERT INTO txt VALUES ('H7270', 'German', 'Anzahl der Zuordnungen nach diesem Lauf.'); +INSERT INTO txt VALUES ('H7270', 'English', 'Number of mappings after this run.'); +INSERT INTO txt VALUES ('H7271', 'German', 'Zuordnungen, die dieser Lauf angelegt hat, weil es sie vorher nicht gab. Die laufende Aktualisierung hätte sie erzeugen müssen.'); +INSERT INTO txt VALUES ('H7271', 'English', 'Mappings this run created because they did not exist before. The running update should have created them.'); +INSERT INTO txt VALUES ('H7272', 'German', 'Zuordnungen, die dieser Lauf entfernt hat, weil sie nicht mehr zutreffen. Die laufende Aktualisierung hätte sie abräumen müssen.'); +INSERT INTO txt VALUES ('H7272', 'English', 'Mappings this run removed because they no longer apply. The running update should have cleared them.'); +INSERT INTO txt VALUES ('H7273', 'German', 'Importe, die zu Beginn des Laufs noch nicht verarbeitet waren. Sind hier Nummern eingetragen, erklärt der Rückstand den Unterschied und der Lauf sagt nichts über die Qualität der laufenden Aktualisierung.'); +INSERT INTO txt VALUES ('H7273', 'English', 'Imports that had not been processed when the run started. If numbers are listed here, the backlog explains the difference and the run says nothing about the quality of the running update.'); +INSERT INTO txt VALUES ('H7274', 'German', 'Was vor diesem Lauf geändert wurde. Deshalb weicht das Ergebnis bewusst vom vorherigen Stand ab, und der Unterschied wird nicht als Problem gemeldet.'); +INSERT INTO txt VALUES ('H7274', 'English', 'What was changed before this run. That is why the result deliberately differs from the previous state, and the difference is not reported as a problem.'); +INSERT INTO txt VALUES ('H7275', 'German', 'Wie die Eigentümerzuordnung aktuell steht, abgeleitet aus dem zuletzt gelaufenen Neuaufbau. Nur Die letzte Prüfung hat Abweichungen gefunden weist auf ein Problem hin. Die unten aufgelisteten Läufe sind Historie: jeder Neuaufbau schreibt selbst den korrekten Stand, der Befund berichtet also, was vorher falsch war. Der Zeitstempel der letzten Prüfung ohne Befund wird nie verdrängt.'); +INSERT INTO txt VALUES ('H7275', 'English', 'How the owner mapping stands right now, taken from the rebuild that ran last. Only The last check found deviations points at a problem. The runs listed below are history: every rebuild writes the correct state itself, so a finding reports what was wrong before it ran. The timestamp of the last check without findings is never pushed out.'); '); INSERT INTO txt VALUES ('H7253', 'English', 'Results of the Import status checks of the active managements are recorded here. If anomalies as overdue or missing imports are found, separate alerts are raised, which can be analysed and handled at Open Alerts. @@ -9138,3 +9246,7 @@ INSERT INTO txt VALUES ('H5914', 'German', 'Standardmäßig zeigt der C INSERT INTO txt VALUES ('H5914', 'English', 'By default, the compliance diff report shows all violations found in the selected interval, including violations for rules that were already non-compliant at the start of the interval and violations that have since been resolved. When enabled, it shows only rules that were compliant at the start of the interval and subsequently became non-compliant. Resolved violations continue to be shown.'); INSERT INTO txt VALUES ('H5915', 'German', 'Deaktiviert: Es wird keine automatische Eigentümerzuordnung berechnet. Dies ist die Voreinstellung. Beim Speichern dieser Quelle und beim Neuberechnen werden bereits berechnete Zuordnungen entfernt.'); INSERT INTO txt VALUES ('H5915', 'English', 'Disabled: No automatic owner mapping is calculated. This is the default. Saving this source and recalculating remove the mappings calculated so far.'); +INSERT INTO txt VALUES ('H5916', 'German', 'Legt fest, wie ausführlich einzelne nicht zuordenbare Regeln und ungültige Eigentümer-Netze im Middleware-Log protokolliert werden. Bei vielen Altregeln ohne Zuordnung erzeugt jeder Lauf sonst eine Meldung pro Regel. Die Zusammenfassung pro Lauf und fehlgeschlagene Importe werden immer protokolliert.'); +INSERT INTO txt VALUES ('H5916', 'English', 'Controls how much detail about single unmappable rules and invalid owner networks is written to the middleware log. With many legacy rules that can never be mapped, every run would otherwise produce one message per rule. The per-run summary and failed imports are always logged.'); +INSERT INTO txt VALUES ('H5917', 'German', 'Protokollierung Zuordnungsprobleme: bestimmt, wie ausführlich einzelne Regeln und Objekte im Middleware-Log auftauchen, die nicht zugeordnet werden konnten. Die Zusammenfassung jedes Laufs, fehlgeschlagene Importe und Alarme werden unabhängig davon immer protokolliert. Die Einstellung gilt nur für die Eigentümerzuordnung und ändert die Protokollierung anderer Komponenten nicht. Die Stufen bauen aufeinander auf:'); +INSERT INTO txt VALUES ('H5917', 'English', 'Logging of mapping issues: sets how much detail about single rules and objects that could not be mapped appears in the middleware log. The summary of every run, failed imports and alerts are always logged regardless. The setting applies to the owner mapping only and does not change the logging of any other component. The levels build on each other:'); diff --git a/roles/lib/files/FWO.Basics/GlobalConstants.cs b/roles/lib/files/FWO.Basics/GlobalConstants.cs index 567a285621..c9e14cbdd6 100644 --- a/roles/lib/files/FWO.Basics/GlobalConstants.cs +++ b/roles/lib/files/FWO.Basics/GlobalConstants.cs @@ -62,6 +62,7 @@ public struct GlobalConst public const string kPlaceholderMarker = "@@"; public const string kModellerGroup = "ModellerGroup_"; public const string kImportChangeNotify = "importChangeNotify"; + public const string kRuleOwnerMapping = "ruleOwnerMapping"; public const string kExternalRequest = "externalRequest"; public const string kComplianceCheck = "complianceCheck"; public const long kPathAnalysisAlgorithmNone = 1; diff --git a/roles/lib/files/FWO.Config.Api/Data/ConfigData.cs b/roles/lib/files/FWO.Config.Api/Data/ConfigData.cs index 4e3269d05a..416a201d06 100644 --- a/roles/lib/files/FWO.Config.Api/Data/ConfigData.cs +++ b/roles/lib/files/FWO.Config.Api/Data/ConfigData.cs @@ -367,6 +367,9 @@ public class ConfigData : ICloneable [JsonProperty("CustomFieldOwnerKey"), JsonPropertyName("CustomFieldOwnerKey")] public string CustomFieldOwnerKey { get; set; } = ""; + [JsonProperty("ruleOwnerMappingLogLevel"), JsonPropertyName("ruleOwnerMappingLogLevel")] + public RuleOwnerMappingLogLevel RuleOwnerMappingLogLevel { get; set; } = RuleOwnerMappingLogLevel.Warning; + [JsonProperty("CustomFieldChangeIdKey"), JsonPropertyName("CustomFieldChangeIdKey")] public string CustomFieldChangeIdKey { get; set; } = GlobalConst.kDefaultChangeIdKeys; diff --git a/roles/lib/files/FWO.Data/Alert.cs b/roles/lib/files/FWO.Data/Alert.cs index e90546e185..cd1b07ddb9 100644 --- a/roles/lib/files/FWO.Data/Alert.cs +++ b/roles/lib/files/FWO.Data/Alert.cs @@ -30,6 +30,7 @@ public enum AlertCode ImportLogData = 44, ImportChangeNotify = 51, + RuleOwnerMapping = 52, ExternalRequest = 61, diff --git a/roles/lib/files/FWO.Data/Enums/RuleOwnerMappingLogLevel.cs b/roles/lib/files/FWO.Data/Enums/RuleOwnerMappingLogLevel.cs new file mode 100644 index 0000000000..e4f23f7675 --- /dev/null +++ b/roles/lib/files/FWO.Data/Enums/RuleOwnerMappingLogLevel.cs @@ -0,0 +1,50 @@ +namespace FWO.Data.Enums +{ + /// + /// How much per-rule and per-object detail the rule_owner mapping writes to the middleware log. + /// Installations with many legacy rules that can never be mapped would otherwise produce a message + /// per rule on every run. + /// + /// Always logged and never affected by this setting: the summary of every run, failed imports, + /// and every alert. The setting applies to the rule_owner mapping only and does not change the + /// log level of any other component. + /// + /// + /// When adding a message, place it by these criteria rather than by how the message feels: + /// how many of them can occur in one run, and whether someone has to act on it. + /// + /// + public enum RuleOwnerMappingLogLevel + { + /// + /// No message about single rules or objects at all. + /// + None = 0, + + /// + /// The configuration itself is unusable, so mapping cannot work as configured. Few messages per + /// run and always worth acting on, for instance an owner network with an invalid IP range or a + /// missing modelled marker. + /// + Error = 1, + + /// + /// Additionally rules that stay unmapped although they were meant to be mapped, which is a real + /// defect someone has to fix - for instance a marker pointing to a connection without an active + /// owner. Bounded by the number of rules that actually have a problem. This is the default. + /// + Warning = 2, + + /// + /// Additionally cases that are plausible in normal operation but useful while investigating a + /// single rule, for instance a rule without any network object. + /// + Info = 3, + + /// + /// Everything, including the cases that are the norm on a grown installation, such as every rule + /// that was never modelled. Highest volume by far. + /// + Debug = 4 + } +} diff --git a/roles/lib/files/FWO.Data/RuleOwner.cs b/roles/lib/files/FWO.Data/RuleOwner.cs index 3005a885d9..7b6163af30 100644 --- a/roles/lib/files/FWO.Data/RuleOwner.cs +++ b/roles/lib/files/FWO.Data/RuleOwner.cs @@ -44,6 +44,9 @@ public class UpdateRuleOwner { [JsonProperty("affected_rows"), JsonPropertyName("affected_rows")] public int AffectedRows { get; set; } + + [JsonProperty("returning"), JsonPropertyName("returning")] + public List Returning { get; set; } = []; } public class InsertRuleOwnerResult diff --git a/roles/lib/files/FWO.Services/EventMediator/Events/UpdateRuleOwnerMappingEventArgs.cs b/roles/lib/files/FWO.Services/EventMediator/Events/UpdateRuleOwnerMappingEventArgs.cs index e68a3c0226..1884120ccd 100644 --- a/roles/lib/files/FWO.Services/EventMediator/Events/UpdateRuleOwnerMappingEventArgs.cs +++ b/roles/lib/files/FWO.Services/EventMediator/Events/UpdateRuleOwnerMappingEventArgs.cs @@ -1,4 +1,5 @@ using FWO.Data; +using FWO.Services; using FWO.Services.EventMediator.Interfaces; using System; using System.Collections.Generic; @@ -11,6 +12,18 @@ public class UpdateRuleOwnerMappingEventArgs : IEventArgs { public bool isFullReInitialize { get; set; } = false; + /// + /// True when the full reinitialize follows a deliberate change, for instance a different marker, + /// another mapping source or edited owner networks. The rebuilt state then differs from the stored + /// one by design, so that difference must not be reported as drift of the incremental mapping. + /// + public bool TriggeredByChange { get; set; } = false; + + /// + /// What was changed, recorded with the run so its result can be understood later on. + /// + public List Changes { get; set; } = []; + public TaskCompletionSource? Completion { get; set; } } diff --git a/roles/lib/files/FWO.Services/RuleOwnerMappingLogger.cs b/roles/lib/files/FWO.Services/RuleOwnerMappingLogger.cs new file mode 100644 index 0000000000..485ff74453 --- /dev/null +++ b/roles/lib/files/FWO.Services/RuleOwnerMappingLogger.cs @@ -0,0 +1,82 @@ +using FWO.Data.Enums; +using FWO.Logging; + +namespace FWO.Services +{ + /// + /// Writes the per-rule and per-object messages of the rule_owner mapping, filtered by the configured + /// level. An installation with many legacy rules that can never be mapped would otherwise get one + /// message per rule on every run. Import failures, alerts and the per-run summary do not go through + /// this filter - they are always logged. + /// + public class RuleOwnerMappingLogger + { + private const string kLogMessageTitle = "Update rule_owner Notifier"; + + /// + /// Logger used where no configuration is available, keeping the behaviour of the default setting. + /// + public static RuleOwnerMappingLogger Default { get; } = new(RuleOwnerMappingLogLevel.Warning); + + private readonly RuleOwnerMappingLogLevel configuredLevel; + + /// + /// Creates a logger for the configured level. + /// + /// Level from the rule owner mapping settings. + public RuleOwnerMappingLogger(RuleOwnerMappingLogLevel configuredLevel) + { + this.configuredLevel = configuredLevel; + } + + /// Logs unusable input, for instance an owner network with an invalid IP range. + /// Message to log. + public void Error(string message) + { + if (IsEnabled(RuleOwnerMappingLogLevel.Error)) + { + Log.WriteError(kLogMessageTitle, message); + } + } + + /// Logs a rule or owner network that could not be mapped. + /// Message to log. + public void Warning(string message) + { + if (IsEnabled(RuleOwnerMappingLogLevel.Warning)) + { + Log.WriteWarning(kLogMessageTitle, message); + } + } + + /// Logs informational detail about a single rule. + /// Message to log. + public void Info(string message) + { + if (IsEnabled(RuleOwnerMappingLogLevel.Info)) + { + Log.WriteInfo(kLogMessageTitle, message); + } + } + + /// Logs why a single rule was skipped. + /// Message to log. + public void Debug(string message) + { + if (IsEnabled(RuleOwnerMappingLogLevel.Debug)) + { + Log.WriteDebug(kLogMessageTitle, message); + } + } + + /// + /// Checks whether messages of the given level are written. + /// + /// Level of the message. + /// True if the configured level covers it. + public bool IsEnabled(RuleOwnerMappingLogLevel messageLevel) + { + return configuredLevel >= messageLevel; + } + } +} diff --git a/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs b/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs new file mode 100644 index 0000000000..9e6f6bd2f2 --- /dev/null +++ b/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs @@ -0,0 +1,362 @@ +using FWO.Api.Client; +using FWO.Api.Client.Queries; +using FWO.Basics; +using FWO.Config.Api.Data; +using FWO.Data; +using FWO.Logging; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace FWO.Services +{ + /// + /// One rule-owner mapping pair as it is recorded in the run history. + /// + public class RuleOwnerPair + { + /// Rule the mapping belongs to. + [JsonPropertyName("ruleId")] + public long RuleId { get; set; } + + /// Owner the rule was mapped to. + [JsonPropertyName("ownerId")] + public int OwnerId { get; set; } + + /// + /// Import the mapping was originally created by. Only set for a removed pair, where it says how long + /// the obsolete mapping had been in place. For an added pair it is the run's own control id and is + /// therefore not repeated here. + /// + [JsonPropertyName("created"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public long? Created { get; set; } + + /// + /// Rule metadata the mapping belongs to. Survives new rule versions, so the affected rule stays + /// identifiable even after it was edited and got a new rule id. + /// + [JsonPropertyName("ruleMetadataId"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public long? RuleMetadataId { get; set; } + } + + /// + /// Settings a recorded change can refer to. + /// + public static class RuleOwnerMappingChangeSetting + { + /// The mapping source itself was switched. + public const string kSource = "source"; + + /// The marker of the name field mapping was changed. + public const string kMarker = "marker"; + + /// The keys of the custom field mapping were changed. + public const string kCustomFieldKeys = "customFieldKeys"; + + /// Owner data the mapping is calculated from was edited, for instance the owner networks. + public const string kOwnerData = "ownerData"; + } + + /// + /// One deliberate change that made a full reinitialize necessary. Recorded so a run whose result + /// differs on purpose can be told apart from one where the incremental mapping failed. + /// + public class RuleOwnerMappingChange + { + /// Setting that was changed, see . + [JsonPropertyName("setting")] + public string Setting { get; set; } = ""; + + /// Value before the change, empty when it cannot be named. + [JsonPropertyName("from")] + public string From { get; set; } = ""; + + /// Value after the change, empty when it cannot be named. + [JsonPropertyName("to")] + public string To { get; set; } = ""; + } + + /// + /// Result of one full reinitialize, kept so drift of the incremental mapping becomes visible. + /// + public class RuleOwnerMappingRun + { + /// When the full reinitialize finished. + [JsonPropertyName("runTime")] + public DateTime RunTime { get; set; } + + /// + /// Import control the full reinitialize was recorded under. Every pair in and + /// belongs to this id in the rule_owner table, so it is not repeated per entry. + /// + [JsonPropertyName("controlId")] + public long ControlId { get; set; } + + /// Mapping source that produced the result. + [JsonPropertyName("mappingSource")] + public string MappingSource { get; set; } = ""; + + /// Number of mappings after the run. + [JsonPropertyName("mappingCount")] + public int MappingCount { get; set; } + + /// Pairs that did not exist before the run. + [JsonPropertyName("addedCount")] + public int AddedCount { get; set; } + + /// Pairs that existed before the run and are gone afterwards. + [JsonPropertyName("removedCount")] + public int RemovedCount { get; set; } + + /// + /// Pairs the full reinitialize established although they did not exist before, so the incremental + /// mapping never created them. Look them up as rule_owner rows with created = . + /// Capped, see ; always holds the full number. + /// + [JsonPropertyName("added")] + public List Added { get; set; } = []; + + /// + /// Pairs that were active before and the full reinitialize does not produce any more, so the + /// incremental mapping left them behind. Look them up as rule_owner rows with + /// removed = . Capped, see ; + /// always holds the full number. + /// + [JsonPropertyName("removed")] + public List Removed { get; set; } = []; + + /// True when the pair lists were cut off and the counts are higher than the listed entries. + [JsonPropertyName("pairListsTruncated")] + public bool PairListsTruncated { get; set; } + + /// Imports that were still waiting to be mapped when the run started. + [JsonPropertyName("pendingImportsBefore")] + public List PendingImportsBefore { get; set; } = []; + + /// + /// True only when no import was pending. Otherwise the difference just reflects the unprocessed + /// backlog instead of drift, and must not be read as a mapping problem. + /// + [JsonPropertyName("diffMeaningful")] + public bool DiffMeaningful { get; set; } + + /// + /// True when the run followed a deliberate change, for instance a different marker, another mapping + /// source or edited owner networks. The rebuilt state then differs from the stored one by design, so + /// the difference is expected and is not reported as drift. + /// + [JsonPropertyName("triggeredByChange")] + public bool TriggeredByChange { get; set; } + + /// + /// What was changed, so a run whose result differs on purpose can be understood later on. + /// + [JsonPropertyName("changes"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public List Changes { get; set; } = []; + } + + /// + /// What is kept about the full reinitialize runs: the runs that found something, and separately the + /// last one that found nothing. Splitting them keeps every slot of the limited history available for + /// actual findings, while "last verified correct" stays visible and can never be pushed out. + /// + public class RuleOwnerMappingRunHistoryData + { + /// Most recent run that found no difference, or if there was none yet. + [JsonPropertyName("lastRunWithoutFindings")] + public RuleOwnerMappingRun? LastRunWithoutFindings { get; set; } + + /// Runs that found a difference, newest first. + [JsonPropertyName("runsWithFindings")] + public List RunsWithFindings { get; set; } = []; + } + + /// + /// Keeps the results of the last full reinitialize runs in a config entry. + /// A full reinitialize replaces every mapping, so the difference between the state before and the + /// rebuilt state is what tells an actual change from a plain rebuild. With a healthy incremental + /// mapping and an empty import backlog that difference is empty. + /// + public class RuleOwnerMappingRunHistory + { + /// Config key the history is stored under. + public const string kConfigKey = "ruleOwnerMappingRunHistory"; + + private const int kMaxRuns = 10; + private const int kMaxListedPairs = 500; + private const string kLogMessageTitle = "Update rule_owner Notifier"; + + private static readonly JsonSerializerOptions SerializerOptions = new() { WriteIndented = false }; + + private readonly ApiConnection apiConnection; + + /// + /// Creates the history writer. + /// + /// GraphQL API connection. + public RuleOwnerMappingRunHistory(ApiConnection apiConnection) + { + this.apiConnection = apiConnection; + } + + /// + /// Builds the result of one full reinitialize from the state before and the rebuilt mappings. + /// + /// Import control of the full reinitialize. + /// Mapping source that produced the result. + /// Mappings that were active before the run. + /// Mappings the run rebuilt. + /// Imports still waiting to be mapped when the run started. + /// True when the run followed a deliberate change. + /// What was changed, empty when nothing was recorded. + /// The recorded run. + public static RuleOwnerMappingRun BuildRun(long controlId, OwnerMappingSourceStm mappingSource, List previousRuleOwners, + List newRuleOwners, List pendingImportsBefore, bool triggeredByChange = false, + List? changes = null) + { + Dictionary<(long RuleId, int OwnerId), RuleOwner> previousByPair = ToPairMap(previousRuleOwners); + Dictionary<(long RuleId, int OwnerId), RuleOwner> newByPair = ToPairMap(newRuleOwners); + + List added = ToPairList(newByPair.Keys.Except(previousByPair.Keys), newByPair, withOrigin: false); + List removed = ToPairList(previousByPair.Keys.Except(newByPair.Keys), previousByPair, withOrigin: true); + + // switching the mapping source replaces every mapping by definition, so listing each pair would + // only fill the config entry without telling anybody anything - the counts carry the information + bool keepPairLists = !(changes ?? []).Any(change => change.Setting == RuleOwnerMappingChangeSetting.kSource); + + return new RuleOwnerMappingRun + { + RunTime = DateTime.UtcNow, + ControlId = controlId, + MappingSource = mappingSource.ToString(), + MappingCount = newByPair.Count, + AddedCount = added.Count, + RemovedCount = removed.Count, + Added = keepPairLists ? added.Take(kMaxListedPairs).ToList() : [], + Removed = keepPairLists ? removed.Take(kMaxListedPairs).ToList() : [], + PairListsTruncated = keepPairLists && (added.Count > kMaxListedPairs || removed.Count > kMaxListedPairs), + PendingImportsBefore = pendingImportsBefore, + DiffMeaningful = pendingImportsBefore.Count == 0, + TriggeredByChange = triggeredByChange, + Changes = changes ?? [] + }; + } + + /// + /// Prepends the run to the stored history and drops everything beyond the newest entries. + /// + /// Run to store. + public async Task Store(RuleOwnerMappingRun run) + { + try + { + RuleOwnerMappingRunHistoryData history = await Load(); + + // a run without findings updates "last verified correct", so a repeated rebuild cannot push + // anything out of the limited history - but only when it could judge at all: with imports + // still pending it proves nothing and must not claim a verification + if (HasNoFindings(run) && run.DiffMeaningful) + { + history.LastRunWithoutFindings = run; + } + + // a deliberate change is kept even when it changed nothing: that it had no effect is exactly + // what somebody who just edited the configuration needs to see + if (!HasNoFindings(run) || run.TriggeredByChange) + { + history.RunsWithFindings.Insert(0, run); + history.RunsWithFindings = history.RunsWithFindings.Take(kMaxRuns).ToList(); + } + + await apiConnection.SendQueryAsync(ConfigQueries.upsertConfigItem, new + { + config_key = kConfigKey, + config_value = JsonSerializer.Serialize(history, SerializerOptions), + config_user = 0 + }); + } + catch (Exception ex) + { + // the history is a diagnostic aid, it must never break the mapping itself + Log.WriteError(kLogMessageTitle, "Error while storing the rule_owner mapping run history.", ex); + } + } + + /// + /// Reads the stored history, runs with findings newest first. + /// + /// The stored history, empty when nothing is stored yet or the entry is unreadable. + public async Task Load() + { + try + { + List? configItems = await apiConnection.SendQueryAsync>(ConfigQueries.getConfigItemByKey, new { key = kConfigKey }); + string? storedValue = configItems?.FirstOrDefault()?.Value; + + return string.IsNullOrWhiteSpace(storedValue) ? new RuleOwnerMappingRunHistoryData() : Deserialize(storedValue); + } + catch (Exception ex) + { + Log.WriteError(kLogMessageTitle, "Error while reading the rule_owner mapping run history.", ex); + return new RuleOwnerMappingRunHistoryData(); + } + } + + /// + /// Reads the stored value, accepting the plain run list written before the history was split into + /// findings and the last clean run, so an installation does not lose what it recorded so far. + /// + /// Stored config value. + /// The history in its current shape. + private static RuleOwnerMappingRunHistoryData Deserialize(string storedValue) + { + if (!storedValue.TrimStart().StartsWith('[')) + { + return JsonSerializer.Deserialize(storedValue) ?? new RuleOwnerMappingRunHistoryData(); + } + + List storedRuns = JsonSerializer.Deserialize>(storedValue) ?? []; + return new RuleOwnerMappingRunHistoryData + { + LastRunWithoutFindings = storedRuns.FirstOrDefault(HasNoFindings), + RunsWithFindings = storedRuns.Where(run => !HasNoFindings(run)).Take(kMaxRuns).ToList() + }; + } + + /// + /// Checks whether a run found any difference at all. + /// + /// Run to check. + /// True if nothing was added or removed. + private static bool HasNoFindings(RuleOwnerMappingRun run) + { + return run.AddedCount + run.RemovedCount == 0; + } + + private static Dictionary<(long RuleId, int OwnerId), RuleOwner> ToPairMap(List ruleOwners) + { + return ruleOwners.GroupBy(ruleOwner => (ruleOwner.RuleId, ruleOwner.OwnerId)) + .ToDictionary(group => group.Key, group => group.First()); + } + + /// + /// Turns the pairs into the stored form. The origin is only kept for mappings that were removed: + /// for an added one it is the run's own control id and would only be repeated. + /// + /// Pairs to store. + /// Mappings the pairs were taken from. + /// True to keep the import the mapping originally came from. + /// The stored pairs, ordered by rule and owner. + private static List ToPairList(IEnumerable<(long RuleId, int OwnerId)> pairs, + Dictionary<(long RuleId, int OwnerId), RuleOwner> ruleOwnersByPair, bool withOrigin) + { + return pairs.OrderBy(pair => pair.RuleId).ThenBy(pair => pair.OwnerId) + .Select(pair => new RuleOwnerPair + { + RuleId = pair.RuleId, + OwnerId = pair.OwnerId, + Created = withOrigin && ruleOwnersByPair.TryGetValue(pair, out RuleOwner? origin) ? origin.Created : null, + RuleMetadataId = ruleOwnersByPair.TryGetValue(pair, out RuleOwner? ruleOwner) ? ruleOwner.RuleMetadataId : null + }).ToList(); + } + } +} diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs index 5f0c7fea1d..561273deee 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs @@ -12,6 +12,10 @@ namespace FWO.Services public abstract class UpdateRuleOwnerMappingBase : IUpdateRuleOwnerMapping { protected const int MaxPendingImportsBeforeFullReinit = 3; + private const int kAlertSeverity = 1; + + private bool triggeredByChange; + private List appliedChanges = []; protected const int RuleOwnerRemovalBatchSize = 500; protected const int RuleOwnerInsertBatchSize = 500; @@ -19,10 +23,17 @@ public abstract class UpdateRuleOwnerMappingBase : IUpdateRuleOwnerMapping protected readonly ApiConnection apiConnection; protected readonly GlobalConfig globalConfig; + /// + /// Writes the per-rule and per-object messages, filtered by the configured level. Import failures, + /// alerts and the per-run summary bypass this and are always logged. + /// + protected RuleOwnerMappingLogger MappingLog { get; } + protected UpdateRuleOwnerMappingBase(ApiConnection apiConnection, GlobalConfig globalConfig) { this.apiConnection = apiConnection; this.globalConfig = globalConfig; + MappingLog = new RuleOwnerMappingLogger(globalConfig.RuleOwnerMappingLogLevel); } public abstract OwnerMappingSourceStm Source { get; } @@ -30,11 +41,28 @@ protected UpdateRuleOwnerMappingBase(ApiConnection apiConnection, GlobalConfig g public abstract Task RunAsync(UpdateRuleOwnerMappingEventArgs? eventArgs = null); /// - /// Chooses between full reinitialize and incremental processing based on the event arguments. + /// Chooses between full reinitialize and incremental processing based on the event arguments and + /// remembers whether the run follows a configuration change, which decides how its result is read. + /// + /// Rebuilds every mapping. + /// Processes the pending imports. + /// Arguments of the triggering event. + /// True if the run succeeded. + protected async Task UpdateRuleOwners(Func> fullReinitFunc, Func> incrementalFunc, UpdateRuleOwnerMappingEventArgs? eventArgs) + { + TakeOverEventArgs(eventArgs); + return (eventArgs?.isFullReInitialize ?? false) ? await fullReinitFunc() : await incrementalFunc(); + } + + /// + /// Remembers what the triggering event said about the run, which decides how its result is read. + /// Sources that do not go through have to call this themselves. /// - protected static async Task UpdateRuleOwners(Func> fullReinitFunc, Func> incrementalFunc, bool isFullReInitialize) + /// Arguments of the triggering event. + protected void TakeOverEventArgs(UpdateRuleOwnerMappingEventArgs? eventArgs) { - return isFullReInitialize ? await fullReinitFunc() : await incrementalFunc(); + triggeredByChange = eventArgs?.TriggeredByChange ?? false; + appliedChanges = eventArgs?.Changes ?? []; } /// @@ -52,15 +80,13 @@ protected async Task RunFullReinitialize(string rulesQuery, /// /// Persists a full reinitialize by replacing all active rule-owner mappings with the provided set. + /// An empty set is a valid result and still replaces the previous state: the configured mapping + /// source can legitimately stop matching any rule, and the obsolete mappings have to go. Because + /// that is almost always a configuration problem, it raises an alert instead of failing silently. /// protected async Task FinalizeFullReinitialize(List newRuleOwners) { - if (!newRuleOwners.Any()) - { - Log.WriteInfo(LogMessageTitle, "No new rule owners to insert. Aborting import."); - return false; - } - + List pendingImportsBefore = await LoadPendingImportControlIds(); long importControlId = await CreateImportControl(); foreach (RuleOwner ruleOwner in newRuleOwners) @@ -68,14 +94,66 @@ protected async Task FinalizeFullReinitialize(List newRuleOwner ruleOwner.Created = importControlId; } - await SetAllActiveRuleOwnersRemoved(importControlId); + List previousRuleOwners = await SetAllActiveRuleOwnersRemoved(importControlId); await InsertNewRuleOwners(newRuleOwners); await CompleteImportControlFullReInit(importControlId); - Log.WriteInfo(LogMessageTitle, "FULL rule_owner reinitialize completed."); + await RecordRun(importControlId, previousRuleOwners, newRuleOwners, pendingImportsBefore); + + if (!newRuleOwners.Any()) + { + await AlertEmptyMappingResult(); + } + + Log.WriteInfo(LogMessageTitle, $"FULL rule_owner reinitialize completed with {newRuleOwners.Count} mappings."); return true; } + /// + /// Stores the result of a full reinitialize and alerts when it changed anything although the + /// incremental mapping was up to date - that difference means the incremental path missed something. + /// + /// Import control of the full reinitialize. + /// Mappings that were active before the run. + /// Mappings the run rebuilt. + /// Imports still waiting to be mapped when the run started. + protected async Task RecordRun(long importControlId, List previousRuleOwners, List newRuleOwners, List pendingImportsBefore) + { + RuleOwnerMappingRun run = RuleOwnerMappingRunHistory.BuildRun(importControlId, Source, previousRuleOwners, newRuleOwners, + pendingImportsBefore, triggeredByChange, appliedChanges); + await new RuleOwnerMappingRunHistory(apiConnection).Store(run); + + Log.WriteInfo(LogMessageTitle, $"Full reinitialize {importControlId}: {run.AddedCount} mappings added, {run.RemovedCount} removed, " + + $"{run.PendingImportsBefore.Count} imports were still pending."); + + if (IndicatesDrift(run)) + { + await AlertMappingDrift(run); + } + } + + /// + /// Decides whether a run result means the incremental mapping missed something. A deliberate change + /// rebuilds a different state on purpose, a pending backlog explains the difference on its own, and an + /// empty result has its own more precise alert - none of those is drift. + /// + /// Result of the full reinitialize. + /// True if the difference points at the incremental mapping. + private static bool IndicatesDrift(RuleOwnerMappingRun run) + { + return run.DiffMeaningful && !run.TriggeredByChange && run.MappingCount > 0 && run.AddedCount + run.RemovedCount > 0; + } + + /// + /// Reads the control ids of the imports that are still waiting to be mapped. + /// + /// The pending control ids, empty when the backlog is clear. + protected async Task> LoadPendingImportControlIds() + { + List? pendingImports = await apiConnection.SendQueryAsync>(ImportQueries.getPendingRuleOwnerImports); + return pendingImports?.Select(import => import.ControlId).ToList() ?? []; + } + /// /// Processes all pending incremental imports in control-id order and falls back to full reinitialize when too many imports are queued. /// @@ -91,9 +169,12 @@ protected async Task RunIncremental(Func processIncre if (pendingImports.Count > MaxPendingImportsBeforeFullReinit) { Log.WriteWarning(LogMessageTitle, $"Found {pendingImports.Count} pending imports. Falling back to full rule_owner reinitialize."); + await AlertFullReinitFallback(pendingImports.Count); return await fullReinitFunc(); } + List failedImportControlIds = []; + foreach (var import in pendingImports.OrderBy(i => i.ControlId)) { try @@ -102,11 +183,19 @@ protected async Task RunIncremental(Func processIncre } catch (Exception ex) { + // one broken import must not block the pending imports behind it, so the loop + // continues and the failures are reported instead of being swallowed + failedImportControlIds.Add(import.ControlId); Log.WriteError(LogMessageTitle, $"Error while processing import_control {import.ControlId}. ", ex); - break; } } + if (failedImportControlIds.Any()) + { + await AlertFailedIncrementalImports(failedImportControlIds); + return false; + } + return true; } @@ -149,10 +238,56 @@ protected async Task FinalizeIncrementalImport(List newRuleOwners, Li } await SetAffectedRuleOwnersRemoved(ruleOwnersToRemove, importControlId); - await InsertNewRuleOwners(newRuleOwners); + await InsertNewRuleOwners(await DropStillActiveMappings(newRuleOwners)); await CompleteImportControl(importControlId); } + /// + /// Drops mappings that are still active after the removal step so the insert cannot collide with the + /// partial unique index on (rule_id, owner_id) where removed is null. An owner insert or reactivation + /// rebuilds the mapping for every rule without removing anything first, and the on_conflict clause on + /// pk_rule_owner (rule_id, owner_id, created) does not catch that because the created value differs. + /// + /// Mappings that were just rebuilt for this import. + /// The mappings that are safe to insert. + protected async Task> DropStillActiveMappings(List newRuleOwners) + { + if (!newRuleOwners.Any()) + { + return newRuleOwners; + } + + List activeRuleOwners = await LoadActiveMappingsForSmallerKeySet(newRuleOwners); + HashSet<(long RuleId, int OwnerId)> activePairs = activeRuleOwners.Select(ruleOwner => (ruleOwner.RuleId, ruleOwner.OwnerId)).ToHashSet(); + List insertableRuleOwners = newRuleOwners.Where(ruleOwner => !activePairs.Contains((ruleOwner.RuleId, ruleOwner.OwnerId))).ToList(); + + int skippedCount = newRuleOwners.Count - insertableRuleOwners.Count; + if (skippedCount > 0) + { + Log.WriteInfo(LogMessageTitle, $"Skipped {skippedCount} rule_owner mappings that are already active."); + } + + return insertableRuleOwners; + } + + /// + /// Loads the currently active mappings, filtering by whichever key set is smaller: an owner import + /// rebuilds few owners across all rules, a rule import few rules across all owners. + /// + /// Mappings that were just rebuilt for this import. + /// The active mappings covering the rebuilt set. + private async Task> LoadActiveMappingsForSmallerKeySet(List newRuleOwners) + { + List ruleIds = newRuleOwners.Select(ruleOwner => ruleOwner.RuleId).Distinct().ToList(); + List ownerIds = newRuleOwners.Select(ruleOwner => ruleOwner.OwnerId).Distinct().ToList(); + + List? activeRuleOwners = ruleIds.Count <= ownerIds.Count + ? await apiConnection.SendQueryAsync>(OwnerQueries.getRuleOwnerToRemoveByRule, new { ruleIds }) + : await apiConnection.SendQueryAsync>(OwnerQueries.getRuleOwnerToRemoveByOwner, new { ownerIds }); + + return activeRuleOwners ?? []; + } + /// /// Loads changed rules for one incremental rule import and fetches affected owners plus removable mappings. /// @@ -206,11 +341,18 @@ protected async Task CreateImportControl() } } - protected async Task SetAllActiveRuleOwnersRemoved(long controlId) + /// + /// Marks every active mapping as removed and returns the state that was just replaced, which the + /// run history diffs against the rebuilt mappings. + /// + /// Import control the removal is recorded under. + /// The mappings that were active before the removal. + protected async Task> SetAllActiveRuleOwnersRemoved(long controlId) { try { - await apiConnection.SendQueryAsync(OwnerQueries.setAllActiveRuleOwnersRemoved, new { controlId }); + UpdateRuleOwner? result = await apiConnection.SendQueryAsync(OwnerQueries.setAllActiveRuleOwnersRemoved, new { controlId }); + return result?.Returning ?? []; } catch (Exception ex) { @@ -360,11 +502,87 @@ await apiConnection.SendQueryAsync(ImportQueries.updateImportCont } + /// + /// Raises an alert for incremental imports that could not be processed, so a failing rule_owner + /// mapping becomes visible outside the middleware log file. + /// + /// Control ids of the imports that failed. + private async Task AlertFailedIncrementalImports(List failedImportControlIds) + { + await RaiseAlert($"Rule owner mapping failed for import_control {string.Join(", ", failedImportControlIds)}. See the middleware log for details."); + } + + /// + /// Raises an alert when the pending import backlog forces a full reinitialize, because that hides + /// whatever stopped the incremental processing from keeping up. + /// + /// Number of imports waiting to be mapped. + private async Task AlertFullReinitFallback(int pendingImportCount) + { + await RaiseAlert($"Rule owner mapping fell back to a full reinitialize because {pendingImportCount} imports were pending."); + } + + /// + /// Raises an alert when a full reinitialize produced no mapping at all, which removes every existing + /// mapping and almost always points at a misconfigured mapping source. + /// + private async Task AlertEmptyMappingResult() + { + await RaiseAlert($"Rule owner mapping source '{Source}' matched no rule. All existing rule_owner mappings were removed."); + } + + /// + /// Raises an alert when a full reinitialize changed mappings although no import was pending. With a + /// correct incremental mapping the rebuilt state matches the stored one, so any difference means the + /// incremental path missed a change. + /// + /// The recorded run holding the difference. + private async Task AlertMappingDrift(RuleOwnerMappingRun run) + { + await RaiseAlert($"Full rule_owner reinitialize {run.ControlId} added {run.AddedCount} and removed {run.RemovedCount} mappings " + + $"although no import was pending. The incremental mapping missed these changes. See config key '{RuleOwnerMappingRunHistory.kConfigKey}' for the affected rules and owners."); + } + + /// + /// Writes a log entry and an alert unless the same alert is already open, so a job repeating every + /// few seconds does not flood the alert list with identical entries. + /// + /// Description shown in the alert and the log entry. + private async Task RaiseAlert(string description) + { + try + { + if (await SameAlertAlreadyOpen(description)) + { + return; + } + + await AlertHelper.AddLogEntry(apiConnection, kAlertSeverity, LogMessageTitle, description, GlobalConst.kRuleOwnerMapping); + await AlertHelper.SetAlert(apiConnection, LogMessageTitle, description, GlobalConst.kRuleOwnerMapping, AlertCode.RuleOwnerMapping, + new AlertHelper.AdditionalAlertData { CompareDesc = true }); + } + catch (Exception ex) + { + Log.WriteError(LogMessageTitle, "Error while raising a rule_owner mapping alert.", ex); + } + } + + /// + /// Checks whether an unacknowledged rule_owner mapping alert with the same description exists. + /// + /// Description to look for. + /// True if such an alert is still open. + private async Task SameAlertAlreadyOpen(string description) + { + List? openAlerts = await apiConnection.SendQueryAsync>(MonitorQueries.getOpenAlerts); + return openAlerts?.Any(alert => alert.AlertCode == AlertCode.RuleOwnerMapping && alert.Description == description) == true; + } + protected static bool ProcessOwnerChanges(List changelogOwners, List ownersToAdd, List ownersToRemove) { if (changelogOwners == null || !changelogOwners.Any()) { - Log.WriteInfo(LogMessageTitle, "No changed owners found for rule-owner mapping. Aborting incremental import."); + Log.WriteInfo(LogMessageTitle, "No changed owners found for rule-owner mapping. Nothing to map for this import."); return false; } foreach (var change in changelogOwners) @@ -395,7 +613,7 @@ protected static bool ProcessRuleChanges(List changelogRules, List RunAsync(UpdateRuleOwnerMappingEventArgs? eventArgs = null) { - bool isFullReInitialize = eventArgs?.isFullReInitialize ?? false; - return await UpdateRuleOwners(RunFullReinitialize, RunIncremental, isFullReInitialize); + return await UpdateRuleOwners(RunFullReinitialize, RunIncremental, eventArgs); } /// @@ -100,7 +99,7 @@ public List BuildNewRuleOwnersCustomField(List rulesToMap, List } catch (Exception ex) { - Log.WriteWarning(LogMessageTitle, $"Rule {rule.Id} has invalid CustomFields: {ex.Message}"); + MappingLog.Warning($"Rule {rule.Id} has invalid CustomFields: {ex.Message}"); } } return newRuleOwners; diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs index a9e1c5b9b4..63bac910dc 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs @@ -37,6 +37,10 @@ public UpdateRuleOwnerMappingDisabled(ApiConnection apiConnection, GlobalConfig /// True if the disabled mapping state was established successfully. public override async Task RunAsync(UpdateRuleOwnerMappingEventArgs? eventArgs = null) { + // this source does not use UpdateRuleOwners, so the change note of the triggering save would + // otherwise be lost and switching the mapping off would look like drift + TakeOverEventArgs(eventArgs); + if (!(eventArgs?.isFullReInitialize ?? false)) { // the scheduled run has nothing to do while the owner mapping is disabled @@ -57,10 +61,15 @@ private async Task RemoveAllRuleOwnerMappings() return true; } + List pendingImportsBefore = await LoadPendingImportControlIds(); long importControlId = await CreateImportControl(); - await SetAllActiveRuleOwnersRemoved(importControlId); + List previousRuleOwners = await SetAllActiveRuleOwnersRemoved(importControlId); await CompleteImportControlFullReInit(importControlId); + // switching the mapping off removes every mapping, which belongs in the run history just like any + // other rebuild - otherwise the entry that explains an empty mapping table would be missing + await RecordRun(importControlId, previousRuleOwners, [], pendingImportsBefore); + Log.WriteInfo(LogMessageTitle, "All rule_owner mappings removed because the owner mapping source is disabled."); return true; } diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingIpBased.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingIpBased.cs index 9c673e4d5f..2de85cb5b8 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingIpBased.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingIpBased.cs @@ -23,8 +23,7 @@ public UpdateRuleOwnerMappingIpBased(ApiConnection apiConnection, GlobalConfig g public override async Task RunAsync(UpdateRuleOwnerMappingEventArgs? eventArgs = null) { - bool isFullReInitialize = eventArgs?.isFullReInitialize ?? false; - return await UpdateRuleOwners(RunFullReinitialize, RunIncremental, isFullReInitialize); + return await UpdateRuleOwners(RunFullReinitialize, RunIncremental, eventArgs); } /// @@ -50,7 +49,7 @@ public List BuildNewRuleOwnersIpBased(List rulesToMap, List BuildNewRuleOwnersIpBased(List rulesToMap, List 0) { - Log.WriteError(LogMessageTitle, $"Invalid range: {start}-{end} (start > end)"); + log.Warning($"Invalid range: {start}-{end} (start > end)"); return (null, null); } @@ -177,14 +177,14 @@ public List PrepareOwnerNetworks(List ownersToMa { if (!nw.IP.TryParseIPStringToRange(out var _)) { - Log.WriteWarning(LogMessageTitle, $"Invalid owner network format for owner {o.Id}: {nw.IP}-{nw.IpEnd}"); + MappingLog.Error($"Invalid owner network format for owner {o.Id}: {nw.IP}-{nw.IpEnd}"); } - var (range, version) = GetIpRangeAndVersion(nw.IP, nw.IpEnd); + var (range, version) = GetIpRangeAndVersion(nw.IP, nw.IpEnd, MappingLog); if (range == null || version == null) { - Log.WriteWarning(LogMessageTitle, $"Skipping owner network with invalid IP range for owner {o.Id}: {nw.IP}-{nw.IpEnd}"); + MappingLog.Error($"Skipping owner network with invalid IP range for owner {o.Id}: {nw.IP}-{nw.IpEnd}"); return null; } @@ -200,8 +200,9 @@ public List PrepareOwnerNetworks(List ownersToMa .ToList(); } - public static Dictionary>> GetMatchingOwnerIds(Rule rule, List ownerNetworksPrepared) + public static Dictionary>> GetMatchingOwnerIds(Rule rule, List ownerNetworksPrepared, RuleOwnerMappingLogger? mappingLog = null) { + RuleOwnerMappingLogger log = mappingLog ?? RuleOwnerMappingLogger.Default; var matchesByOwner = new Dictionary>>(); var ruleNetworksWithDirections = rule.Froms.Where(n => n?.Object != null).Select(n => (Obj: n.Object!, Direction: "From")) @@ -210,7 +211,7 @@ public static Dictionary>> GetMatchi if (!ruleNetworksWithDirections.Any()) { - Log.WriteWarning(LogMessageTitle, $"Rule {rule.Id} has no network locations and will be skipped."); + log.Info($"Rule {rule.Id} has no network locations and will be skipped."); return matchesByOwner; } @@ -222,7 +223,7 @@ public static Dictionary>> GetMatchi continue; } - var (ruleRange, ruleIpVersion) = GetIpRangeAndVersion(obj.IP, obj.IpEnd); + var (ruleRange, ruleIpVersion) = GetIpRangeAndVersion(obj.IP, obj.IpEnd, log); if (ruleRange == null || ruleIpVersion == null) { diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs index 11ca665ebd..c59b33e149 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs @@ -24,8 +24,7 @@ public UpdateRuleOwnerMappingNameField(ApiConnection apiConnection, GlobalConfig public override async Task RunAsync(UpdateRuleOwnerMappingEventArgs? eventArgs = null) { - bool isFullReInitialize = eventArgs?.isFullReInitialize ?? false; - return await UpdateRuleOwners(RunFullReinitialize, RunIncremental, isFullReInitialize); + return await UpdateRuleOwners(RunFullReinitialize, RunIncremental, eventArgs); } /// @@ -86,6 +85,12 @@ private List BuildNewRuleOwnersNameField(List rulesToMap, List< var connectionsToOwnerMap = connectionOwnersToMap.Where(c => c.AppId.HasValue) .ToDictionary(c => c.Id, c => c.AppId!.Value); var newRuleOwners = new List(); + if (rulesToMap.Any() && string.IsNullOrWhiteSpace(globalConfig.ModModelledMarker)) + { + // reported once per run instead of once per rule, and only when there is something to map, + // so an idle scheduled run stays silent: without a marker no rule can be mapped at all + MappingLog.Error("No modelled marker is configured, so no rule can be mapped by name field."); + } int rulesWithoutMarker = 0; int rulesWithoutMatchingConnection = 0; int invalidRules = 0; @@ -109,16 +114,20 @@ private List BuildNewRuleOwnersNameField(List rulesToMap, List< if (status == NameFieldExtractionStatus.NoMarker) { + // expected for every rule that was never modelled, so only on the most detailed level rulesWithoutMarker++; + MappingLog.Debug($"Rule {rule.Id}: {errorMessage}"); } else if (nameFieldValue.HasValue) { + // the marker is there but points nowhere, which usually is a real modelling problem rulesWithoutMatchingConnection++; + MappingLog.Warning($"Rule {rule.Id}: marker points to connection {nameFieldValue.Value}, which has no active owner."); } else { invalidRules++; - Log.WriteDebug(LogMessageTitle, $"Rule {rule.Id}: {errorMessage}"); + LogUnreadableRule(status, rule.Id, errorMessage); } } @@ -130,6 +139,23 @@ private List BuildNewRuleOwnersNameField(List rulesToMap, List< return newRuleOwners; } + /// + /// Logs a rule whose marker could not be read, at the level matching how much attention it needs. + /// + /// Why the marker could not be read. + /// Rule the marker was read from. + /// Detail of the failed extraction. + private void LogUnreadableRule(NameFieldExtractionStatus status, long ruleId, string? errorMessage) + { + if (status == NameFieldExtractionStatus.InvalidConnectionId || status == NameFieldExtractionStatus.Error) + { + // the marker is there but unusable, so the rule was modelled and still stays unmapped + MappingLog.Warning($"Rule {ruleId}: {errorMessage}"); + return; + } + MappingLog.Info($"Rule {ruleId}: {errorMessage}"); + } + public static int? ExtractNameFieldValue(Rule rule, string modelledMarker, out string? errorMessage) { return ExtractNameFieldValue(rule, modelledMarker, out errorMessage, out _); diff --git a/roles/middleware/files/FWO.Middleware.Server/Jobs/UpdateRuleOwnerMappingJob.cs b/roles/middleware/files/FWO.Middleware.Server/Jobs/UpdateRuleOwnerMappingJob.cs index 4cd3bfd4aa..9047b5df25 100644 --- a/roles/middleware/files/FWO.Middleware.Server/Jobs/UpdateRuleOwnerMappingJob.cs +++ b/roles/middleware/files/FWO.Middleware.Server/Jobs/UpdateRuleOwnerMappingJob.cs @@ -14,6 +14,7 @@ namespace FWO.Middleware.Server.Jobs public class UpdateRuleOwnerMappingJob : IJob { private const string LogMessageTitle = "Update rule_owner Notify"; + private const int kAlertSeverity = 1; private readonly ApiConnection apiConnection; private readonly GlobalConfig globalConfig; @@ -38,7 +39,7 @@ public async Task Execute(IJobExecutionContext context) } catch (Exception exc) { - await AlertHelper.LogErrorsWithAlert(apiConnection, globalConfig, 1, LogMessageTitle, GlobalConst.kImportChangeNotify, AlertCode.ImportChangeNotify, exc); + await AlertHelper.LogErrorsWithAlert(apiConnection, globalConfig, kAlertSeverity, LogMessageTitle, GlobalConst.kRuleOwnerMapping, AlertCode.RuleOwnerMapping, exc); } } } diff --git a/roles/tests-unit/files/FWO.Test/OwnerMappingSourceHandlerTest.cs b/roles/tests-unit/files/FWO.Test/OwnerMappingSourceHandlerTest.cs index aa33c43695..eaf3a4b309 100644 --- a/roles/tests-unit/files/FWO.Test/OwnerMappingSourceHandlerTest.cs +++ b/roles/tests-unit/files/FWO.Test/OwnerMappingSourceHandlerTest.cs @@ -1,5 +1,8 @@ using FWO.Basics; using FWO.Config.Api.Data; +using FWO.Services; +using System.Collections.Generic; +using System.Linq; using FWO.Ui.Services; using NUnit.Framework; @@ -657,5 +660,105 @@ public void ApplyTo_RequestsNoRebuild_WhenOnlyUnusedSettingOfOtherSourceChanged( Assert.That(configData.ModModelledMarker, Is.EqualTo("OLD")); }); } + + [Test] + public void ApplyTo_RecordsAChangedMarker_SoTheRebuildResultCanBeUnderstood() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() { OwnerSoruceMappingID = (int)OwnerMappingSourceStm.NameField, ModModelledMarker = "FWOC" }; + handler.Init(configData); + + handler.ModelledMarker = "APP"; + handler.ApplyTo(configData); + + RuleOwnerMappingChange change = handler.AppliedChanges.Single(); + + Assert.Multiple(() => + { + Assert.That(change.Setting, Is.EqualTo(RuleOwnerMappingChangeSetting.kMarker)); + Assert.That(change.From, Is.EqualTo("FWOC")); + Assert.That(change.To, Is.EqualTo("APP")); + }); + } + + [Test] + public void ApplyTo_RecordsASwitchedSourceByItsName() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() { OwnerSoruceMappingID = (int)OwnerMappingSourceStm.IpBased }; + handler.Init(configData); + + handler.SelectSource(OwnerMappingSourceStm.NameField); + handler.ApplyTo(configData); + + RuleOwnerMappingChange change = handler.AppliedChanges.Single(); + + Assert.Multiple(() => + { + Assert.That(change.Setting, Is.EqualTo(RuleOwnerMappingChangeSetting.kSource)); + // the names resolve to localized texts in the display + Assert.That(change.From, Is.EqualTo(nameof(OwnerMappingSourceStm.IpBased))); + Assert.That(change.To, Is.EqualTo(nameof(OwnerMappingSourceStm.NameField))); + }); + } + + [Test] + public void ApplyTo_RecordsNothing_WhenNoMappingRelevantSettingChanged() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() { OwnerSoruceMappingID = (int)OwnerMappingSourceStm.NameField, ModModelledMarker = "FWOC" }; + handler.Init(configData); + + Assert.Multiple(() => + { + Assert.That(handler.ApplyTo(configData), Is.False); + Assert.That(handler.AppliedChanges, Is.Empty); + }); + } + + [Test] + public void AppliedChanges_SurviveAFailedSaveAndAreDroppedOnceTheRebuildIsDone() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() { OwnerSoruceMappingID = (int)OwnerMappingSourceStm.NameField, ModModelledMarker = "FWOC" }; + handler.Init(configData); + + handler.ModelledMarker = "APP"; + handler.ApplyTo(configData); + // a retry after a failed write must still name what was changed + handler.ApplyTo(configData); + + Assert.That(handler.AppliedChanges, Has.Count.EqualTo(1)); + + handler.ConfirmRuleOwnerRebuild(); + + Assert.That(handler.AppliedChanges, Is.Empty); + } + + [Test] + public void ApplyTo_RecordsChangedCustomFieldKeys() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() + { + OwnerSoruceMappingID = (int)OwnerMappingSourceStm.CustomField, + CustomFieldOwnerKey = @"[""owner""]" + }; + handler.Init(configData); + + handler.ActiveOwnerKey = "app_id"; + handler.AddOwnerKey(); + handler.Validate(); + handler.ApplyTo(configData); + + RuleOwnerMappingChange change = handler.AppliedChanges.Single(); + + Assert.Multiple(() => + { + Assert.That(change.Setting, Is.EqualTo(RuleOwnerMappingChangeSetting.kCustomFieldKeys)); + Assert.That(change.From, Is.EqualTo(@"[""owner""]")); + Assert.That(change.To, Does.Contain("app_id")); + }); + } } } diff --git a/roles/tests-unit/files/FWO.Test/RuleOwnerMappingLogLevelTest.cs b/roles/tests-unit/files/FWO.Test/RuleOwnerMappingLogLevelTest.cs new file mode 100644 index 0000000000..c656c46f14 --- /dev/null +++ b/roles/tests-unit/files/FWO.Test/RuleOwnerMappingLogLevelTest.cs @@ -0,0 +1,101 @@ +using FWO.Basics; +using FWO.Config.Api.Data; +using FWO.Data.Enums; +using FWO.Services; +using FWO.Ui.Services; +using NUnit.Framework; +using System.Collections.Generic; + +namespace FWO.Test +{ + /// + /// Covers the configurable logging of rule_owner mapping issues: which messages a level still lets + /// through, and how the setting travels between the editor and the configuration. + /// + [TestFixture] + [Parallelizable] + internal class RuleOwnerMappingLogLevelTest + { + [TestCase(RuleOwnerMappingLogLevel.None, false, false, false, false)] + [TestCase(RuleOwnerMappingLogLevel.Error, true, false, false, false)] + [TestCase(RuleOwnerMappingLogLevel.Warning, true, true, false, false)] + [TestCase(RuleOwnerMappingLogLevel.Info, true, true, true, false)] + [TestCase(RuleOwnerMappingLogLevel.Debug, true, true, true, true)] + public void IsEnabled_LetsThroughEverythingUpToTheConfiguredLevel(RuleOwnerMappingLogLevel configuredLevel, + bool error, bool warning, bool info, bool debug) + { + RuleOwnerMappingLogger logger = new(configuredLevel); + + Assert.Multiple(() => + { + Assert.That(logger.IsEnabled(RuleOwnerMappingLogLevel.Error), Is.EqualTo(error)); + Assert.That(logger.IsEnabled(RuleOwnerMappingLogLevel.Warning), Is.EqualTo(warning)); + Assert.That(logger.IsEnabled(RuleOwnerMappingLogLevel.Info), Is.EqualTo(info)); + Assert.That(logger.IsEnabled(RuleOwnerMappingLogLevel.Debug), Is.EqualTo(debug)); + }); + } + + [Test] + public void Default_KeepsTheBehaviourOfTheDefaultSetting() + { + // used where no configuration is reachable, so it must not start logging more than configured by default + Assert.Multiple(() => + { + Assert.That(RuleOwnerMappingLogger.Default.IsEnabled(RuleOwnerMappingLogLevel.Warning), Is.True); + Assert.That(RuleOwnerMappingLogger.Default.IsEnabled(RuleOwnerMappingLogLevel.Info), Is.False); + }); + } + + [Test] + public void ConfigData_DefaultsToWarning() + { + // an installation that never wrote the setting keeps logging unmappable rules as before + Assert.That(new ConfigData().RuleOwnerMappingLogLevel, Is.EqualTo(RuleOwnerMappingLogLevel.Warning)); + } + + [Test] + public void Handler_OffersEveryLevel() + { + OwnerMappingSourceHandler handler = new(); + + Assert.That(handler.LogLevels, Is.EquivalentTo(new List + { + RuleOwnerMappingLogLevel.None, + RuleOwnerMappingLogLevel.Error, + RuleOwnerMappingLogLevel.Warning, + RuleOwnerMappingLogLevel.Info, + RuleOwnerMappingLogLevel.Debug + })); + } + + [Test] + public void ApplyTo_WritesTheSelectedLevelWithoutRequestingARebuild() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() { OwnerSoruceMappingID = (int)OwnerMappingSourceStm.NameField }; + handler.Init(configData); + + handler.LogLevel = RuleOwnerMappingLogLevel.None; + bool rebuildRequired = handler.ApplyTo(configData); + + Assert.Multiple(() => + { + Assert.That(configData.RuleOwnerMappingLogLevel, Is.EqualTo(RuleOwnerMappingLogLevel.None)); + Assert.That(rebuildRequired, Is.False, "the log level does not influence the mappings, so no rebuild is needed"); + }); + } + + [Test] + public void DiscardEdits_RestoresTheStoredLevel() + { + OwnerMappingSourceHandler handler = new(); + ConfigData configData = new() { RuleOwnerMappingLogLevel = RuleOwnerMappingLogLevel.Debug }; + handler.Init(configData); + + handler.LogLevel = RuleOwnerMappingLogLevel.None; + handler.DiscardEdits(); + + Assert.That(handler.LogLevel, Is.EqualTo(RuleOwnerMappingLogLevel.Debug)); + } + } +} diff --git a/roles/tests-unit/files/FWO.Test/RuleOwnerMappingRunHandlerTest.cs b/roles/tests-unit/files/FWO.Test/RuleOwnerMappingRunHandlerTest.cs new file mode 100644 index 0000000000..c2b0bcf87d --- /dev/null +++ b/roles/tests-unit/files/FWO.Test/RuleOwnerMappingRunHandlerTest.cs @@ -0,0 +1,295 @@ +using FWO.Services; +using FWO.Ui.Services; +using NUnit.Framework; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace FWO.Test +{ + /// + /// Covers how a recorded rule owner mapping run is presented: which of the stored runs is shown, and + /// how its result has to be read - only a real deviation may be shown as a problem. + /// + [TestFixture] + [Parallelizable] + internal class RuleOwnerMappingRunHandlerTest + { + private static RuleOwnerMappingRun Run(long controlId, int added = 0, int removed = 0, + bool diffMeaningful = true, bool triggeredByChange = false, int runMinute = 0) + { + return new RuleOwnerMappingRun + { + RunTime = new DateTime(2026, 9, 16, 12, runMinute, 0, DateTimeKind.Utc), + ControlId = controlId, + MappingCount = 10, + AddedCount = added, + RemovedCount = removed, + Added = Enumerable.Range(1, added).Select(i => new RuleOwnerPair { RuleId = 100 + i, OwnerId = 1 }).ToList(), + Removed = Enumerable.Range(1, removed).Select(i => new RuleOwnerPair { RuleId = 200 + i, OwnerId = 2, Created = 7 }).ToList(), + DiffMeaningful = diffMeaningful, + TriggeredByChange = triggeredByChange + }; + } + + private static RuleOwnerMappingRunHistoryData History(params RuleOwnerMappingRun[] runsWithFindings) + { + return new RuleOwnerMappingRunHistoryData { RunsWithFindings = runsWithFindings.ToList() }; + } + + [Test] + public void Init_KeepsTheLastCleanRunApartFromTheFindings() + { + // the clean run must never be pushed out by newer findings - it answers "last verified correct" + RuleOwnerMappingRunHandler handler = new(); + handler.Init(new RuleOwnerMappingRunHistoryData + { + LastRunWithoutFindings = Run(99), + RunsWithFindings = [Run(30, added: 1)] + }); + + Assert.Multiple(() => + { + Assert.That(handler.LastRunWithoutFindings!.ControlId, Is.EqualTo(99)); + Assert.That(handler.Runs, Has.Count.EqualTo(1)); + Assert.That(handler.SelectedRun!.ControlId, Is.EqualTo(30)); + }); + } + + [Test] + public void SelectedRun_IsNull_WhenNothingWasRecordedYet() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(new RuleOwnerMappingRunHistoryData()); + + Assert.Multiple(() => + { + Assert.That(handler.SelectedRun, Is.Null); + Assert.That(handler.HasNewer, Is.False); + Assert.That(handler.HasOlder, Is.False); + Assert.That(handler.GetSelectedEntries(), Is.Empty); + }); + } + + [Test] + public void Init_ShowsTheNewestRun() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(30, added: 1), Run(20, added: 1), Run(10, added: 1))); + + Assert.Multiple(() => + { + Assert.That(handler.SelectedRun!.ControlId, Is.EqualTo(30)); + Assert.That(handler.HasNewer, Is.False, "the newest run has nothing newer"); + Assert.That(handler.HasOlder, Is.True); + }); + } + + [Test] + public void SelectOlderAndNewer_StepThroughTheRunsAndStopAtTheEnds() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(30, added: 1), Run(20, added: 1), Run(10, added: 1))); + + handler.SelectOlder(); + handler.SelectOlder(); + handler.SelectOlder(); + + Assert.That(handler.SelectedRun!.ControlId, Is.EqualTo(10), "stepping past the oldest run must not wrap around"); + + handler.SelectNewer(); + Assert.That(handler.SelectedRun!.ControlId, Is.EqualTo(20)); + + handler.SelectNewer(); + handler.SelectNewer(); + Assert.That(handler.SelectedRun!.ControlId, Is.EqualTo(30), "stepping past the newest run must not wrap around"); + } + + [Test] + public void GetSelectedState_ReportsInSync_WhenNothingChanged() + { + // a run without findings is not listed any more, but the state is still what describes it + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(10))); + + Assert.That(handler.GetSelectedState(), Is.EqualTo(RuleOwnerMappingRunState.InSync)); + } + + [Test] + public void GetSelectedState_ReportsDrift_WhenTheRebuiltStateDiffers() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(10, added: 2, removed: 1))); + + Assert.That(handler.GetSelectedState(), Is.EqualTo(RuleOwnerMappingRunState.Drift)); + } + + [Test] + public void GetSelectedState_DoesNotReportDrift_WhenImportsWerePending() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(10, added: 5, diffMeaningful: false))); + + Assert.That(handler.GetSelectedState(), Is.EqualTo(RuleOwnerMappingRunState.ImportsPending), + "an unprocessed backlog explains the difference on its own"); + } + + [Test] + public void GetSelectedState_DoesNotReportDrift_AfterADeliberateChange() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(10, added: 5, triggeredByChange: true))); + + Assert.That(handler.GetSelectedState(), Is.EqualTo(RuleOwnerMappingRunState.ChangeApplied), + "a deliberate change produces a different state by design"); + } + + [Test] + public void GetSelectedEntries_LabelsAddedAsMissingAndRemovedAsLeftOver() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(10, added: 2, removed: 1))); + + List entries = handler.GetSelectedEntries(); + + Assert.Multiple(() => + { + Assert.That(entries, Has.Count.EqualTo(3)); + Assert.That(entries.Where(entry => entry.Finding == RuleOwnerMappingFinding.Missing).Select(entry => entry.RuleId), + Is.EquivalentTo(new List { 101, 102 }), "added pairs were never created by the incremental mapping"); + Assert.That(entries.Where(entry => entry.Finding == RuleOwnerMappingFinding.Superfluous).Select(entry => entry.RuleId), + Is.EquivalentTo(new List { 201 }), "removed pairs were left behind by the incremental mapping"); + }); + } + + [Test] + public void GetSelectedEntries_MakeEveryRowFindableInRuleOwner() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(42, added: 1, removed: 1))); + + RuleOwnerMappingRunEntry missing = handler.GetSelectedEntries().Single(entry => entry.Finding == RuleOwnerMappingFinding.Missing); + RuleOwnerMappingRunEntry leftOver = handler.GetSelectedEntries().Single(entry => entry.Finding == RuleOwnerMappingFinding.Superfluous); + + Assert.Multiple(() => + { + Assert.That(missing.Created, Is.EqualTo(42), "the run established the missing mapping"); + Assert.That(missing.Removed, Is.Null, "it is active afterwards"); + Assert.That(leftOver.Created, Is.EqualTo(7), "the left over mapping came from an older import"); + Assert.That(leftOver.Removed, Is.EqualTo(42), "the run removed it"); + }); + } + + [Test] + public void GetSelectedEntries_FallBackToTheRunsControlId_WhenNoOriginWasRecorded() + { + // entries written before the origin was recorded carry no created value + RuleOwnerMappingRun run = Run(42, removed: 1); + run.Removed[0].Created = null; + + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(run)); + + Assert.That(handler.GetSelectedEntries().Single().Created, Is.EqualTo(42)); + } + + [Test] + public void GetStateStyle_MarksOnlyDriftAsCritical() + { + Assert.Multiple(() => + { + Assert.That(RuleOwnerMappingRunHandler.GetStateStyle(RuleOwnerMappingRunState.Drift), Is.EqualTo("danger")); + Assert.That(RuleOwnerMappingRunHandler.GetStateStyle(RuleOwnerMappingRunState.InSync), Is.EqualTo("success")); + Assert.That(RuleOwnerMappingRunHandler.GetStateStyle(RuleOwnerMappingRunState.ImportsPending), Is.EqualTo("warning")); + Assert.That(RuleOwnerMappingRunHandler.GetStateStyle(RuleOwnerMappingRunState.ChangeApplied), Is.EqualTo("secondary")); + }); + } + + [Test] + public void CurrentState_IsInSync_WhenTheNewestCheckFoundNothing() + { + // the listed runs are then already dealt with - without this the page would read as if the + // findings were current, because the history never shows the clean runs + RuleOwnerMappingRunHandler handler = new(); + handler.Init(new RuleOwnerMappingRunHistoryData + { + LastRunWithoutFindings = Run(50, runMinute: 37), + RunsWithFindings = [Run(45, added: 13, runMinute: 36)] + }); + + Assert.Multiple(() => + { + Assert.That(handler.CurrentState, Is.EqualTo(RuleOwnerMappingRunState.InSync)); + Assert.That(handler.SelectedRun!.AddedCount, Is.EqualTo(13), "the listed run stays readable as history"); + }); + } + + [Test] + public void CurrentState_IsDrift_WhenAFindingFollowedTheLastCleanCheck() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(new RuleOwnerMappingRunHistoryData + { + LastRunWithoutFindings = Run(50, runMinute: 30), + RunsWithFindings = [Run(55, added: 2, runMinute: 40)] + }); + + Assert.Multiple(() => + { + Assert.That(handler.CurrentState, Is.EqualTo(RuleOwnerMappingRunState.Drift)); + Assert.That(handler.GetSelectedState(), Is.EqualTo(RuleOwnerMappingRunState.Drift)); + }); + } + + [Test] + public void CurrentState_IsDrift_WhenNothingWasEverVerified() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(45, added: 1))); + + Assert.That(handler.CurrentState, Is.EqualTo(RuleOwnerMappingRunState.Drift)); + } + + [Test] + public void CurrentState_IsChangeApplied_WhenTheNewestRunFollowedADeliberateChange() + { + // a deliberate change makes the result differ on purpose, so the banner must not read as a problem + RuleOwnerMappingRunHandler handler = new(); + handler.Init(new RuleOwnerMappingRunHistoryData + { + LastRunWithoutFindings = Run(50, runMinute: 30), + RunsWithFindings = [Run(55, added: 13, triggeredByChange: true, runMinute: 40)] + }); + + Assert.That(handler.CurrentState, Is.EqualTo(RuleOwnerMappingRunState.ChangeApplied)); + } + + [Test] + public void CurrentState_IsImportsPending_WhenTheNewestRunCouldNotJudge() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(55, added: 2, diffMeaningful: false, runMinute: 40))); + + Assert.That(handler.CurrentState, Is.EqualTo(RuleOwnerMappingRunState.ImportsPending)); + } + + [Test] + public void CurrentState_IsNull_WhenNothingWasRecordedAtAll() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(new RuleOwnerMappingRunHistoryData()); + + Assert.That(handler.CurrentState, Is.Null); + } + + [Test] + public void GetSelectedState_ReportsInSync_WhenAChangeTurnedOutToHaveNoEffect() + { + RuleOwnerMappingRunHandler handler = new(); + handler.Init(History(Run(10, triggeredByChange: true))); + + Assert.That(handler.GetSelectedState(), Is.EqualTo(RuleOwnerMappingRunState.InSync), + "no difference is the strongest statement, whatever triggered the run"); + } + } +} diff --git a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs new file mode 100644 index 0000000000..794af6637e --- /dev/null +++ b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs @@ -0,0 +1,679 @@ +using FWO.Api.Client; +using FWO.Api.Client.Queries; +using FWO.Basics; +using FWO.Config.Api; +using FWO.Config.Api.Data; +using FWO.Data; +using FWO.Services; +using FWO.Services.EventMediator.Events; +using NUnit.Framework; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using System.Threading.Tasks; + +namespace FWO.Test +{ + /// + /// Regression tests for the incremental rule_owner mapping. The simulated API mirrors two properties of + /// the real backend that the older simulations did not: the owner and rule queries always return the + /// current state regardless of which import is being processed, and an insert that would create a second + /// active mapping for the same (rule_id, owner_id) pair fails like the partial unique index does. + /// + [TestFixture] + public class UpdateRuleOwnerMappingIncrementalTest + { + private const string kCustomFieldOwnerKey = @"[""owner""]"; + private const long kRuleId = 101; + private const int kOwnerId = 1; + + private static GlobalConfig CustomFieldConfig() + { + return new GlobalConfig { CustomFieldOwnerKey = kCustomFieldOwnerKey }; + } + + [Test] + public async Task RunAsync_ShouldSkipStillActiveMapping_WhenOwnerImportFollowsRuleImport() + { + // the rule import maps the rule to the owner because the owner already exists by the time the + // pending import is processed; the owner insert then rebuilds the very same pair without + // removing anything first, which used to collide with the partial unique index + RuleOwnerMappingFake apiConnection = new(); + apiConnection.AddPendingImport(1, ImportType.RULE); + apiConnection.AddPendingImport(2, ImportType.OWNER); + apiConnection.AddRuleChange(1, ChangelogActionType.INSERT, kRuleId); + apiConnection.AddOwnerChange(2, ChangelogActionType.INSERT, kOwnerId); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + bool result = await service.RunAsync(); + + Assert.Multiple(() => + { + Assert.That(result, Is.True, "both imports should be processed without error"); + Assert.That(apiConnection.CompletedImports, Is.EquivalentTo(new List { 1, 2 }), "the owner import must not stay pending"); + Assert.That(apiConnection.ActivePairs, Is.EquivalentTo(new List { "101->1" }), "the rule must keep exactly one active mapping"); + Assert.That(apiConnection.RaisedAlerts, Is.Empty, "a clean run must not raise an alert"); + }); + } + + [Test] + public async Task RunAsync_ShouldStillProcessLaterImports_WhenOneImportFails() + { + RuleOwnerMappingFake apiConnection = new(); + apiConnection.AddPendingImport(1, ImportType.RULE); + apiConnection.AddPendingImport(2, ImportType.RULE); + apiConnection.AddRuleChange(2, ChangelogActionType.INSERT, kRuleId); + apiConnection.FailRuleChangeLookupForImport = 1; + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + bool result = await service.RunAsync(); + + Assert.Multiple(() => + { + Assert.That(result, Is.False, "a failed import must not be reported as success"); + Assert.That(apiConnection.CompletedImports, Does.Contain(2L), "the healthy import behind the broken one must still be processed"); + Assert.That(apiConnection.CompletedImports, Does.Not.Contain(1L), "the failed import has to stay pending for the next run"); + }); + } + + [Test] + public async Task RunAsync_ShouldRaiseAlert_WhenImportFails() + { + RuleOwnerMappingFake apiConnection = new(); + apiConnection.AddPendingImport(1, ImportType.RULE); + apiConnection.FailRuleChangeLookupForImport = 1; + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(); + + Assert.That(apiConnection.RaisedAlerts, Has.Exactly(1).Contains("import_control 1")); + } + + [Test] + public async Task RunAsync_ShouldRemoveObsoleteMappingsAndAlert_WhenFullReinitializeMatchesNothing() + { + // the configured mapping source stops matching, for instance after the custom field key changed: + // that is a valid result and the obsolete mappings have to go + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(kRuleId, kOwnerId, 50); + apiConnection.ClearOwners(); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + bool result = await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + + Assert.Multiple(() => + { + Assert.That(result, Is.True, "an empty result is valid and must not be reported as failure"); + Assert.That(apiConnection.ActivePairs, Is.Empty, "the obsolete mappings must be removed"); + Assert.That(apiConnection.RaisedAlerts, Has.Exactly(1).Contains("matched no rule")); + }); + } + + [Test] + public async Task RunAsync_ShouldRecordDriftAndAlert_WhenFullReinitializeChangesMappingsWithoutPendingImports() + { + // a stale mapping the rebuild does not produce any more: with a correct incremental mapping it + // would already be gone, so the full reinitialize finding it means the incremental path missed it + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(999, kOwnerId, 50); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + + List runs = apiConnection.StoredRuns; + + Assert.Multiple(() => + { + Assert.That(runs, Has.Count.EqualTo(1), "the run has to be recorded in the history"); + Assert.That(runs[0].DiffMeaningful, Is.True, "no import was pending, so the difference is drift"); + Assert.That(runs[0].AddedCount, Is.EqualTo(1)); + Assert.That(runs[0].Added.Single().RuleId, Is.EqualTo(kRuleId)); + Assert.That(runs[0].RemovedCount, Is.EqualTo(1)); + Assert.That(runs[0].Removed.Single().RuleId, Is.EqualTo(999)); + Assert.That(apiConnection.RaisedAlerts, Has.Exactly(1).Contains("incremental mapping missed")); + }); + } + + [Test] + public async Task RunAsync_ShouldNotReportDrift_WhenTheRunFollowsADeliberateChange() + { + // same starting point as the drift test above, but triggered by saving a changed mapping + // configuration: the rebuilt state differs by design, so it is not the incremental mapping's fault + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(999, kOwnerId, 50); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true, TriggeredByChange = true }); + + List runs = apiConnection.StoredRuns; + + Assert.Multiple(() => + { + Assert.That(runs[0].TriggeredByChange, Is.True, "the run has to be marked so the difference can be read correctly"); + Assert.That(runs[0].AddedCount, Is.EqualTo(1), "the change is still recorded in full"); + Assert.That(runs[0].RemovedCount, Is.EqualTo(1)); + Assert.That(apiConnection.RaisedAlerts, Has.None.Contains("incremental mapping missed"), + "a deliberate configuration change must not be reported as drift"); + }); + } + + [Test] + public async Task RunAsync_ShouldKeepRunsWithoutFindingsOutOfTheLimitedHistory() + { + // pressing the rebuild button repeatedly must not push runs that did find something out of the + // history, so a run without findings only updates "last verified correct" + RuleOwnerMappingFake apiConnection = new(); + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + + RuleOwnerMappingRunHistoryData history = apiConnection.StoredHistory; + + Assert.Multiple(() => + { + Assert.That(history.RunsWithFindings, Has.Count.EqualTo(1), "only the first run found something"); + Assert.That(history.RunsWithFindings[0].AddedCount, Is.EqualTo(1)); + Assert.That(history.LastRunWithoutFindings, Is.Not.Null, "the later clean runs are recorded separately"); + }); + } + + [Test] + public async Task RunAsync_ShouldMarkDiffAsNotMeaningful_WhenImportsWereStillPending() + { + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(999, kOwnerId, 50); + apiConnection.AddPendingImport(7, ImportType.RULE); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + + List runs = apiConnection.StoredRuns; + + Assert.Multiple(() => + { + Assert.That(runs[0].DiffMeaningful, Is.False, "a pending backlog explains the difference, it is not drift"); + Assert.That(runs[0].PendingImportsBefore, Is.EquivalentTo(new List { 7 }), "the unprocessed imports have to be documented"); + Assert.That(apiConnection.RaisedAlerts, Has.None.Contains("incremental mapping missed"), "drift must not be reported while imports are pending"); + }); + } + + [Test] + public async Task RunAsync_ShouldKeepOnlyTheCounts_WhenTheMappingSourceWasSwitched() + { + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(999, kOwnerId, 50); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs + { + isFullReInitialize = true, + TriggeredByChange = true, + Changes = [new RuleOwnerMappingChange { Setting = RuleOwnerMappingChangeSetting.kSource, From = "IpBased", To = "CustomField" }] + }); + + RuleOwnerMappingRun run = apiConnection.StoredHistory.RunsWithFindings.Single(); + + Assert.Multiple(() => + { + Assert.That(run.AddedCount, Is.EqualTo(1), "the counts stay complete"); + Assert.That(run.RemovedCount, Is.EqualTo(1)); + Assert.That(run.Added, Is.Empty, "a source switch replaces every mapping, listing them says nothing"); + Assert.That(run.Removed, Is.Empty); + Assert.That(run.PairListsTruncated, Is.False, "the lists were left out on purpose, not cut off"); + }); + } + + [Test] + public async Task RunAsync_ShouldKeepThePairLists_WhenOnlyTheMarkerWasChanged() + { + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(999, kOwnerId, 50); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs + { + isFullReInitialize = true, + TriggeredByChange = true, + Changes = [new RuleOwnerMappingChange { Setting = RuleOwnerMappingChangeSetting.kMarker, From = "FWOC", To = "APP" }] + }); + + RuleOwnerMappingRun run = apiConnection.StoredHistory.RunsWithFindings.Single(); + + Assert.Multiple(() => + { + Assert.That(run.Added, Has.Count.EqualTo(1)); + Assert.That(run.Removed, Has.Count.EqualTo(1)); + }); + } + + [Test] + public async Task RunAsyncDisabled_ShouldRecordTheRunWithItsChangeNote() + { + // switching the mapping off removes everything, and that belongs in the history just like any + // other rebuild - otherwise nothing explains why the mapping table is empty + RuleOwnerMappingFake apiConnection = new(); + apiConnection.SeedActiveMapping(kRuleId, kOwnerId, 50); + + UpdateRuleOwnerMappingDisabled service = new(apiConnection, CustomFieldConfig()); + + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs + { + isFullReInitialize = true, + TriggeredByChange = true, + Changes = [new RuleOwnerMappingChange { Setting = RuleOwnerMappingChangeSetting.kSource, From = "NameField", To = "Disabled" }] + }); + + RuleOwnerMappingRun run = apiConnection.StoredHistory.RunsWithFindings.Single(); + + Assert.Multiple(() => + { + Assert.That(run.RemovedCount, Is.EqualTo(1), "the removed mappings are counted"); + Assert.That(run.MappingCount, Is.EqualTo(0), "nothing is mapped afterwards"); + Assert.That(run.TriggeredByChange, Is.True, "switching off is deliberate and must not read as drift"); + Assert.That(run.Changes.Single().To, Is.EqualTo("Disabled"), "the change note has to survive this path too"); + Assert.That(apiConnection.RaisedAlerts, Is.Empty, "a deliberate switch off raises no alert"); + }); + } + + [Test] + public async Task RunAsync_ShouldRecordAChangeEvenWhenItHadNoEffect() + { + // that an edited setting changed nothing is exactly what somebody who just edited it needs to + // see - without this the change note would vanish and only the clean stamp would be refreshed + RuleOwnerMappingFake apiConnection = new(); + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + // first run establishes the mapping, the second changes a setting without any effect + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs + { + isFullReInitialize = true, + TriggeredByChange = true, + Changes = [new RuleOwnerMappingChange { Setting = RuleOwnerMappingChangeSetting.kCustomFieldKeys, From = @"[""owner""]", To = @"[""owner"",""unused""]" }] + }); + + RuleOwnerMappingRunHistoryData history = apiConnection.StoredHistory; + RuleOwnerMappingRun changeRun = history.RunsWithFindings[0]; + + Assert.Multiple(() => + { + Assert.That(changeRun.TriggeredByChange, Is.True); + Assert.That(changeRun.AddedCount + changeRun.RemovedCount, Is.EqualTo(0), "the change had no effect"); + Assert.That(changeRun.Changes.Single().To, Does.Contain("unused"), "the note survives all the same"); + Assert.That(history.LastRunWithoutFindings, Is.Not.Null, "it still counts as a verification"); + }); + } + + [Test] + public async Task RunAsync_ShouldNotClaimAVerification_WhenImportsWerePending() + { + // such a run cannot judge anything, so it must not refresh "last verified without deviation" + RuleOwnerMappingFake apiConnection = new(); + apiConnection.AddPendingImport(7, ImportType.OWNER); + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + await service.RunAsync(new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true }); + + Assert.That(apiConnection.StoredHistory.LastRunWithoutFindings, Is.Null); + } + + /// + /// Simulated API connection for the CustomField mapping source. + /// + private sealed class RuleOwnerMappingFake : SimulatedApiConnection + { + private static readonly ReturnId[] AlertReturnIds = [new ReturnId { NewIdLong = 1 }]; + + private readonly List pendingImports = []; + private readonly Dictionary> ruleChangesByImport = []; + private readonly Dictionary> ownerChangesByImport = []; + private readonly List activeRuleOwners = []; + private readonly List rules = [CreateRule(kRuleId)]; + private List owners = [new() { Id = kOwnerId, ExtAppId = "A" }]; + + public List CompletedImports { get; } = []; + public List RaisedAlerts { get; } = []; + public long? FailRuleChangeLookupForImport { get; set; } + public string? StoredHistoryJson { get; private set; } + + public RuleOwnerMappingRunHistoryData StoredHistory => StoredHistoryJson == null + ? new RuleOwnerMappingRunHistoryData() + : JsonSerializer.Deserialize(StoredHistoryJson) ?? new RuleOwnerMappingRunHistoryData(); + + public List StoredRuns + { + get + { + RuleOwnerMappingRunHistoryData history = StoredHistory; + return history.LastRunWithoutFindings == null + ? history.RunsWithFindings + : [.. history.RunsWithFindings, history.LastRunWithoutFindings]; + } + } + + public List ActivePairs => activeRuleOwners.Select(ruleOwner => $"{ruleOwner.RuleId}->{ruleOwner.OwnerId}").OrderBy(pair => pair).ToList(); + + public void AddPendingImport(long controlId, int importTypeId) + { + pendingImports.Add(new ImportControl { ControlId = controlId, ImportTypeId = importTypeId }); + } + + public void AddRuleChange(long controlId, char action, long ruleId) + { + ruleChangesByImport[controlId] = [new RuleChange { ChangeAction = action, NewRule = CreateRule(ruleId) }]; + } + + public void AddOwnerChange(long controlId, char action, int ownerId) + { + ownerChangesByImport[controlId] = [new OwnerChange { ChangeAction = action, NewOwner = owners.First(owner => owner.Id == ownerId) }]; + } + + public void SeedActiveMapping(long ruleId, int ownerId, long created) + { + activeRuleOwners.Add(new RuleOwner { RuleId = ruleId, OwnerId = ownerId, Created = created }); + } + + public void ClearOwners() + { + owners = []; + } + + public override Task SendQueryAsync(string query, object? variables = null, string? operationName = null, QueryChunkingOptions? chunkingOptions = null) + { + return Task.FromResult(Handle(query, variables)); + } + + private QueryResponseType Handle(string query, object? variables) + { + if (TryHandleImportQuery(query, variables, out object? importResult)) + { + return (QueryResponseType)importResult!; + } + + if (TryHandleMappingInputQuery(query, variables, out object? inputResult)) + { + return (QueryResponseType)inputResult!; + } + + if (TryHandleAlertQuery(query, variables, out object? alertResult)) + { + return (QueryResponseType)alertResult!; + } + + if (TryHandleConfigQuery(query, variables, out object? configResult)) + { + return (QueryResponseType)configResult!; + } + + if (TryHandleMappingWriteQuery(query, variables, out object? writeResult)) + { + return writeResult == null ? default! : (QueryResponseType)writeResult; + } + + throw new InvalidOperationException($"Unexpected query: {query}"); + } + + private bool TryHandleImportQuery(string query, object? variables, out object? result) + { + result = null; + + if (query == ImportQueries.getPendingRuleOwnerImports) + { + result = pendingImports.Where(import => !CompletedImports.Contains(import.ControlId)).ToList(); + return true; + } + + if (query == ImportQueries.addImportForRuleOwner) + { + result = new InsertImportControl { Returning = [new ImportControl { ControlId = 999 }] }; + return true; + } + + if (query == ImportQueries.updateImportControlForRuleOwnerInc || query == ImportQueries.updateImportControlForRuleOwnerFull) + { + CompletedImports.Add(ReadLong(variables, "controlId")); + result = new ImportControl(); + return true; + } + + return false; + } + + private bool TryHandleMappingInputQuery(string query, object? variables, out object? result) + { + result = null; + + if (query == RuleQueries.getChangedRulesForRuleOwnerMappingCustomField) + { + long controlId = ReadLong(variables, "controlId"); + if (FailRuleChangeLookupForImport == controlId) + { + throw new InvalidOperationException($"Simulated API failure for import_control {controlId}."); + } + result = ruleChangesByImport.TryGetValue(controlId, out List? changes) ? changes : new List(); + return true; + } + + if (query == OwnerQueries.getChangedOwnersForRuleOwnerMappingCustomField) + { + long controlId = ReadLong(variables, "controlId"); + result = ownerChangesByImport.TryGetValue(controlId, out List? changes) ? changes : new List(); + return true; + } + + // both queries always return the current state, exactly like the real API does + if (query == OwnerQueries.getOwnersForRuleOwnerCustomField) + { + result = owners.ToList(); + return true; + } + + if (query == RuleQueries.getRulesForRuleOwnerCustomField || query == RuleQueries.getRulesForOwnerMappingCustomField) + { + result = rules.ToList(); + return true; + } + + if (query == OwnerQueries.getActiveRuleOwners) + { + result = activeRuleOwners.Select(Clone).ToList(); + return true; + } + + if (query == OwnerQueries.getRuleOwnerToRemoveByRule) + { + List ruleIds = ReadList(variables, "ruleIds"); + result = activeRuleOwners.Where(ruleOwner => ruleIds.Contains(ruleOwner.RuleId)).Select(Clone).ToList(); + return true; + } + + if (query == OwnerQueries.getRuleOwnerToRemoveByOwner) + { + List ownerIds = ReadList(variables, "ownerIds"); + result = activeRuleOwners.Where(ruleOwner => ownerIds.Contains(ruleOwner.OwnerId)).Select(Clone).ToList(); + return true; + } + + return false; + } + + private bool TryHandleMappingWriteQuery(string query, object? variables, out object? result) + { + result = null; + + if (query == OwnerQueries.setAllActiveRuleOwnersRemoved) + { + // returning delivers the state that was just replaced, like the real mutation does + result = new UpdateRuleOwner { AffectedRows = activeRuleOwners.Count, Returning = activeRuleOwners.Select(Clone).ToList() }; + activeRuleOwners.Clear(); + return true; + } + + if (query == OwnerQueries.setAffectedRuleOwnersRemoved) + { + RemoveAffectedRuleOwners(variables); + return true; + } + + if (query == OwnerQueries.insertRuleOwners) + { + InsertRuleOwners(variables); + return true; + } + + return false; + } + + private bool TryHandleConfigQuery(string query, object? variables, out object? result) + { + result = null; + + if (query == ConfigQueries.getConfigItemByKey) + { + result = StoredHistoryJson == null ? new List() : new List { new() { Value = StoredHistoryJson } }; + return true; + } + + if (query == ConfigQueries.upsertConfigItem) + { + StoredHistoryJson = ReadString(variables, "config_value"); + result = new object(); + return true; + } + + return false; + } + + private bool TryHandleAlertQuery(string query, object? variables, out object? result) + { + result = null; + + if (query == MonitorQueries.getOpenAlerts) + { + result = new List(); + return true; + } + + if (query == MonitorQueries.addLogEntry) + { + result = new ReturnIdWrapper { ReturnIds = AlertReturnIds }; + return true; + } + + if (query == MonitorQueries.addAlert) + { + RaisedAlerts.Add(ReadString(variables, "description")); + result = new ReturnIdWrapper { ReturnIds = AlertReturnIds }; + return true; + } + + if (query == MonitorQueries.acknowledgeAlert) + { + result = new ReturnId(); + return true; + } + + return false; + } + + private void InsertRuleOwners(object? variables) + { + foreach (RuleOwner ruleOwner in ReadList(variables, "objects")) + { + if (activeRuleOwners.Any(existing => existing.RuleId == ruleOwner.RuleId && existing.OwnerId == ruleOwner.OwnerId)) + { + // mirrors idx_rule_owner_removed_is_null_unique, which on_conflict on pk_rule_owner cannot catch + throw new InvalidOperationException("Uniqueness violation. duplicate key value violates unique constraint " + + $"\"idx_rule_owner_removed_is_null_unique\". Key (rule_id, owner_id)=({ruleOwner.RuleId}, {ruleOwner.OwnerId}) already exists."); + } + activeRuleOwners.Add(Clone(ruleOwner)); + } + } + + private void RemoveAffectedRuleOwners(object? variables) + { + object? objects = variables?.GetType().GetProperty("objects")?.GetValue(variables); + if (objects is not System.Collections.IEnumerable entries) + { + return; + } + + foreach (object entry in entries) + { + long ruleId = ReadNestedLong(entry, "rule_id"); + int ownerId = (int)ReadNestedLong(entry, "owner_id"); + long created = ReadNestedLong(entry, "created"); + activeRuleOwners.RemoveAll(ruleOwner => ruleOwner.RuleId == ruleId && ruleOwner.OwnerId == ownerId && ruleOwner.Created == created); + } + } + + private static Rule CreateRule(long ruleId) + { + return new Rule + { + Id = ruleId, + CustomFields = "{'owner':'A'}", + Metadata = new RuleMetadata { Id = ruleId + 1000 } + }; + } + + private static RuleOwner Clone(RuleOwner source) + { + return new RuleOwner + { + RuleId = source.RuleId, + OwnerId = source.OwnerId, + Created = source.Created, + RuleMetadataId = source.RuleMetadataId, + OwnerMappingSourceId = source.OwnerMappingSourceId + }; + } + + private static long ReadLong(object? variables, string propertyName) + { + object? value = variables?.GetType().GetProperty(propertyName)?.GetValue(variables); + return value switch + { + long longValue => longValue, + int intValue => intValue, + _ => throw new InvalidOperationException($"Missing long property '{propertyName}'.") + }; + } + + private static string ReadString(object? variables, string propertyName) + { + return variables?.GetType().GetProperty(propertyName)?.GetValue(variables) as string ?? ""; + } + + private static List ReadList(object? variables, string propertyName) + { + object? value = variables?.GetType().GetProperty(propertyName)?.GetValue(variables); + return value as List ?? []; + } + + private static long ReadNestedLong(object source, string propertyName) + { + object? wrapper = source.GetType().GetProperty(propertyName)?.GetValue(source); + object? eqValue = wrapper?.GetType().GetProperty("_eq")?.GetValue(wrapper); + return eqValue switch + { + long longValue => longValue, + int intValue => intValue, + _ => throw new InvalidOperationException($"Missing nested _eq value for '{propertyName}'.") + }; + } + } + } +} diff --git a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingTests.cs b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingTests.cs index 0a23dacb4d..12ffdbbfa6 100644 --- a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingTests.cs +++ b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingTests.cs @@ -473,12 +473,20 @@ public async Task RunAsyncCustomField_ShouldReachSameInitialMapping_RegardlessOf } [Test] - public void RunAsyncCustomField_ShouldNotTryToInsertDuplicateActiveRuleOwnerPair() + public async Task RunAsyncCustomField_ShouldNotTryToInsertDuplicateActiveRuleOwnerPair() { DuplicateInsertGuardCustomFieldApiConnection apiConnection = new(); UpdateRuleOwnerMappingCustomField service = new(apiConnection, new GlobalConfig { CustomFieldOwnerKey = @"[""owner""]" }); - Assert.DoesNotThrowAsync(async () => await service.RunAsync(), "Incremental mapping should remove the active pair before re-inserting it."); + // RunIncremental catches every exception, so asserting that RunAsync does not throw would pass + // even when the insert collided - the completed imports are what shows whether it really worked + bool result = await service.RunAsync(); + + Assert.Multiple(() => + { + Assert.That(result, Is.True, "Incremental mapping should remove the active pair before re-inserting it."); + Assert.That(apiConnection.CompletedImports, Is.EquivalentTo(new List { 1, 2, 3 }), "Every pending import has to be completed."); + }); } [Test] @@ -1194,8 +1202,8 @@ private sealed class InitialOrderCustomFieldApiConnection : SimulatedApiConnecti private readonly List pendingImports; private readonly Dictionary> ruleChangesByImport; private readonly Dictionary> ownerChangesByImport; - private readonly Dictionary> rulesByImport; - private readonly Dictionary> ownersByImport; + private readonly List allRules; + private readonly List allOwners; private readonly List activeRuleOwners = []; private long currentImportId; @@ -1260,29 +1268,10 @@ public InitialOrderCustomFieldApiConnection(bool ruleImportFirst) [2] = [] }; - rulesByImport = ruleImportFirst - ? new() - { - [1] = BuildRules(rules), - [2] = BuildRules(rules) - } - : new() - { - [1] = [], - [2] = BuildRules(rules) - }; - - ownersByImport = ruleImportFirst - ? new() - { - [1] = [], - [2] = BuildOwners(owners) - } - : new() - { - [1] = BuildOwners(owners), - [2] = BuildOwners(owners) - }; + // getRulesForRuleOwnerCustomField and getOwnersForRuleOwnerCustomField read the live state + // in production - neither is filtered by the import that is currently being processed + allRules = BuildRules(rules); + allOwners = BuildOwners(owners); } public override Task SendQueryAsync(string query, object? variables = null, string? operationName = null, QueryChunkingOptions? chunkingOptions = null) @@ -1306,12 +1295,12 @@ public override Task SendQueryAsync(string if (query == FWO.Api.Client.Queries.OwnerQueries.getOwnersForRuleOwnerCustomField) { - return Task.FromResult((QueryResponseType)(object)ownersByImport[currentImportId]); + return Task.FromResult((QueryResponseType)(object)allOwners.ToList()); } if (query == FWO.Api.Client.Queries.RuleQueries.getRulesForRuleOwnerCustomField) { - return Task.FromResult((QueryResponseType)(object)rulesByImport[currentImportId]); + return Task.FromResult((QueryResponseType)(object)allRules.ToList()); } if (query == FWO.Api.Client.Queries.OwnerQueries.getRuleOwnerToRemoveByRule) @@ -1483,13 +1472,14 @@ private sealed class DuplicateInsertGuardCustomFieldApiConnection : SimulatedApi Metadata = new RuleMetadata { Id = 1101 } }; private readonly FwoOwner trackedOwner = new() { Id = 1, ExtAppId = "A" }; - private readonly List completedImports = []; + + public List CompletedImports { get; } = []; public override Task SendQueryAsync(string query, object? variables = null, string? operationName = null, QueryChunkingOptions? chunkingOptions = null) { if (query == FWO.Api.Client.Queries.ImportQueries.getPendingRuleOwnerImports) { - return Task.FromResult((QueryResponseType)(object)pendingImports.Where(import => !completedImports.Contains(import.ControlId)).ToList()); + return Task.FromResult((QueryResponseType)(object)pendingImports.Where(import => !CompletedImports.Contains(import.ControlId)).ToList()); } if (query == FWO.Api.Client.Queries.RuleQueries.getChangedRulesForRuleOwnerMappingCustomField) @@ -1556,7 +1546,7 @@ public override Task SendQueryAsync(string if (query == FWO.Api.Client.Queries.ImportQueries.updateImportControlForRuleOwnerInc) { - completedImports.Add(ReadLong(variables, "controlId")); + CompletedImports.Add(ReadLong(variables, "controlId")); return Task.FromResult(default(QueryResponseType)!); } diff --git a/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringRuleOwnerMapping.cshtml b/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringRuleOwnerMapping.cshtml new file mode 100644 index 0000000000..2a53fb7f25 --- /dev/null +++ b/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringRuleOwnerMapping.cshtml @@ -0,0 +1,25 @@ +@page "/help/monitoring/rule_owner_mapping" +@model FWO.Ui.Pages.Help.MainModel +@{ + Layout = "HelpLayout"; +} +@section sidebar{ + @{ + await Html.RenderPartialAsync("HelpMonitoringSidebar.cshtml"); + } +} +@using FWO.Config.Api +@inject UserConfig userConfig + +
    +

    @userConfig.GetText("rule_owner_mapping_runs")

    + @(Html.Raw(userConfig.GetText("H7261"))) +

    +
      +
    • @(Html.Raw(userConfig.GetText("H7262")))
      • +
    • @(Html.Raw(userConfig.GetText("H7263")))
      • +
    • @(Html.Raw(userConfig.GetText("H7264")))
      • +
    • @(Html.Raw(userConfig.GetText("H7265")))
      • +
    • @(Html.Raw(userConfig.GetText("H7276")))
      • +
    +
    diff --git a/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringSidebar.cshtml b/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringSidebar.cshtml index 13cd7c87ae..8649584739 100644 --- a/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringSidebar.cshtml +++ b/roles/ui/files/FWO.UI/Pages/Help/HelpMonitoringSidebar.cshtml @@ -29,6 +29,9 @@ @(userConfig.GetText("daily_checks")) + + @(userConfig.GetText("rule_owner_mapping_runs")) +

    @(userConfig.GetText("import"))
    diff --git a/roles/ui/files/FWO.UI/Pages/Help/HelpSettingsOwners.cshtml b/roles/ui/files/FWO.UI/Pages/Help/HelpSettingsOwners.cshtml index 98a6d8a6c8..fc2f32583a 100644 --- a/roles/ui/files/FWO.UI/Pages/Help/HelpSettingsOwners.cshtml +++ b/roles/ui/files/FWO.UI/Pages/Help/HelpSettingsOwners.cshtml @@ -40,5 +40,6 @@
  • @(Html.Raw(userConfig.GetText("H5915")))
  • @(Html.Raw(userConfig.GetText("H5912")))
  • @(Html.Raw(userConfig.GetText("H5913")))
    • +
  • @(Html.Raw(userConfig.GetText("H5917")))
    • diff --git a/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor b/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor new file mode 100644 index 0000000000..295d4d896c --- /dev/null +++ b/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor @@ -0,0 +1,258 @@ +@using FWO.Services +@using FWO.Ui.Services + +@inject ApiConnection apiConnection +@inject UserConfig userConfig + +@page "/monitoring/rule_owner_mapping" +@attribute [Authorize(Roles = $"{Roles.Admin}, {Roles.FwAdmin}, {Roles.Auditor}")] + +
    +

    @(userConfig.GetText("rule_owner_mapping_runs"))

    + +
    +@(userConfig.GetText("U7550")) +
    + +@if (!InitComplete) +{ + +} +else +{ + @* the history below only holds runs that found something, so it cannot show how the mapping stands + right now - that is what this banner is for, in the colour of the newest run's own state *@ + @if (Handler.CurrentState != null) + { +
    + @(userConfig.GetText($"rule_owner_current_{Handler.CurrentState}")) + @if (Handler.LastRunWithoutFindings != null) + { + – @(userConfig.GetText("rule_owner_last_clean_run")): + @(Handler.LastRunWithoutFindings.RunTime.ToLocalTime().ToString("g")) + (@(userConfig.GetText("import_id")) @(Handler.LastRunWithoutFindings.ControlId)) + } +
    + } + + @if (Handler.SelectedRun == null) + { + @* nothing recorded at all reads differently from recorded and never found anything *@ +
    + @(userConfig.GetText(Handler.LastRunWithoutFindings == null ? RuleOwnerMappingRunHandler.kNoHistoryText : "U7553")) +
    + } +} + +@if (InitComplete && Handler.SelectedRun != null) +{ +
    @(userConfig.GetText("rule_owner_runs_with_findings"))
    +
    + + +
    + @(userConfig.GetText("run")) @(Handler.SelectedIndex + 1) / @(Handler.Runs.Count) + +
    +
    + + + + + + + @if (Handler.SelectedRun.Changes.Count > 0) + { + + + + + } + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    @(userConfig.GetText("state")):@(GetStateText(State))
    @(userConfig.GetText("rule_owner_applied_changes")): + @foreach (RuleOwnerMappingChange change in Handler.SelectedRun.Changes) + { +
    @(DescribeChange(change))
    + } +
    @(userConfig.GetText("timestamp")):@(Handler.SelectedRun.RunTime.ToLocalTime().ToString("g"))
    @(userConfig.GetText("import_id")):@(Handler.SelectedRun.ControlId)
    @(userConfig.GetText("source")):@(userConfig.GetText(Handler.SelectedRun.MappingSource))
    @(userConfig.GetText("rule_owner_mappings_total")):@(Handler.SelectedRun.MappingCount)
    @(GetAddedLabel()):@(Handler.SelectedRun.AddedCount)
    @(GetRemovedLabel()):@(Handler.SelectedRun.RemovedCount)
    @(userConfig.GetText("rule_owner_pending_imports")): + @if (Handler.SelectedRun.PendingImportsBefore.Count == 0) + { + @(userConfig.GetText("none")) + } + else + { + @(string.Join(", ", Handler.SelectedRun.PendingImportsBefore)) + } +
    +
    +
    + + @if (Handler.SelectedRun.PairListsTruncated) + { +
    @(userConfig.GetText("U7552"))
    + } + + @if (Entries.Count == 0) + { + @* a source switch replaces every mapping, so only the counts are kept *@ +
    @(userConfig.GetText("U7554"))
    + } + else + { +
    + + + + + + + + + + + +
    +
    + } +} + +@code +{ + [CascadingParameter] + Action DisplayMessageInUi { get; set; } = DefaultInit.DoNothing; + + private readonly RuleOwnerMappingRunHandler Handler = new(); + private bool InitComplete = false; + + private RuleOwnerMappingRunState State => Handler.GetSelectedState(); + private List Entries => Handler.GetSelectedEntries(); + + protected override async Task OnInitializedAsync() + { + try + { + Handler.Init(await new RuleOwnerMappingRunHistory(apiConnection).Load()); + } + catch (Exception exception) + { + DisplayMessageInUi(exception, userConfig.GetText("rule_owner_mapping_runs"), "", true); + } + finally + { + InitComplete = true; + } + } + + /// + /// Resolves the display text of a run state. + /// + /// State to display. + /// The localized text. + private string GetStateText(RuleOwnerMappingRunState state) + { + return userConfig.GetText($"rule_owner_run_{state}"); + } + + /// + /// Describes one recorded change as "setting: from to". Mapping source names are localized, the other + /// values are the configured texts themselves and are shown as they were entered. + /// + /// Change to describe. + /// The readable description. + private string DescribeChange(RuleOwnerMappingChange change) + { + bool isSource = change.Setting == RuleOwnerMappingChangeSetting.kSource; + string from = isSource ? userConfig.GetText(change.From) : change.From; + string to = isSource ? userConfig.GetText(change.To) : change.To; + return $"{userConfig.GetText(GetChangeSettingTextKey(change.Setting))}: {from} → {to}"; + } + + /// + /// Resolves the text key naming a changed setting. + /// + /// Setting identifier as it was recorded. + /// The text key. + private static string GetChangeSettingTextKey(string setting) + { + return setting switch + { + RuleOwnerMappingChangeSetting.kSource => "source", + RuleOwnerMappingChangeSetting.kMarker => "modelledMarker", + RuleOwnerMappingChangeSetting.kCustomFieldKeys => "custom_field_Owner_key", + _ => "change" + }; + } + + /// + /// Resolves the display text of a finding. After a deliberate change the difference is the intended + /// result, so it is named neutrally instead of as something the incremental mapping failed to do. + /// + /// Finding to display. + /// The localized text. + private string GetFindingText(RuleOwnerMappingFinding finding) + { + return finding == RuleOwnerMappingFinding.Missing ? GetAddedLabel() : GetRemovedLabel(); + } + + /// + /// Names the mappings the run established, as an omission or as a plain addition. + /// + /// The localized text. + private string GetAddedLabel() + { + return userConfig.GetText(State == RuleOwnerMappingRunState.ChangeApplied + ? "rule_owner_change_added" + : "rule_owner_finding_Missing"); + } + + /// + /// Names the mappings the run dropped, as an omission or as a plain removal. + /// + /// The localized text. + private string GetRemovedLabel() + { + return userConfig.GetText(State == RuleOwnerMappingRunState.ChangeApplied + ? "rule_owner_change_removed" + : "rule_owner_finding_Superfluous"); + } +} diff --git a/roles/ui/files/FWO.UI/Pages/Settings/OwnerMapping.razor b/roles/ui/files/FWO.UI/Pages/Settings/OwnerMapping.razor index aa6ed16315..d1b820f20a 100644 --- a/roles/ui/files/FWO.UI/Pages/Settings/OwnerMapping.razor +++ b/roles/ui/files/FWO.UI/Pages/Settings/OwnerMapping.razor @@ -119,7 +119,9 @@ } /// - /// Triggers a full rule-owner mapping rebuild after mapping configuration changes were saved. + /// Triggers a full rule-owner mapping rebuild after mapping configuration changes were saved. The rebuilt + /// state differs from the stored one by design here, so the run is marked as following a configuration + /// change and its difference is not reported as drift of the incremental mapping. /// private async Task TriggerFullRuleOwnerReinitialize() { @@ -127,7 +129,7 @@ try { var tcs = new TaskCompletionSource(); - var args = new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true, Completion = tcs }; + var args = new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true, TriggeredByChange = true, Changes = ownerMappingSourceHandler.AppliedChanges, Completion = tcs }; EventMediator.Publish("UpdateOwnerRuleMappings", new UpdateRuleOwnerMappingEvent(args)); return await tcs.Task; } diff --git a/roles/ui/files/FWO.UI/Pages/Settings/SettingsImport.razor b/roles/ui/files/FWO.UI/Pages/Settings/SettingsImport.razor index 03b7c5515e..ebfa4fec8b 100644 --- a/roles/ui/files/FWO.UI/Pages/Settings/SettingsImport.razor +++ b/roles/ui/files/FWO.UI/Pages/Settings/SettingsImport.razor @@ -123,7 +123,7 @@ else private async Task TriggerFullRuleOwnerReinitialize() { var tcs = new TaskCompletionSource(); - var args = new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true, Completion = tcs }; + var args = new UpdateRuleOwnerMappingEventArgs { isFullReInitialize = true, TriggeredByChange = true, Changes = ownerMappingSourceHandler.AppliedChanges, Completion = tcs }; EventMediator.Publish("UpdateOwnerRuleMappings", new UpdateRuleOwnerMappingEvent(args)); return await tcs.Task; } diff --git a/roles/ui/files/FWO.UI/Services/OwnerMappingSourceHandler.cs b/roles/ui/files/FWO.UI/Services/OwnerMappingSourceHandler.cs index c1b769e9fe..53c6e9f63d 100644 --- a/roles/ui/files/FWO.UI/Services/OwnerMappingSourceHandler.cs +++ b/roles/ui/files/FWO.UI/Services/OwnerMappingSourceHandler.cs @@ -1,5 +1,7 @@ using FWO.Basics; using FWO.Config.Api.Data; +using FWO.Services; +using FWO.Data.Enums; using FWO.Logging; using System.Text.Json; @@ -65,7 +67,28 @@ [.. Enum.GetValues() ///     public string ModelledMarker { get; set; } = ""; + /// + /// Log levels offered for selection. + /// + public List LogLevels { get; } = + [.. Enum.GetValues().Cast()]; + + /// + /// How much detail about single unmappable rules is written to the log. Changing it never requires a + /// rule owner rebuild, because it does not influence the mappings themselves. + /// + public RuleOwnerMappingLogLevel LogLevel { get; set; } = RuleOwnerMappingLogLevel.Warning; + + /// + /// Settings changed by the last that require a rule owner rebuild. Passed on with + /// the rebuild so its result can be told apart from a run where the incremental mapping failed. Kept + /// until reports the rebuild as done, so a retry after a failed + /// save still names what was changed. + /// + public List AppliedChanges { get; private set; } = []; + private bool ruleOwnerRebuildPending; + private RuleOwnerMappingLogLevel storedLogLevel = RuleOwnerMappingLogLevel.Warning; private bool appliedSettingsRequireRuleOwnerRebuild; private int storedSource; private string storedModelledMarker = ""; @@ -95,6 +118,7 @@ public void TakeOverStoredSettings(ConfigData configData) storedSource = configData.OwnerSoruceMappingID; rawOwnerKeys = configData.CustomFieldOwnerKey ?? ""; storedModelledMarker = configData.ModModelledMarker ?? ""; + storedLogLevel = configData.RuleOwnerMappingLogLevel; ruleOwnerRebuildPending |= appliedSettingsRequireRuleOwnerRebuild; appliedSettingsRequireRuleOwnerRebuild = false; } @@ -111,6 +135,7 @@ public void DiscardEdits() OwnerKeysToAdd = []; OwnerKeysToDelete = []; ModelledMarker = storedModelledMarker; + LogLevel = storedLogLevel; } /// @@ -208,11 +233,20 @@ public bool ApplyTo(ConfigData configData) ? ModelledMarker : storedModelledMarker; + // the log level applies to every source and never changes the mappings, so it is not part of the + // comparison deciding whether a rule owner rebuild is required + configData.RuleOwnerMappingLogLevel = LogLevel; + // the stored settings are compared, not the ones of the given configuration: a retry after a failed // write would otherwise compare the configuration the previous attempt already changed against itself // the requirement is only remembered once the settings are stored, so a write which failed leaves no // rebuild outstanding for settings the database never received - appliedSettingsRequireRuleOwnerRebuild = NeedsRuleOwnerReinitialize(storedSource, rawOwnerKeys, storedModelledMarker, configData); + List changes = CollectChanges(storedSource, rawOwnerKeys, storedModelledMarker, configData); + appliedSettingsRequireRuleOwnerRebuild = changes.Count > 0; + if (changes.Count > 0) + { + AppliedChanges = changes; + } return ruleOwnerRebuildPending || appliedSettingsRequireRuleOwnerRebuild; } @@ -224,29 +258,65 @@ public bool ApplyTo(ConfigData configData) public void ConfirmRuleOwnerRebuild() { ruleOwnerRebuildPending = false; + AppliedChanges = []; } /// - /// Decides whether the saved settings require a full rule owner mapping rebuild. + /// Collects the saved settings that require a full rule owner mapping rebuild. An empty result means + /// nothing mapping-relevant was changed, so it also answers whether a rebuild is needed at all. /// /// Mapping source before the change. /// Serialized owner keys before the change. /// Name field marker before the change. /// Configuration holding the saved settings. - /// True if a rebuild is required. - private static bool NeedsRuleOwnerReinitialize(int oldSource, string oldOwnerKeys, string oldModelledMarker, ConfigData configData) + /// The mapping-relevant changes, newest state first. + private static List CollectChanges(int oldSource, string oldOwnerKeys, string oldModelledMarker, ConfigData configData) { + List changes = []; + if (oldSource != configData.OwnerSoruceMappingID) { - return true; + changes.Add(new RuleOwnerMappingChange + { + Setting = RuleOwnerMappingChangeSetting.kSource, + From = DescribeSource(oldSource), + To = DescribeSource(configData.OwnerSoruceMappingID) + }); } if (configData.OwnerSoruceMappingID == (int)OwnerMappingSourceStm.CustomField && oldOwnerKeys != configData.CustomFieldOwnerKey) { - return true; + changes.Add(new RuleOwnerMappingChange + { + Setting = RuleOwnerMappingChangeSetting.kCustomFieldKeys, + From = oldOwnerKeys, + To = configData.CustomFieldOwnerKey ?? "" + }); + } + + if (configData.OwnerSoruceMappingID == (int)OwnerMappingSourceStm.NameField && oldModelledMarker != configData.ModModelledMarker) + { + changes.Add(new RuleOwnerMappingChange + { + Setting = RuleOwnerMappingChangeSetting.kMarker, + From = oldModelledMarker, + To = configData.ModModelledMarker ?? "" + }); } - return configData.OwnerSoruceMappingID == (int)OwnerMappingSourceStm.NameField && oldModelledMarker != configData.ModModelledMarker; + return changes; + } + + /// + /// Names a mapping source by its enum name, which the display resolves to a localized text. + /// + /// Stored mapping source id. + /// The enum name, or the raw id when it is not a known source. + private static string DescribeSource(int source) + { + return Enum.IsDefined(typeof(OwnerMappingSourceStm), source) + ? ((OwnerMappingSourceStm)source).ToString() + : source.ToString(); } /// diff --git a/roles/ui/files/FWO.UI/Services/RuleOwnerMappingRunHandler.cs b/roles/ui/files/FWO.UI/Services/RuleOwnerMappingRunHandler.cs new file mode 100644 index 0000000000..8f6b254094 --- /dev/null +++ b/roles/ui/files/FWO.UI/Services/RuleOwnerMappingRunHandler.cs @@ -0,0 +1,231 @@ +using FWO.Services; + +namespace FWO.Ui.Services +{ + /// + /// What a recorded difference says about a single rule-owner pair. + /// + public enum RuleOwnerMappingFinding + { + /// The full reinitialize established the mapping, so the incremental mapping never created it. + Missing, + + /// The full reinitialize does not produce the mapping, so the incremental mapping left it behind. + Superfluous + } + + /// + /// One rule-owner pair of a recorded run together with what it says about the incremental mapping. + /// + public class RuleOwnerMappingRunEntry + { + /// Rule the mapping belongs to. + public long RuleId { get; init; } + + /// Owner the rule was mapped to. + public int OwnerId { get; init; } + + /// Rule metadata the mapping belongs to, which survives new versions of the rule. + public long? RuleMetadataId { get; init; } + + /// What the difference means for this pair. + public RuleOwnerMappingFinding Finding { get; init; } + + /// + /// Import that created the rule_owner row. For a missing pair this is the run itself, for a left over + /// pair the import it had originally been established by. + /// + public long Created { get; init; } + + /// + /// Import that removed the rule_owner row, or while the mapping is active. + /// A left over pair is removed by the run itself, a missing pair is active afterwards. + /// + public long? Removed { get; init; } + } + + /// + /// How a recorded run has to be read. Only points at a problem of the incremental + /// mapping; the other states explain the difference by something else. + /// + public enum RuleOwnerMappingRunState + { + /// Rebuilt state matches the stored one, nothing to do. + InSync, + + /// Imports were still waiting to be mapped, so the difference is just the backlog. + ImportsPending, + + /// The run followed a deliberate change, so a different result is expected. + ChangeApplied, + + /// The incremental mapping missed the listed changes. + Drift + } + + /// + /// Editor state of the rule owner mapping run history: which of the stored runs is shown and how its + /// result has to be read. + /// + public class RuleOwnerMappingRunHandler + { + /// Text key shown while no full reinitialize has been recorded yet. + public const string kNoHistoryText = "U7551"; + + /// Recorded runs that found a difference, newest first. + public List Runs { get; private set; } = []; + + /// + /// Most recent run that found no difference. Kept apart from so it can never be + /// pushed out by newer findings - it is the answer to "when was the mapping last verified correct". + /// + public RuleOwnerMappingRun? LastRunWithoutFindings { get; private set; } + + /// Position of the shown run, 0 being the newest. + public int SelectedIndex { get; private set; } + + /// Run currently shown, or when nothing was recorded yet. + public RuleOwnerMappingRun? SelectedRun => SelectedIndex < Runs.Count ? Runs[SelectedIndex] : null; + + /// + /// How the mapping stands right now, taken from whichever run happened last. The history below only + /// holds runs that found something and therefore cannot answer this - which is the question the page + /// is opened for. Null while nothing was recorded at all. + /// + public RuleOwnerMappingRunState? CurrentState + { + get + { + if (Runs.Count == 0) + { + return LastRunWithoutFindings == null ? null : RuleOwnerMappingRunState.InSync; + } + return LastRunWithoutFindings != null && LastRunWithoutFindings.RunTime >= Runs[0].RunTime + ? RuleOwnerMappingRunState.InSync + : GetState(Runs[0]); + } + } + + /// True when a newer run than the shown one exists. + public bool HasNewer => SelectedIndex > 0; + + /// True when an older run than the shown one exists. + public bool HasOlder => SelectedIndex + 1 < Runs.Count; + + /// + /// Takes over the recorded history and shows the newest run that found a difference. + /// + /// History as it is stored. + public void Init(RuleOwnerMappingRunHistoryData history) + { + Runs = history.RunsWithFindings; + LastRunWithoutFindings = history.LastRunWithoutFindings; + SelectedIndex = 0; + } + + /// Shows the next newer run, if there is one. + public void SelectNewer() + { + if (HasNewer) + { + SelectedIndex--; + } + } + + /// Shows the next older run, if there is one. + public void SelectOlder() + { + if (HasOlder) + { + SelectedIndex++; + } + } + + /// + /// Lists the pairs of the shown run, missing ones first, each with what it says about the + /// incremental mapping. + /// + /// The listed pairs, empty when the run found no difference. + public List GetSelectedEntries() + { + RuleOwnerMappingRun? run = SelectedRun; + if (run == null) + { + return []; + } + + // the run establishes the missing pairs and removes the left over ones, so both rows are + // findable in rule_owner by the run's control id - only the origin of a left over pair is older + List entries = run.Added + .Select(pair => new RuleOwnerMappingRunEntry + { + RuleId = pair.RuleId, + OwnerId = pair.OwnerId, + RuleMetadataId = pair.RuleMetadataId, + Finding = RuleOwnerMappingFinding.Missing, + Created = pair.Created ?? run.ControlId, + Removed = null + }) + .ToList(); + + entries.AddRange(run.Removed + .Select(pair => new RuleOwnerMappingRunEntry + { + RuleId = pair.RuleId, + OwnerId = pair.OwnerId, + RuleMetadataId = pair.RuleMetadataId, + Finding = RuleOwnerMappingFinding.Superfluous, + Created = pair.Created ?? run.ControlId, + Removed = run.ControlId + })); + + return entries; + } + + /// + /// Decides how the result of the shown run has to be read. A pending backlog and a configuration + /// change both explain a difference on their own, so neither is reported as a problem. + /// + /// The state of the shown run. + public RuleOwnerMappingRunState GetSelectedState() + { + return SelectedRun == null ? RuleOwnerMappingRunState.ImportsPending : GetState(SelectedRun); + } + + /// + /// Decides how the result of one run has to be read. + /// + /// Run to judge. + /// The state of that run. + private static RuleOwnerMappingRunState GetState(RuleOwnerMappingRun run) + { + if (!run.DiffMeaningful) + { + return RuleOwnerMappingRunState.ImportsPending; + } + // no difference is the strongest statement there is, whatever triggered the run - a change that + // turned out to have no effect is still a verification that the stored state was correct + if (run.AddedCount + run.RemovedCount == 0) + { + return RuleOwnerMappingRunState.InSync; + } + return run.TriggeredByChange ? RuleOwnerMappingRunState.ChangeApplied : RuleOwnerMappingRunState.Drift; + } + + /// + /// Resolves the bootstrap context of a state, so what needs attention reads at a glance. + /// + /// State to display. + /// The bootstrap context name. + public static string GetStateStyle(RuleOwnerMappingRunState state) + { + return state switch + { + RuleOwnerMappingRunState.InSync => "success", + RuleOwnerMappingRunState.Drift => "danger", + RuleOwnerMappingRunState.ImportsPending => "warning", + _ => "secondary" + }; + } + } +} diff --git a/roles/ui/files/FWO.UI/Shared/MonitoringLayout.razor b/roles/ui/files/FWO.UI/Shared/MonitoringLayout.razor index 380484c955..c738508544 100644 --- a/roles/ui/files/FWO.UI/Shared/MonitoringLayout.razor +++ b/roles/ui/files/FWO.UI/Shared/MonitoringLayout.razor @@ -51,6 +51,11 @@ Scheduler +
  • + + @(userConfig.GetText("rule_owner_mapping_runs")) + +
  • @(userConfig.GetText("import"))
    diff --git a/roles/ui/files/FWO.UI/Shared/OwnerMappingSourceEditor.razor b/roles/ui/files/FWO.UI/Shared/OwnerMappingSourceEditor.razor index 640d4a6a2a..6cac37ab67 100644 --- a/roles/ui/files/FWO.UI/Shared/OwnerMappingSourceEditor.razor +++ b/roles/ui/files/FWO.UI/Shared/OwnerMappingSourceEditor.razor @@ -1,3 +1,5 @@ +@using FWO.Data.Enums + @inject UserConfig userConfig @* editor for the rule owner mapping source settings, shared by every settings page offering them *@ @@ -49,6 +51,21 @@ } +@if (Handler.SelectedSource != OwnerMappingSourceStm.Disabled) +{ +
    + +
    + + + @GetLogLevelText(level) + + +
    +
    +} @code { @@ -78,4 +95,14 @@ { return userConfig.GetText(source?.ToString() ?? ""); } + + /// + /// Resolves the display text of a log level. + /// + /// Log level to display. + /// The localized text. + private string GetLogLevelText(RuleOwnerMappingLogLevel? level) + { + return userConfig.GetText(level?.ToString() ?? ""); + } } From c7dcfd1e917a79cb2c72e3b14a34f03017a3337e Mon Sep 17 00:00:00 2001 From: cd Date: Thu, 17 Sep 2026 12:50:32 +0200 Subject: [PATCH 02/25] fix: mistake fworch-text wrong id --- roles/database/files/sql/idempotent/fworch-texts.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/roles/database/files/sql/idempotent/fworch-texts.sql b/roles/database/files/sql/idempotent/fworch-texts.sql index 2c388a9cdf..6f41281ad5 100644 --- a/roles/database/files/sql/idempotent/fworch-texts.sql +++ b/roles/database/files/sql/idempotent/fworch-texts.sql @@ -8063,6 +8063,10 @@ INSERT INTO txt VALUES ('H7252', 'English', 'Sample data (defined by the ending '); INSERT INTO txt VALUES ('H7253', 'German', 'Die Ergebnisse der Prüfung des Import-Status der aktiven Managements sind hier protokolliert. Werden Anomalien wie überlange Import-Zeiten oder fehlende Imports festgestellt, werden einzelne Alarme ausgelöst, die unter     Offenen Alarme analysiert und behandelt werden können. Hier wird lediglich die Anzahl der gefundenen Probleme protokolliert. +'); +INSERT INTO txt VALUES ('H7253', 'English', 'Results of the Import status checks of the active managements are recorded here. If anomalies as overdue or missing imports are found, + separate alerts are raised, which can be analysed and handled at Open Alerts. +'); INSERT INTO txt VALUES ('H7261', 'German', 'Ein vollständiger Neuaufbau berechnet alle Zuordnungen von Grund auf und vergleicht das Ergebnis mit dem gespeicherten Stand. Der Balken oben sagt, wie die Zuordnung aktuell steht; er nimmt den zuletzt gelaufenen Neuaufbau. Darunter stehen die aufgezeichneten Läufe, zwischen denen mit Neuer und Älter gewechselt wird.'); INSERT INTO txt VALUES ('H7261', 'English', 'A full rebuild recalculates every mapping from scratch and compares the result with the stored state. The banner at the top says how the mapping stands right now, taken from the rebuild that ran last. Below it are the recorded runs; use Newer and Older to move between them.'); INSERT INTO txt VALUES ('H7262', 'German', 'Die vier Zustände: Unverändert – der Lauf fand keinen Unterschied, die laufende Aktualisierung ist auf dem richtigen Stand. Änderung angewendet – eine Einstellung wurde geändert, das Ergebnis weicht deshalb bewusst ab. Importe offen – es war noch etwas unverarbeitet, der Lauf kann nichts aussagen. Abweichung – Unterschied ohne offene Importe und ohne Änderung; nur dieser Zustand ist ein Problem.'); @@ -8095,10 +8099,6 @@ INSERT INTO txt VALUES ('H7274', 'German', 'Was vor diesem Lauf geändert wu INSERT INTO txt VALUES ('H7274', 'English', 'What was changed before this run. That is why the result deliberately differs from the previous state, and the difference is not reported as a problem.'); INSERT INTO txt VALUES ('H7275', 'German', 'Wie die Eigentümerzuordnung aktuell steht, abgeleitet aus dem zuletzt gelaufenen Neuaufbau. Nur Die letzte Prüfung hat Abweichungen gefunden weist auf ein Problem hin. Die unten aufgelisteten Läufe sind Historie: jeder Neuaufbau schreibt selbst den korrekten Stand, der Befund berichtet also, was vorher falsch war. Der Zeitstempel der letzten Prüfung ohne Befund wird nie verdrängt.'); INSERT INTO txt VALUES ('H7275', 'English', 'How the owner mapping stands right now, taken from the rebuild that ran last. Only The last check found deviations points at a problem. The runs listed below are history: every rebuild writes the correct state itself, so a finding reports what was wrong before it ran. The timestamp of the last check without findings is never pushed out.'); -'); -INSERT INTO txt VALUES ('H7253', 'English', 'Results of the Import status checks of the active managements are recorded here. If anomalies as overdue or missing imports are found, - separate alerts are raised, which can be analysed and handled at Open Alerts. -'); INSERT INTO txt VALUES ('H7301', 'German', 'Hier werden die Ausgaben der verschiedenen Importe protokolliert. '); INSERT INTO txt VALUES ('H7301', 'English', 'Here the output of the different imports are documented. From dd15834197b7fad2207b8f2f287a437b0f42bd3d Mon Sep 17 00:00:00 2001 From: cd Date: Thu, 17 Sep 2026 13:30:05 +0200 Subject: [PATCH 03/25] wip --- .../files/sql/idempotent/fworch-texts.sql | 8 ++--- .../UpdateRuleOwnerMappingBase.cs | 35 ++++++++++++++----- .../UpdateRuleOwnerMappingIncrementalTest.cs | 35 ++++++++++++++++++- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/roles/database/files/sql/idempotent/fworch-texts.sql b/roles/database/files/sql/idempotent/fworch-texts.sql index 7cb64ec0f8..1e3c9a33a5 100644 --- a/roles/database/files/sql/idempotent/fworch-texts.sql +++ b/roles/database/files/sql/idempotent/fworch-texts.sql @@ -8090,10 +8090,10 @@ INSERT INTO txt VALUES ('H7263', 'German', 'Aufgezeichnet werden Läufe mit INSERT INTO txt VALUES ('H7263', 'English', 'Recorded are runs with a difference and runs that applied a changed setting – the latter even when they had no effect, because that is exactly what you want to see. A rebuild without a difference takes no slot, it only refreshes the timestamp above. Up to ten runs are kept.'); INSERT INTO txt VALUES ('H7264', 'German', 'In der Tabelle hängt die Bezeichnung vom Anlass ab. Bei einer Abweichung heißt es Nicht angelegt beziehungsweise Nicht entfernt – die laufende Aktualisierung hätte das tun müssen. Nach einer bewussten Änderung heißt dasselbe Neu hinzugekommen und Weggefallen, denn dort ist es das gewollte Ergebnis. Regel, Eigentümer und Metadaten sind als IDs angegeben; über Erstellt und Entfernt lässt sich die Zeile in der Tabelle rule_owner wiederfinden.'); INSERT INTO txt VALUES ('H7264', 'English', 'In the table the wording depends on what caused the run. For a deviation it reads Not created or Not removed – the running update should have done that. After a deliberate change the same thing reads Newly added and No longer applies, because there it is the intended result. Rule, owner and metadata are given as IDs; Created and Removed let you find the row again in the rule_owner table.'); -INSERT INTO txt VALUES ('H7265', 'German', 'Beim Wechsel der Mapping-Quelle ändert sich jede Zuordnung. Die Einzelliste wird dann nicht gespeichert, weil sie nichts aussagen würde; die Zähler bleiben vollständig. Waren Importe offen, stehen deren Nummern dabei und der Lauf frischt den Zeitstempel oben nicht auf – er hat ja nichts nachgewiesen.'); -INSERT INTO txt VALUES ('H7265', 'English', 'Switching the mapping source changes every mapping. The individual list is then not stored because it would say nothing; the counts stay complete. If imports were pending, their numbers are listed and the run does not refresh the timestamp above – it proved nothing.'); -INSERT INTO txt VALUES ('H7276', 'German', 'Ein Alarm wird gemeldet, wenn ein Import nicht verarbeitet werden konnte, wenn wegen zu vieler offener Importe auf einen vollständigen Neuaufbau ausgewichen wurde, wenn die Mapping-Quelle überhaupt keine Regel mehr trifft, und bei einer echten Abweichung. Eine bewusste Änderung und offene Importe lösen keinen Alarm aus. Alarme stehen unter Monitoring – Alarme.'); -INSERT INTO txt VALUES ('H7276', 'English', 'An alert is raised when an import could not be processed, when too many pending imports forced a full rebuild, when the mapping source no longer matches any rule at all, and on a real deviation. A deliberate change and pending imports raise no alert. Alerts are listed under Monitoring – Alerts.'); +INSERT INTO txt VALUES ('H7265', 'German', 'Beim Wechsel der Mapping-Quelle ändert sich jede Zuordnung. Die Einzelliste wird dann nicht gespeichert, weil sie nichts aussagen würde; die Zähler bleiben vollständig. Waren Importe offen, stehen deren Nummern dabei und der Lauf frischt den Zeitstempel oben nicht auf – er hat ja nichts nachgewiesen. Ein Import, der zweimal hintereinander nicht verarbeitet werden konnte, löst selbst einen vollständigen Neuaufbau aus; der erscheint dann hier mit dieser Import-Nummer und zeigt, welche Zuordnungen dadurch gefehlt haben.'); +INSERT INTO txt VALUES ('H7265', 'English', 'Switching the mapping source changes every mapping. The individual list is then not stored because it would say nothing; the counts stay complete. If imports were pending, their numbers are listed and the run does not refresh the timestamp above – it proved nothing. An import that could not be processed twice in a row triggers a full rebuild itself; that run then appears here with the import number and shows which mappings had been missing because of it.'); +INSERT INTO txt VALUES ('H7276', 'German', 'Ein Alarm wird gemeldet, wenn ein Import nicht verarbeitet werden konnte, wenn wegen zu vieler offener Importe auf einen vollständigen Neuaufbau ausgewichen wurde, wenn die Mapping-Quelle überhaupt keine Regel mehr trifft, und bei einer echten Abweichung. Scheitert derselbe Import erneut, wird kein zweiter Alarm gemeldet – der erste steht ja noch offen – sondern der Zustand durch einen vollständigen Neuaufbau repariert. Eine bewusste Änderung und offene Importe lösen keinen Alarm aus. Alarme stehen unter Monitoring – Alarme.'); +INSERT INTO txt VALUES ('H7276', 'English', 'An alert is raised when an import could not be processed, when too many pending imports forced a full rebuild, when the mapping source no longer matches any rule at all, and on a real deviation. If the same import fails again, no second alert is raised – the first one is still open – and the state is repaired by a full rebuild instead. A deliberate change and pending imports raise no alert. Alerts are listed under Monitoring – Alerts.'); INSERT INTO txt VALUES ('H7266', 'German', 'Wie das Ergebnis dieses Laufs zu lesen ist. Nur Abweichung weist auf ein Problem hin; die anderen Zustände erklären den Unterschied von selbst.'); INSERT INTO txt VALUES ('H7266', 'English', 'How the result of this run has to be read. Only Deviation points at a problem; the other states explain the difference themselves.'); INSERT INTO txt VALUES ('H7267', 'German', 'Zeitpunkt, zu dem die vollständige Neuberechnung abgeschlossen und ihr Ergebnis festgehalten wurde.'); diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs index 561273deee..96d421f07c 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs @@ -192,8 +192,7 @@ protected async Task RunIncremental(Func processIncre if (failedImportControlIds.Any()) { - await AlertFailedIncrementalImports(failedImportControlIds); - return false; + return await HandleFailedImports(failedImportControlIds, fullReinitFunc); } return true; @@ -503,13 +502,28 @@ await apiConnection.SendQueryAsync(ImportQueries.updateImportCont /// - /// Raises an alert for incremental imports that could not be processed, so a failing rule_owner - /// mapping becomes visible outside the middleware log file. + /// Reports imports that could not be processed and repairs the state when the same ones already + /// failed on the previous run. A single failure is usually transient and is retried by the next run, + /// so processing simply continues. One that persists would leave its changes unapplied indefinitely, + /// because the healthy imports drain and the pending backlog never reaches the fallback threshold - + /// the full reinitialize takes over instead, recomputing everything and completing the stuck import. /// /// Control ids of the imports that failed. - private async Task AlertFailedIncrementalImports(List failedImportControlIds) + /// Rebuilds every mapping. + /// False after the first failure, the result of the rebuild after a repeated one. + private async Task HandleFailedImports(List failedImportControlIds, Func> fullReinitFunc) { - await RaiseAlert($"Rule owner mapping failed for import_control {string.Join(", ", failedImportControlIds)}. See the middleware log for details."); + string failedIds = string.Join(", ", failedImportControlIds); + bool failedBefore = await RaiseAlert($"Rule owner mapping failed for import_control {failedIds}. See the middleware log for details."); + + if (!failedBefore) + { + return false; + } + + // the alert of the previous run is still open, so this is at least the second attempt + Log.WriteWarning(LogMessageTitle, $"import_control {failedIds} failed again. Falling back to full rule_owner reinitialize."); + return await fullReinitFunc(); } /// @@ -545,16 +559,18 @@ await RaiseAlert($"Full rule_owner reinitialize {run.ControlId} added {run.Added /// /// Writes a log entry and an alert unless the same alert is already open, so a job repeating every - /// few seconds does not flood the alert list with identical entries. + /// few seconds does not flood the alert list with identical entries. The already open alert also + /// tells a repeated problem from a first occurrence, see . /// /// Description shown in the alert and the log entry. - private async Task RaiseAlert(string description) + /// True if an alert with the same description was already open and nothing was written. + private async Task RaiseAlert(string description) { try { if (await SameAlertAlreadyOpen(description)) { - return; + return true; } await AlertHelper.AddLogEntry(apiConnection, kAlertSeverity, LogMessageTitle, description, GlobalConst.kRuleOwnerMapping); @@ -565,6 +581,7 @@ await AlertHelper.SetAlert(apiConnection, LogMessageTitle, description, GlobalCo { Log.WriteError(LogMessageTitle, "Error while raising a rule_owner mapping alert.", ex); } + return false; } /// diff --git a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs index 794af6637e..b99bb920d1 100644 --- a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs +++ b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs @@ -329,6 +329,36 @@ public async Task RunAsync_ShouldNotClaimAVerification_WhenImportsWerePending() Assert.That(apiConnection.StoredHistory.LastRunWithoutFindings, Is.Null); } + [Test] + public async Task RunAsync_ShouldRepairByFullReinitialize_WhenTheSameImportFailsAgain() + { + // the healthy imports drain, so a stuck one never lets the backlog reach the fallback threshold - + // without this its changes would stay unapplied for good + RuleOwnerMappingFake apiConnection = new(); + apiConnection.AddPendingImport(1, ImportType.RULE); + apiConnection.FailRuleChangeLookupForImport = 1; + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + bool firstRun = await service.RunAsync(); + + Assert.Multiple(() => + { + Assert.That(firstRun, Is.False, "the first failure is reported and retried, not repaired"); + Assert.That(apiConnection.FullReinitializeCount, Is.EqualTo(0)); + Assert.That(apiConnection.RaisedAlerts, Has.Exactly(1).Contains("import_control 1")); + }); + + await service.RunAsync(); + + Assert.Multiple(() => + { + Assert.That(apiConnection.FullReinitializeCount, Is.EqualTo(1), "the repeated failure triggers the rebuild"); + Assert.That(apiConnection.CompletedImports, Does.Contain(1L), "the rebuild completes the stuck import"); + Assert.That(apiConnection.RaisedAlerts, Has.Exactly(1).Contains("import_control 1"), "the open alert is not raised twice"); + }); + } + /// /// Simulated API connection for the CustomField mapping source. /// @@ -346,6 +376,7 @@ private sealed class RuleOwnerMappingFake : SimulatedApiConnection public List CompletedImports { get; } = []; public List RaisedAlerts { get; } = []; public long? FailRuleChangeLookupForImport { get; set; } + public int FullReinitializeCount { get; private set; } public string? StoredHistoryJson { get; private set; } public RuleOwnerMappingRunHistoryData StoredHistory => StoredHistoryJson == null @@ -437,6 +468,7 @@ private bool TryHandleImportQuery(string query, object? variables, out object? r if (query == ImportQueries.addImportForRuleOwner) { + FullReinitializeCount++; result = new InsertImportControl { Returning = [new ImportControl { ControlId = 999 }] }; return true; } @@ -562,7 +594,8 @@ private bool TryHandleAlertQuery(string query, object? variables, out object? re if (query == MonitorQueries.getOpenAlerts) { - result = new List(); + // like the real table: an alert stays open until somebody acknowledges it + result = RaisedAlerts.Select(description => new Alert { AlertCode = AlertCode.RuleOwnerMapping, Description = description }).ToList(); return true; } From c268ecffd4e3e4652f62ec821c6058b0cf91f81b Mon Sep 17 00:00:00 2001 From: cd Date: Thu, 17 Sep 2026 13:59:04 +0200 Subject: [PATCH 04/25] fix findings --- .../files/sql/idempotent/fworch-texts.sql | 4 +-- .../RuleOwnerMappingRunHistory.cs | 3 +- .../UpdateRuleOwnerMappingBase.cs | 15 +++++++--- .../UpdateRuleOwnerMappingDisabled.cs | 10 +++++-- .../UpdateRuleOwnerMappingNameField.cs | 2 +- .../UpdateRuleOwnerMappingIncrementalTest.cs | 30 ++++++++++++++++++- .../Monitoring/MonitorRuleOwnerMapping.razor | 4 +-- 7 files changed, 54 insertions(+), 14 deletions(-) diff --git a/roles/database/files/sql/idempotent/fworch-texts.sql b/roles/database/files/sql/idempotent/fworch-texts.sql index 1e3c9a33a5..3410b365eb 100644 --- a/roles/database/files/sql/idempotent/fworch-texts.sql +++ b/roles/database/files/sql/idempotent/fworch-texts.sql @@ -4262,8 +4262,8 @@ INSERT INTO txt VALUES ('U7401', 'German', 'Archiv der Autodiscovery-Nachrichte INSERT INTO txt VALUES ('U7401', 'English', 'View the past autodiscovery messages'); INSERT INTO txt VALUES ('U7501', 'German', 'Archiv der Nachrichten der täglichen Checks'); INSERT INTO txt VALUES ('U7501', 'English', 'View the past daily check messages'); -INSERT INTO txt VALUES ('U7550', 'German', 'Ergebnis der letzten vollständigen Neuberechnungen der Eigentümerzuordnung. Ein vollständiger Neuaufbau berechnet alle Zuordnungen neu; weicht das Ergebnis vom gespeicherten Stand ab, obwohl kein Import offen war, hat die laufende Aktualisierung etwas übersehen. Aufgelistet werden nur Läufe mit Abweichung; wann zuletzt ohne Befund geprüft wurde, steht oben.'); -INSERT INTO txt VALUES ('U7550', 'English', 'Result of the last full recalculations of the owner mapping. A full rebuild recalculates every mapping; if the result differs from the stored state although no import was pending, the running update missed something. Only runs with a deviation are listed; when it was last verified without findings is shown above.'); +INSERT INTO txt VALUES ('U7550', 'German', 'Ergebnis der letzten vollständigen Neuberechnungen der Eigentümerzuordnung. Ein vollständiger Neuaufbau berechnet alle Zuordnungen neu; weicht das Ergebnis vom gespeicherten Stand ab, obwohl kein Import offen war, hat die laufende Aktualisierung etwas übersehen. Aufgelistet werden nur Läufe mit Abweichung; wann zuletzt ohne Befund geprüft wurde, steht oben.'); +INSERT INTO txt VALUES ('U7550', 'English', 'Result of the last full recalculations of the owner mapping. A full rebuild recalculates every mapping; if the result differs from the stored state although no import was pending, the running update missed something. Only runs with a deviation are listed; when it was last verified without findings is shown above.'); INSERT INTO txt VALUES ('U7551', 'German', 'Bisher wurde keine vollständige Neuberechnung aufgezeichnet. Sie wird unter Einstellungen - Eigentümer - Eigentümerzuordnung ausgelöst.'); INSERT INTO txt VALUES ('U7551', 'English', 'No full recalculation has been recorded yet. It is triggered under Settings - Owners - Owner Mapping.'); INSERT INTO txt VALUES ('U7552', 'German', 'Es wurden mehr Abweichungen gefunden, als hier aufgelistet sind. Die Anzahl oben ist vollständig, die Liste zeigt nur die ersten Einträge.'); diff --git a/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs b/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs index 9e6f6bd2f2..4ebfc5b3af 100644 --- a/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs +++ b/roles/lib/files/FWO.Services/RuleOwnerMappingRunHistory.cs @@ -181,7 +181,8 @@ public class RuleOwnerMappingRunHistory /// Config key the history is stored under. public const string kConfigKey = "ruleOwnerMappingRunHistory"; - private const int kMaxRuns = 10; + /// How many runs with a finding are kept. Shown on the page, so it stays in one place. + public const int kMaxRuns = 10; private const int kMaxListedPairs = 500; private const string kLogMessageTitle = "Update rule_owner Notifier"; diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs index 96d421f07c..819b9b7fd2 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingBase.cs @@ -96,11 +96,12 @@ protected async Task FinalizeFullReinitialize(List newRuleOwner List previousRuleOwners = await SetAllActiveRuleOwnersRemoved(importControlId); await InsertNewRuleOwners(newRuleOwners); - await CompleteImportControlFullReInit(importControlId); + // recorded before the completion so a failure there does not lose the result of the run await RecordRun(importControlId, previousRuleOwners, newRuleOwners, pendingImportsBefore); + await CompleteImportControlFullReInit(importControlId); - if (!newRuleOwners.Any()) + if (newRuleOwners.Count == 0) { await AlertEmptyMappingResult(); } @@ -190,7 +191,7 @@ protected async Task RunIncremental(Func processIncre } } - if (failedImportControlIds.Any()) + if (failedImportControlIds.Count > 0) { return await HandleFailedImports(failedImportControlIds, fullReinitFunc); } @@ -251,7 +252,7 @@ protected async Task FinalizeIncrementalImport(List newRuleOwners, Li /// The mappings that are safe to insert. protected async Task> DropStillActiveMappings(List newRuleOwners) { - if (!newRuleOwners.Any()) + if (newRuleOwners.Count == 0) { return newRuleOwners; } @@ -442,7 +443,10 @@ await apiConnection.SendQueryAsync(ImportQueries.updateImportCont } catch (Exception ex) { + // an import that was not marked done stays pending and would be processed again, so the + // failure has to reach RunIncremental instead of leaving the run looking successful Log.WriteError(LogMessageTitle, "Error while updating import control completion status.", ex); + throw; } } @@ -465,7 +469,10 @@ await apiConnection.SendQueryAsync(ImportQueries.updateImportCont } catch (Exception ex) { + // same reasoning as in CompleteImportControl: a rebuild whose import control was not + // completed must not be reported as a successful run Log.WriteError(LogMessageTitle, "Error while updating import control completion status.", ex); + throw; } } diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs index 63bac910dc..f7e004923d 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingDisabled.cs @@ -17,6 +17,9 @@ public class UpdateRuleOwnerMappingDisabled : UpdateRuleOwnerMappingBase { private const int kActiveRuleOwnerProbeLimit = 1; + /// Result of the disabled mapping source: it never produces a mapping. + private static readonly List NoRuleOwners = []; + /// public override OwnerMappingSourceStm Source => OwnerMappingSourceStm.Disabled; @@ -64,11 +67,12 @@ private async Task RemoveAllRuleOwnerMappings() List pendingImportsBefore = await LoadPendingImportControlIds(); long importControlId = await CreateImportControl(); List previousRuleOwners = await SetAllActiveRuleOwnersRemoved(importControlId); - await CompleteImportControlFullReInit(importControlId); // switching the mapping off removes every mapping, which belongs in the run history just like any - // other rebuild - otherwise the entry that explains an empty mapping table would be missing - await RecordRun(importControlId, previousRuleOwners, [], pendingImportsBefore); + // other rebuild - otherwise the entry that explains an empty mapping table would be missing. + // recorded before the completion so a failure there does not lose the result of the run + await RecordRun(importControlId, previousRuleOwners, NoRuleOwners, pendingImportsBefore); + await CompleteImportControlFullReInit(importControlId); Log.WriteInfo(LogMessageTitle, "All rule_owner mappings removed because the owner mapping source is disabled."); return true; diff --git a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs index c59b33e149..a99869044f 100644 --- a/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs +++ b/roles/lib/files/FWO.Services/UpdateRuleOwnerMappingNameField.cs @@ -85,7 +85,7 @@ private List BuildNewRuleOwnersNameField(List rulesToMap, List< var connectionsToOwnerMap = connectionOwnersToMap.Where(c => c.AppId.HasValue) .ToDictionary(c => c.Id, c => c.AppId!.Value); var newRuleOwners = new List(); - if (rulesToMap.Any() && string.IsNullOrWhiteSpace(globalConfig.ModModelledMarker)) + if (rulesToMap.Count > 0 && string.IsNullOrWhiteSpace(globalConfig.ModModelledMarker)) { // reported once per run instead of once per rule, and only when there is something to map, // so an idle scheduled run stays silent: without a marker no rule can be mapped at all diff --git a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs index b99bb920d1..d7ce7febf6 100644 --- a/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs +++ b/roles/tests-unit/files/FWO.Test/UpdateRuleOwnerMappingIncrementalTest.cs @@ -79,6 +79,28 @@ public async Task RunAsync_ShouldStillProcessLaterImports_WhenOneImportFails() }); } + [Test] + public async Task RunAsync_ShouldReportFailure_WhenMarkingTheImportDoneFails() + { + // the mapping changes are written but the import stays pending, so it would be processed again - + // reporting the run as successful would hide exactly that + RuleOwnerMappingFake apiConnection = new(); + apiConnection.AddPendingImport(1, ImportType.RULE); + apiConnection.AddRuleChange(1, ChangelogActionType.INSERT, kRuleId); + apiConnection.FailCompletionForImport = 1; + + UpdateRuleOwnerMappingCustomField service = new(apiConnection, CustomFieldConfig()); + + bool result = await service.RunAsync(); + + Assert.Multiple(() => + { + Assert.That(result, Is.False, "a completion that failed must not be reported as success"); + Assert.That(apiConnection.CompletedImports, Is.Empty, "the import stays pending"); + Assert.That(apiConnection.RaisedAlerts, Has.Exactly(1).Contains("import_control 1")); + }); + } + [Test] public async Task RunAsync_ShouldRaiseAlert_WhenImportFails() { @@ -376,6 +398,7 @@ private sealed class RuleOwnerMappingFake : SimulatedApiConnection public List CompletedImports { get; } = []; public List RaisedAlerts { get; } = []; public long? FailRuleChangeLookupForImport { get; set; } + public long? FailCompletionForImport { get; set; } public int FullReinitializeCount { get; private set; } public string? StoredHistoryJson { get; private set; } @@ -475,7 +498,12 @@ private bool TryHandleImportQuery(string query, object? variables, out object? r if (query == ImportQueries.updateImportControlForRuleOwnerInc || query == ImportQueries.updateImportControlForRuleOwnerFull) { - CompletedImports.Add(ReadLong(variables, "controlId")); + long completedId = ReadLong(variables, "controlId"); + if (FailCompletionForImport == completedId) + { + throw new InvalidOperationException($"Simulated API failure while completing import_control {completedId}."); + } + CompletedImports.Add(completedId); result = new ImportControl(); return true; } diff --git a/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor b/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor index 295d4d896c..7e01f39c17 100644 --- a/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor +++ b/roles/ui/files/FWO.UI/Pages/Monitoring/MonitorRuleOwnerMapping.razor @@ -11,7 +11,7 @@

    @(userConfig.GetText("rule_owner_mapping_runs"))

    -@(userConfig.GetText("U7550")) +@((MarkupString)userConfig.GetText("U7550"))
    @if (!InitComplete) @@ -47,7 +47,7 @@ else @if (InitComplete && Handler.SelectedRun != null) { -
    @(userConfig.GetText("rule_owner_runs_with_findings"))
    +
    @(userConfig.GetText("rule_owner_runs_with_findings")) (@(userConfig.GetText("last")) @(RuleOwnerMappingRunHistory.kMaxRuns))