Merge pull request #89 from af83/4707-simplify-compliance-control-copy

Fixes: #4707@1.5h;

4 files changed, 41 insertions, 68 deletions

diff --git a/config/locales/referential_suites.en.yml b/config/locales/referential_suites.en.yml
new file mode 100644
index 000000000..1a77d9026
--- /dev/null
+++ b/config/locales/referential_suites.en.yml
@@ -0,0 +1,2 @@
+en:
+  referential_suites:
diff --git a/config/locales/referential_suites.fr.yml b/config/locales/referential_suites.fr.yml
new file mode 100644
index 000000000..c87b32fc7
--- /dev/null
+++ b/config/locales/referential_suites.fr.yml
@@ -0,0 +1,2 @@
+fr:
+  referential_suites:
diff --git a/lib/compliance_control_set_copier.rb b/lib/compliance_control_set_copier.rb
index b015735c2..0a688c964 100644
--- a/lib/compliance_control_set_copier.rb
+++ b/lib/compliance_control_set_copier.rb
@@ -1,7 +1,3 @@
-# We use a class instead of a singleton object because we will use
-# a cache to avoid copying instancs of ComplianceControl twice as they
-# might be reachable from CCSet **and** its descendent CCBlock.
-# More generally spoken, we copy a DAG, not a tree.
 class ComplianceControlSetCopier
 
   # Naming Convention: As we are in a domain with quite long names we
@@ -14,13 +10,12 @@ class ComplianceControlSetCopier
     @cc_set_id      = cc_set_id
     @referential_id = referential_id
     check_organisation_coherence!
-    copy_dag
+    copy_set
   end
 
 
   private
 
-
   # Workers
   # -------
   def check_organisation_coherence!
@@ -28,62 +23,48 @@ class ComplianceControlSetCopier
     raise ArgumentError, "Incoherent organisation of referential"
   end
 
-  def copy_dag
-    # Assure cck_set's existance, just in case cc_set is _empty_.
+  # Copy Set:
+  def copy_set
+    # Force lazy creation of cck_set, just in case cc_set is _empty_.
     cck_set
-    make_cck_blocks
+    # Copy all ccs -> ccks
     make_ccks_from_ccs
+    # Copy all cc_blocks -> cck_blocks
+    make_cck_blocks
   end
 
-  def make_all_cck_block_children cc_block, cck_block
-    cc_block
-      .compliance_controls
-      .each{ |compliance_control| make_compliance_check(compliance_control, cck_block.id) } 
-  end
+  # Copy Blocks:
   def make_cck_block cc_block
-    cck_block =
-      cck_set.compliance_check_blocks.create(
-        name: name_with_refid(cc_block.name))
-
-    make_all_cck_block_children cc_block, cck_block
+    cck_set.compliance_check_blocks.create(
+      name: name_with_refid(cc_block.name)).tap do | cck_block |
+        relink_checks_to_block cc_block, cck_block
+      end
   end
   def make_cck_blocks
     cc_set.compliance_control_blocks.each(&method(:make_cck_block))
   end
+  def relink_checks_to_block cc_block, cck_block
+    cc_block
+      .compliance_controls
+      .each do | compliance_control |
+        control_id_to_check[compliance_control.id]
+          .update(compliance_check_block_id: cck_block.id)
+      end
+  end
 
+  # Copy Checks:
   def make_ccks_from_ccs
     cc_set.compliance_controls.each(&method(:make_compliance_check))
   end
-
-  def make_compliance_check(compliance_control, cck_block_id = nil)
-    already_there = get_from_cache compliance_control.id
-    # We do not want to impose traversal order of the DAG, that would
-    # make the code more resistant to change...
-    # So we check if we need to update the compliance_check_block_id
-    # of a control found in the cache.
-    # N.B. By traversing the indirect descendents from a set first,
-    #      or IOW, traversing the control_blocks before the controls,
-    #      this check could go away and we could return from the
-    #      method in case of a cache hit.
-    if already_there
-      # Purely defensive:
-      if already_there.compliance_check_block_id.nil? && cck_block_id
-        already_there.update compliance_check_block_id: cck_block_id
-      end
-      return
+  def make_compliance_check(compliance_control)
+    cck_set.compliance_checks.create(
+      criticity: compliance_control.criticity,
+      name: name_with_refid(compliance_control.name),
+      code: compliance_control.code,
+      origin_code: compliance_control.origin_code
+    ).tap do | compliance_check |
+      control_id_to_check.update compliance_control.id => compliance_check
     end
-    make_compliance_check!(compliance_control, cck_block_id)
-  end
-  def make_compliance_check!(compliance_control, cck_block_id)
-    add_to_cache(
-      compliance_control.id,
-      cck_set.compliance_checks.create(
-        criticity: compliance_control.criticity,
-        name: name_with_refid(compliance_control.name),
-        code: compliance_control.code,
-        origin_code: compliance_control.origin_code,
-        compliance_check_block_id: cck_block_id
-      ))
   end
 
   def name_with_refid name
@@ -104,23 +85,11 @@ class ComplianceControlSetCopier
       status: 'new'
     )
   end
+  def control_id_to_check
+    # Map: compliance_control_id -> compliance_check
+    @__control_id_to_check__ ||= Hash.new
+  end
   def referential
     @__referential__ ||= Referential.find(referential_id)
   end
-
-  # Copy Cache
-  # ----------
-  def add_to_cache key, obj
-    # Right now we map key -> obj, in case memory consumption becomes too important
-    # we can map key -> obj.id and fetch the object from the database in get_from_cache
-    # (time vs. space tradeoff)
-    copy_cache.merge!(key => obj)
-  end
-  def get_from_cache key
-    copy_cache[key].tap do | ele |
-    end
-  end
-  def copy_cache
-    @__copy_cache__ ||= Hash.new
-  end
 end
diff --git a/spec/lib/compliance_control_set_copier_spec.rb b/spec/lib/compliance_control_set_copier_spec.rb
index b620d0657..1b4a75634 100644
--- a/spec/lib/compliance_control_set_copier_spec.rb
+++ b/spec/lib/compliance_control_set_copier_spec.rb
@@ -44,16 +44,16 @@ RSpec.describe ComplianceControlSetCopier do
       let( :ccks ){ cck_set.compliance_checks }
 
       it 'correctly creates a cck_set for a complete DAG' do
-        # Slowness of tests constrain us to create a minimum of objects in the DB,
+        # Slowness of tests constrains us to create a minimum of objects in the DB,
         # hence only one example :(
         counts = object_counts
         subject.copy(cc_set.id, ref.id)
 
         # Did not change the original objects
         # Correct numbers
-        expect( ComplianceControlSet.count).to eq(counts.cc_set_count)
-        expect( ComplianceControlBlock.count).to eq(counts.cc_block_count)
-        expect( ComplianceControl.count).to eq(counts.cc_count)
+        expect( ComplianceControlSet.count ).to eq(counts.cc_set_count)
+        expect( ComplianceControlBlock.count ).to eq(counts.cc_block_count)
+        expect( ComplianceControl.count ).to eq(counts.cc_count)
 
         expect( ComplianceCheckSet.count ).to eq(counts.cck_set_count + 1)
         expect( cck_blox.count ).to eq(counts.cck_block_count + cc_blox.size)