operation: Fix issue with re-running setup when it fails first time

When setup is run from the application thread after startup, it continuously
tries until it succeeds. However, after making the first attempt, it does not
collect the status of the operation keeping the operation object in operation
manager. When trying for the second time, trying to create operation with same
ID fails since the operation is already present.

Fix this by allowing the operation to be recreated if the existing operation has
failed.

Tests:

- Unit tests pass.

- Functional tests for bepasty app pass.

- Install an app. Create an error in the setup mechanism for an app. Increment
is app version number. Start the service and notice that setup of app is
attempted and fails. Few seconds later the setup is attempted again and the
process continues. Each time the failure is due to fault in the app's setup
method rather than operation not being accepted.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/operation.py

@@ -187,7 +187,8 @@ class OperationsManager:
     def new(self, op_id: str, *args, **kwargs) -> Operation:
         """Create a new operation instance and add to global list."""
         with self._lock:
-            if op_id in self._operations:
+            if (op_id in self._operations and self._operations[op_id].state !=
+                    Operation.State.COMPLETED):
                 raise KeyError('Operation in progress/scheduled')
 
             kwargs['on_complete'] = self._on_operation_complete

plinth/tests/test_operation.py

@@ -285,16 +285,29 @@ def test_manager_new_without_show_message():
 def test_manager_new_raises():
     """Test that a new operation is always unique."""
     manager = OperationsManager()
-    operation1 = manager.new('testop1', 'testapp', 'op1', Mock())
+    event1 = threading.Event()
+    event2 = threading.Event()
 
-    # Creating operation with same id throws exception
+    operation1 = manager.new('testop1', 'testapp', 'op1',
+                             lambda: event1.wait())
+
+    # Creating operation with different ID works
+    operation2 = manager.new('testop2', 'testapp', 'op3', lambda: event2.set())
+
+    assert manager._operations == OrderedDict(testop1=operation1,
+                                              testop2=operation2)
+
+    # Creating operation with same id while the operation is running/scheduled
+    # throws exception
     with pytest.raises(KeyError):
         manager.new('testop1', 'testapp', 'op1', Mock())
 
-    # Creating operation with different ID works
-    operation2 = manager.new('testop2', 'testapp', 'op3', Mock())
+    # Creating operation with same id after the operation is completed works.
+    event1.set()  # Allow the first operation to complete
+    event2.wait(10)  # Wait until the second operation has started
+    operation1_new = manager.new('testop1', 'testapp', 'op1', Mock())
 
-    assert manager._operations == OrderedDict(testop1=operation1,
+    assert manager._operations == OrderedDict(testop1=operation1_new,
                                               testop2=operation2)