diff --git a/editor/src/ComplianceEvidenceBundle.h b/editor/src/ComplianceEvidenceBundle.h index a8c386e..1978763 100644 --- a/editor/src/ComplianceEvidenceBundle.h +++ b/editor/src/ComplianceEvidenceBundle.h @@ -5,6 +5,8 @@ #include #include +#include "ValidationErrorUtil.h" + struct ComplianceEvidenceItem { std::string itemId; std::string controlId; @@ -17,11 +19,11 @@ public: bool addEvidence(const ComplianceEvidenceItem& item, std::string* error) { if (!error) return false; error->clear(); - if (item.itemId.empty()) return fail(error, "item_id_missing"); - if (item.controlId.empty()) return fail(error, "control_id_missing"); - if (item.artifactPath.empty()) return fail(error, "artifact_path_missing"); - if (item.summary.empty()) return fail(error, "summary_missing"); - if (items_.count(item.itemId) != 0) return fail(error, "item_duplicate"); + if (item.itemId.empty()) return failWith(error, "item_id_missing"); + if (item.controlId.empty()) return failWith(error, "control_id_missing"); + if (item.artifactPath.empty()) return failWith(error, "artifact_path_missing"); + if (item.summary.empty()) return failWith(error, "summary_missing"); + if (items_.count(item.itemId) != 0) return failWith(error, "item_duplicate"); items_[item.itemId] = item; order_.push_back(item.itemId); return true; @@ -49,9 +51,4 @@ public: private: std::map items_; std::vector order_; - - static bool fail(std::string* error, const char* code) { - *error = code; - return false; - } }; diff --git a/editor/src/ComplianceOperationalReadiness.h b/editor/src/ComplianceOperationalReadiness.h index 07fc05a..63ca704 100644 --- a/editor/src/ComplianceOperationalReadiness.h +++ b/editor/src/ComplianceOperationalReadiness.h @@ -5,6 +5,8 @@ #include #include +#include "ValidationErrorUtil.h" + struct ComplianceReadinessSnapshot { int controlsCovered = 0; int artifactCount = 0; @@ -19,12 +21,12 @@ public: static bool validateInput(const ComplianceReadinessSnapshot& s, std::string* error) { if (!error) return false; error->clear(); - if (s.controlsCovered < 0) return fail(error, "controls_covered_invalid"); - if (s.artifactCount < 0) return fail(error, "artifact_count_invalid"); - if (s.signedAttestations < 0) return fail(error, "signed_attestations_invalid"); - if (s.expiringSoon < 0) return fail(error, "expiring_soon_invalid"); - if (s.approvedExceptions < 0) return fail(error, "approved_exceptions_invalid"); - if (s.openRunbooks < 0) return fail(error, "open_runbooks_invalid"); + if (s.controlsCovered < 0) return failWith(error, "controls_covered_invalid"); + if (s.artifactCount < 0) return failWith(error, "artifact_count_invalid"); + if (s.signedAttestations < 0) return failWith(error, "signed_attestations_invalid"); + if (s.expiringSoon < 0) return failWith(error, "expiring_soon_invalid"); + if (s.approvedExceptions < 0) return failWith(error, "approved_exceptions_invalid"); + if (s.openRunbooks < 0) return failWith(error, "open_runbooks_invalid"); return true; } @@ -53,8 +55,4 @@ public: } private: - static bool fail(std::string* error, const char* code) { - *error = code; - return false; - } }; diff --git a/editor/src/ControlAttestationRegistry.h b/editor/src/ControlAttestationRegistry.h index 353fd1c..c7f8bf9 100644 --- a/editor/src/ControlAttestationRegistry.h +++ b/editor/src/ControlAttestationRegistry.h @@ -5,6 +5,8 @@ #include #include +#include "ValidationErrorUtil.h" + enum class AttestationStatus { Draft, Signed, @@ -25,12 +27,12 @@ public: bool submit(const ControlAttestation& attestation, std::string* error) { if (!error) return false; error->clear(); - if (attestation.attestationId.empty()) return fail(error, "attestation_id_missing"); - if (attestation.controlId.empty()) return fail(error, "control_id_missing"); - if (attestation.owner.empty()) return fail(error, "owner_missing"); - if (attestation.daysUntilExpiry < 0) return fail(error, "expiry_days_invalid"); - if (attestation.status == AttestationStatus::Expired) return fail(error, "status_invalid"); - if (items_.count(attestation.attestationId) != 0) return fail(error, "attestation_duplicate"); + if (attestation.attestationId.empty()) return failWith(error, "attestation_id_missing"); + if (attestation.controlId.empty()) return failWith(error, "control_id_missing"); + if (attestation.owner.empty()) return failWith(error, "owner_missing"); + if (attestation.daysUntilExpiry < 0) return failWith(error, "expiry_days_invalid"); + if (attestation.status == AttestationStatus::Expired) return failWith(error, "status_invalid"); + if (items_.count(attestation.attestationId) != 0) return failWith(error, "attestation_duplicate"); items_[attestation.attestationId] = attestation; order_.push_back(attestation.attestationId); return true; @@ -42,9 +44,9 @@ public: if (!error) return false; error->clear(); auto it = items_.find(attestationId); - if (it == items_.end()) return fail(error, "attestation_missing"); - if (signer.empty()) return fail(error, "signer_missing"); - if (it->second.status == AttestationStatus::Expired) return fail(error, "attestation_expired"); + if (it == items_.end()) return failWith(error, "attestation_missing"); + if (signer.empty()) return failWith(error, "signer_missing"); + if (it->second.status == AttestationStatus::Expired) return failWith(error, "attestation_expired"); it->second.status = AttestationStatus::Signed; it->second.signer = signer; return true; @@ -54,7 +56,7 @@ public: if (!error) return false; error->clear(); auto it = items_.find(attestationId); - if (it == items_.end()) return fail(error, "attestation_missing"); + if (it == items_.end()) return failWith(error, "attestation_missing"); it->second.status = AttestationStatus::Expired; return true; } @@ -83,9 +85,4 @@ public: private: std::map items_; std::vector order_; - - static bool fail(std::string* error, const char* code) { - *error = code; - return false; - } }; diff --git a/editor/src/SecurityExceptionReviewBoard.h b/editor/src/SecurityExceptionReviewBoard.h index a599908..0e4d918 100644 --- a/editor/src/SecurityExceptionReviewBoard.h +++ b/editor/src/SecurityExceptionReviewBoard.h @@ -5,6 +5,8 @@ #include #include +#include "ValidationErrorUtil.h" + enum class ExceptionDecision { Pending, Approved, @@ -29,11 +31,11 @@ public: std::string* error) { if (!error) return false; error->clear(); - if (caseId.empty()) return fail(error, "case_id_missing"); - if (operation.empty()) return fail(error, "operation_missing"); - if (requester.empty()) return fail(error, "requester_missing"); - if (justification.empty()) return fail(error, "justification_missing"); - if (cases_.count(caseId) != 0) return fail(error, "case_duplicate"); + if (caseId.empty()) return failWith(error, "case_id_missing"); + if (operation.empty()) return failWith(error, "operation_missing"); + if (requester.empty()) return failWith(error, "requester_missing"); + if (justification.empty()) return failWith(error, "justification_missing"); + if (cases_.count(caseId) != 0) return failWith(error, "case_duplicate"); cases_[caseId] = {caseId, operation, requester, justification, ExceptionDecision::Pending, ""}; order_.push_back(caseId); return true; @@ -46,9 +48,9 @@ public: if (!error) return false; error->clear(); auto it = cases_.find(caseId); - if (it == cases_.end()) return fail(error, "case_missing"); - if (reviewer.empty()) return fail(error, "reviewer_missing"); - if (decision == ExceptionDecision::Pending) return fail(error, "decision_invalid"); + if (it == cases_.end()) return failWith(error, "case_missing"); + if (reviewer.empty()) return failWith(error, "reviewer_missing"); + if (decision == ExceptionDecision::Pending) return failWith(error, "decision_invalid"); it->second.decision = decision; it->second.reviewer = reviewer; return true; @@ -78,9 +80,4 @@ public: private: std::map cases_; std::vector order_; - - static bool fail(std::string* error, const char* code) { - *error = code; - return false; - } }; diff --git a/editor/src/ValidationErrorUtil.h b/editor/src/ValidationErrorUtil.h new file mode 100644 index 0000000..a22ea5b --- /dev/null +++ b/editor/src/ValidationErrorUtil.h @@ -0,0 +1,9 @@ +#pragma once + +#include + +inline bool failWith(std::string* error, const char* code) { + if (!error) return false; + *error = code; + return false; +} diff --git a/progress.md b/progress.md index 6673e5d..75ef56c 100644 --- a/progress.md +++ b/progress.md @@ -11665,3 +11665,48 @@ surfaces release-blocking findings. - `editor/src/ComplianceOperationalReadiness.h` within header-size limit (`60` <= `600`) - `editor/tests/step603_test.cpp` within test-file size guidance (`146` lines) - Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md` + +## Sprint 34 Refactor Pass (Architecture Compliance) +**Status:** PASS + +Completed the end-of-sprint refactor and architecture audit for Steps 594-603. +The refactor extracts repeated validation-error plumbing into a shared utility +without changing behavior. + +**Files added:** +- `editor/src/ValidationErrorUtil.h` - shared `failWith()` helper for uniform + validation failure handling across sprint compliance/security modules. + +**Files modified:** +- `editor/src/SecurityExceptionReviewBoard.h` +- `editor/src/ComplianceEvidenceBundle.h` +- `editor/src/ControlAttestationRegistry.h` +- `editor/src/ComplianceOperationalReadiness.h` + +**Refactor result:** +- Removed duplicated private `fail()` helpers from sprint modules. +- Kept all modules header-only and within architecture size constraints. +- Preserved public interfaces and runtime behavior. + +**Verification run (full sprint matrix):** +- `cmake -S editor -B editor/build-native` - PASS +- `cmake --build editor/build-native --target step594_test step595_test step596_test step597_test step598_test step599_test step600_test step601_test step602_test step603_test` - PASS +- `./editor/build-native/step594_test` - PASS (12/12) +- `./editor/build-native/step595_test` - PASS (12/12) +- `./editor/build-native/step596_test` - PASS (12/12) +- `./editor/build-native/step597_test` - PASS (12/12) +- `./editor/build-native/step598_test` - PASS (8/8) +- `./editor/build-native/step599_test` - PASS (12/12) +- `./editor/build-native/step600_test` - PASS (12/12) +- `./editor/build-native/step601_test` - PASS (12/12) +- `./editor/build-native/step602_test` - PASS (12/12) +- `./editor/build-native/step603_test` - PASS (12/12) +- Sprint 34 matrix total: **116/116 passing** + +**Architecture gate check:** +- `editor/src/ValidationErrorUtil.h` (`9` <= `600`) +- `editor/src/SecurityExceptionReviewBoard.h` (`83` <= `600`) +- `editor/src/ComplianceEvidenceBundle.h` (`54` <= `600`) +- `editor/src/ControlAttestationRegistry.h` (`88` <= `600`) +- `editor/src/ComplianceOperationalReadiness.h` (`58` <= `600`) +- Naming conventions and header-only module constraints remain aligned with `ARCHITECTURE.md`.