pattern: FOR pattern correctness and unit test

* FIX: Saves previous data if an iteration has been done.  (closes inveniosoftware-contrib#39)

tests/test_patterns.py

@@ -11,6 +11,7 @@
 import os
 import time
 import random
+import pytest
 
 p = os.path.abspath(os.path.dirname(__file__) + '/../')
 if p not in sys.path:
@@ -472,3 +473,61 @@ def test_RUN_WF02(self):
         assert d.count('bum') == 2
         assert 'end' in d
         assert 'eng-end' not in d  # it must not be present if reinit=True
+
+    # ------------------- testing for -----------------------------
+    def test_FOR01(self):
+        """Test FOR pattern with different options"""
+        we = GenericWorkflowEngine()
+        doc = [[], []]
+
+        # We check normal workflow appending 50 times pony to a list.
+        # This test will check multiple object processing with for loop.
+        we.setWorkflow([cf.FOR(range(0, 50), "_loops", [a("pony")])])
+        we.process(doc)
+        # First object has been correctly processed.
+        assert len(doc[0]) == 50
+        # Second object has been correctly processed.
+        assert len(doc[1]) == 50
+        # We have done the correct number of iterations last expected
+        # value is 49. (From 0 to 49).
+        assert we.extra_data["_loops"] == 49
+
+        # He we do a special case where there is no iteration to do.
+        doc = [[], []]
+        we.setWorkflow([cf.FOR(range(0, 0), "_loops", [a("pony")])])
+        we.process(doc)
+        # First object has been correctly no processed.
+        assert len(doc[0]) == 0
+        # Second object has been correctly no processed.
+        assert len(doc[1]) == 0
+        # range will generate empty list so no object should be processed.
+        assert we.extra_data["_loops"] is None
+
+        def generate_task_list(obj, eng):
+            return [obj.append("pony")]
+
+        def generate_interval():
+            return range(0, 50)
+
+        # Same first check but with reverse order and cached list and
+        # callable branch.
+        doc = [[], []]
+        we.setWorkflow([cf.FOR(generate_interval, "_loops",
+                               generate_task_list, True, "DSC")])
+        we.process(doc)
+        # First object has been correctly processed.
+        assert len(doc[0]) == 50
+        # Second object has been correctly processed.
+        assert len(doc[1]) == 50
+        # We have done the correct number of iterations last expected
+        # value is 0. (From 49 to 0).
+        assert we.extra_data["_loops"] == 0
+
+        # We check that if the parameter we will iterate over is wrong
+        # we raise and exception.
+        with pytest.raises(TypeError):
+            doc = [[], []]
+            we = GenericWorkflowEngine()
+            # 1 is not iterable and not callable.
+            we.setWorkflow([1, "_loops", [a("pony")]])
+            we.process(doc)

workflow/patterns/controlflow.py

@@ -246,8 +246,6 @@ def FOR(get_list_function, setter, branch, cache_data=False, order="ASC"):
     :param get_list_function: function returning the list on which we should
     iterate.
     :param branch: block of functions to run
-    :param savename: name of variable to save the current loop state in the
-    extra_data in case you want to reuse the value somewhere in a task.
     :param cache_data: can be True or False in case of True, the list will be
     cached in memory instead of being recomputed everytime. In case of caching
     the list is no more dynamic.
@@ -257,8 +255,6 @@ def FOR(get_list_function, setter, branch, cache_data=False, order="ASC"):
     :param setter: function to call in order to save the current item of the
     list that is being iterated over.
     expected to take arguments (obj, eng, val)
-    :param getter: function to call in order to retrieve the current item of
-    the list that is being iterated over. expected to take arguments(obj, eng)
     """
     # be sane
     assert order in ('ASC', 'DSC')
@@ -281,49 +277,56 @@ def get_list():
                 return eng.extra_data["_Iterators"][step]["cache"]
             except KeyError:
                 if callable(get_list_function):
-                    return get_list()
+                    return get_list_function()
                 elif isinstance(get_list_function, collections.Iterable):
                     return list(get_list_function)
                 else:
-                    raise TypeError("get_list_function is not a callable nor a"
-                                    " iterable")
+                    raise TypeError("get_list_function is not callable nor an"
+                                    " iterable.")
 
-        my_list_to_process = get_list()
+        list_to_process = get_list()
 
         # First time we are in this step
         if step not in eng.extra_data["_Iterators"]:
             eng.extra_data["_Iterators"][step] = {}
             # Cache list
             if cache_data:
-                eng.extra_data["_Iterators"][step]["cache"] = get_list()
+                eng.extra_data["_Iterators"][step]["cache"] = list_to_process
             # Initialize step value
             eng.extra_data["_Iterators"][step]["value"] = {
                 "ASC": 0,
-                "DSC": len(my_list_to_process) - 1}[order]
+                "DSC": len(list_to_process) - 1}[order]
             # Store previous data
-            if 'current_data' in eng.extra_data["_Iterators"][step]:
-                eng.extra_data["_Iterators"][step]["previous_data"] = \
-                    eng.extra_data["_Iterators"][step]["current_data"]
+
+        elif 'current_data' in eng.extra_data["_Iterators"][step]:
+            eng.extra_data["_Iterators"][step]["previous_data"] = \
+                eng.extra_data["_Iterators"][step]["current_data"]
 
         # Increment or decrement step value
         step_value = eng.extra_data["_Iterators"][step]["value"]
         currently_within_list_bounds = \
-            (order == "ASC" and step_value < len(my_list_to_process)) or \
+            (order == "ASC" and step_value < len(list_to_process)) or \
             (order == "DSC" and step_value > -1)
         if currently_within_list_bounds:
             # Store current data for ourselves
             eng.extra_data["_Iterators"][step]["current_data"] = \
-                my_list_to_process[step_value]
+                list_to_process[step_value]
             # Store for the user
             if setter:
-                setter(obj, eng, step, my_list_to_process[step_value])
+                setter(obj, eng, step, list_to_process[step_value])
             if order == 'ASC':
                 eng.extra_data["_Iterators"][step]["value"] += 1
             elif order == 'DSC':
                 eng.extra_data["_Iterators"][step]["value"] -= 1
         else:
-            setter(obj, eng, step,
-                   eng.extra_data["_Iterators"][step]["previous_data"])
+            # Special case were no iteration is needed.
+            if "previous_data" in eng.extra_data["_Iterators"][step]:
+                setter(obj, eng, step,
+                       eng.extra_data["_Iterators"][step]["previous_data"])
+            else:
+                # We set None as no value should have been generated if
+                # no iteration has been done.
+                setter(obj, eng, step, None)
             del eng.extra_data["_Iterators"][step]
             eng.breakFromThisLoop()