Skip to content

Commit 0327e28

Browse files
Mohamed-Haceneab-smith
Mohamed-Hacene
and
ab-smith
authored
fix(library): stored library deletion on update (#3441)
* fix(library): stored library deletion on update * chore: format * fix: whitelist updatable fields for a requirement node * chore: format * feat: message to differentiate between addition and update * fixup --------- Co-authored-by: Abderrahmane Smimite <smimite@gmail.com>
1 parent 145a4d1 commit 0327e28

5 files changed

Lines changed: 127 additions & 38 deletions

File tree

‎backend/core/models.py‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -649,13 +649,14 @@ def update_frameworks(self):
649649
requirement_nodes = new_framework["requirement_nodes"]
650650
framework_dict = {**new_framework}
651651
del framework_dict["requirement_nodes"]
652+
framework_dict["urn"] = framework_dict["urn"].lower()
652653
prev_fw = Framework.objects.filter(urn=framework_dict["urn"]).first()
653654
prev_min = getattr(prev_fw, "min_score", None)
654655
prev_max = getattr(prev_fw, "max_score", None)
655656
prev_def = getattr(prev_fw, "scores_definition", None)
656657

657658
new_framework, _ = Framework.objects.update_or_create(
658-
urn=new_framework["urn"],
659+
urn=framework_dict["urn"],
659660
defaults=framework_dict,
660661
create_defaults={
661662
**self.referential_object_dict,
@@ -820,23 +821,68 @@ def update_frameworks(self):
820821
for k, v in requirement_node.items()
821822
if k not in ["urn", "depth", "reference_controls", "threats"]
822823
}
824+
if (
825+
"parent_urn" in requirement_node_dict
826+
and requirement_node_dict["parent_urn"]
827+
):
828+
requirement_node_dict["parent_urn"] = requirement_node_dict[
829+
"parent_urn"
830+
].lower()
823831
requirement_node_dict["order_id"] = order_id
824832
order_id += 1
825-
all_fields_to_update.update(requirement_node_dict.keys())
833+
834+
# Fields safe to update on existing requirement nodes.
835+
# Excludes structural fields (parent_urn) that
836+
# would break the framework hierarchy.
837+
UPDATABLE_FIELDS = {
838+
"name",
839+
"description",
840+
"annotation",
841+
"typical_evidence",
842+
"translations",
843+
"assessable",
844+
"implementation_groups",
845+
"questions",
846+
"weight",
847+
"importance",
848+
"order_id",
849+
}
826850

827851
if urn in existing_requirement_node_objects:
828852
requirement_node_object = existing_requirement_node_objects[urn]
829-
for key, value in requirement_node_dict.items():
853+
update_dict = {
854+
k: v
855+
for k, v in requirement_node_dict.items()
856+
if k in UPDATABLE_FIELDS
857+
}
858+
for key, value in update_dict.items():
830859
setattr(requirement_node_object, key, value)
860+
all_fields_to_update.update(update_dict.keys())
831861
requirement_node_objects_to_update.append(
832862
requirement_node_object
833863
)
834864
else:
835-
requirement_node_object = RequirementNode.objects.create(
836-
urn=urn,
837-
framework=new_framework,
838-
**self.referential_object_dict,
839-
**requirement_node_dict,
865+
existing_node = RequirementNode.objects.filter(urn=urn).first()
866+
if (
867+
existing_node
868+
and existing_node.framework_id != new_framework.id
869+
):
870+
raise ValidationError("requirementNodeUrnConflict")
871+
requirement_node_object, _ = (
872+
RequirementNode.objects.update_or_create(
873+
urn=urn,
874+
defaults={
875+
"framework": new_framework,
876+
**self.referential_object_dict,
877+
**requirement_node_dict,
878+
},
879+
create_defaults={
880+
"framework": new_framework,
881+
**self.referential_object_dict,
882+
**self.i18n_object_dict,
883+
**requirement_node_dict,
884+
},
885+
)
840886
)
841887
for ca in compliance_assessments:
842888
requirement_assessment_objects_to_create.append(

‎backend/library/views.py‎

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -464,17 +464,24 @@ def upload_library(self, request):
464464

465465
if library is not None:
466466
logger.info("Attempting to load newly uploaded library")
467+
# Check if a LoadedLibrary already exists for this urn/locale.
468+
# If so, this is an update scenario: the StoredLibrary must be
469+
# kept so the user can trigger the update via the _update endpoint.
470+
already_loaded = LoadedLibrary.objects.filter(
471+
urn=library.urn, locale=library.locale
472+
).exists()
473+
467474
try:
468475
load_error = library.load()
469476
except (ValueError, ValidationError) as load_exc:
470477
validation_detail = load_exc.args[0] if load_exc.args else None
471478
logger.error(
472-
"Validation error while loading newly uploaded library, removing stored entry",
473-
name=library.name,
479+
"Validation error while loading newly uploaded library",
474480
urn=library.urn,
475481
error=validation_detail,
476482
)
477-
library.delete()
483+
if not already_loaded:
484+
library.delete()
478485
return HttpResponse(
479486
json.dumps(
480487
{
@@ -485,13 +492,13 @@ def upload_library(self, request):
485492
),
486493
status=HTTP_422_UNPROCESSABLE_ENTITY,
487494
)
488-
except Exception:
495+
except Exception as load_exc:
489496
logger.exception(
490-
"Unexpected exception while loading newly uploaded library, removing stored entry",
497+
"Unexpected exception while loading newly uploaded library",
491498
urn=library.urn,
492-
name=library.name,
493499
)
494-
library.delete()
500+
if not already_loaded:
501+
library.delete()
495502
return HttpResponse(
496503
json.dumps(
497504
{
@@ -503,29 +510,47 @@ def upload_library(self, request):
503510
)
504511

505512
if load_error is not None:
506-
logger.error(
507-
"Failed to load newly uploaded library, removing stored entry",
508-
error=load_error,
509-
urn=library.urn,
510-
)
511-
library.delete()
512-
return HttpResponse(
513-
json.dumps(
513+
if not already_loaded:
514+
logger.error(
515+
"Failed to load newly uploaded library, removing stored entry",
516+
error=load_error,
517+
urn=library.urn,
518+
)
519+
library.delete()
520+
return HttpResponse(
521+
json.dumps(
522+
{
523+
"error": "libraryLoadFailed",
524+
"detail": load_error,
525+
}
526+
),
527+
status=HTTP_422_UNPROCESSABLE_ENTITY,
528+
)
529+
else:
530+
logger.info(
531+
"Library already loaded, stored new version for update",
532+
urn=library.urn,
533+
)
534+
return Response(
514535
{
515-
"error": "libraryLoadFailed",
516-
"detail": load_error,
517-
}
518-
),
519-
status=HTTP_422_UNPROCESSABLE_ENTITY,
520-
)
536+
**StoredLibrarySerializer(library).data,
537+
"warning": "libraryStoredForUpdate",
538+
},
539+
status=HTTP_201_CREATED,
540+
)
521541

522542
return Response(
523543
StoredLibrarySerializer(library).data, status=HTTP_201_CREATED
524544
)
525545

526546
except ValueError as e:
527547
logger.error("Failed to store library content", error=e)
528-
if library is not None:
548+
if (
549+
library is not None
550+
and not LoadedLibrary.objects.filter(
551+
urn=library.urn, locale=library.locale
552+
).exists()
553+
):
529554
library.delete()
530555
return HttpResponse(
531556
json.dumps({"error": "Failed to store library content."}),
@@ -539,7 +564,12 @@ def upload_library(self, request):
539564
)
540565
except Exception:
541566
logger.exception("Upload library failed")
542-
if library is not None:
567+
if (
568+
library is not None
569+
and not LoadedLibrary.objects.filter(
570+
urn=library.urn, locale=library.locale
571+
).exists()
572+
):
543573
library.delete()
544574
return HttpResponse(
545575
json.dumps({"error": "invalidLibraryFileError"}),

‎frontend/messages/en.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,7 @@
676676
"anErrorOccurred": "An error has occurred",
677677
"attachmentDeleted": "The attachment has been successfully deleted",
678678
"librarySuccessfullyLoaded": "The library has been successfully loaded",
679+
"libraryStoredForUpdate": "A new version of the library has been stored. Please trigger the update to apply it.",
679680
"noLibraryDetected": "No library detected",
680681
"errorLoadingLibrary": "Error loading library",
681682
"updateThisLibrary": "Update this library",

‎frontend/messages/fr.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@
630630
"anErrorOccurred": "Une erreur est survenue",
631631
"attachmentDeleted": "La pièce jointe a été supprimée avec succès",
632632
"librarySuccessfullyLoaded": "La bibliothèque a été chargée avec succès",
633+
"libraryStoredForUpdate": "Une nouvelle version de la bibliothèque a été enregistrée. Veuillez déclencher la mise à jour pour l'appliquer.",
633634
"noLibraryDetected": "Aucune bibliothèque détectée",
634635
"errorLoadingLibrary": "Erreur lors du chargement de la bibliothèque",
635636
"updateThisLibrary": "Mettre à jour cette bibliothèque",

‎frontend/src/routes/(app)/(internal)/libraries/+page.server.ts‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,24 @@ export const actions: Actions = {
101101
delete form.data['file']; // This removes a warning: Cannot stringify arbitrary non-POJOs (data..form.data.file)
102102
return fail(400, { form });
103103
}
104-
setFlash(
105-
{
106-
type: 'success',
107-
message: m.librarySuccessfullyLoaded({}, locale ? { locale } : {})
108-
},
109-
event
110-
);
104+
const response = await req.json();
105+
if (response.warning === 'libraryStoredForUpdate') {
106+
setFlash(
107+
{
108+
type: 'success',
109+
message: m.libraryStoredForUpdate({}, locale ? { locale } : {})
110+
},
111+
event
112+
);
113+
} else {
114+
setFlash(
115+
{
116+
type: 'success',
117+
message: m.librarySuccessfullyLoaded({}, locale ? { locale } : {})
118+
},
119+
event
120+
);
121+
}
111122
} else {
112123
setFlash({ type: 'error', message: m.noLibraryDetected() }, event);
113124
return fail(400, { form });

0 commit comments

Comments
 (0)