Final update of TODOs before moving on.

Author: brunorijsman

TODO

@@ -1,27 +1,13 @@
-TODO: Stop all nodes after each test (to make sure they are stopped if test case fails)
+TODO: Stop all nodes after each test (to make sure they are stopped if test case fails).
 
-TODO: Type annotations everywhere
+TODO: Type annotations everywhere; check type annotations in check script.
 
-TODO: Check type annotations (in check script and in VSC)
+TODO: Better OpenAPI (Swagger) documentation:
+      - Better type information
+      - Better explanations
+      - All error responses
 
-TODO: OpenAPI (Swagger) documentation should show proper type for request and response data
-
-TODO: Add test case: start hub later than client (registration retry logic in client)
-
-TODO: Forbid extra query parameters for API calls
-      For example foobar in:
-      curl 'http://127.0.0.1:8105/client/carol/etsi/api/v1/keys/curtis/enc_keys?foobar=256'
-      See https://fastapi.tiangolo.com/tutorial/query-param-models/#forbid-extra-query-parameters
-
-TODO: nr_required_shares -> required_nr_shares or min_nr_shares
-
-TODO: I noticed that the response to a POST key-share is the string null; that's not right
-
-TODO: Testcase for invalid signature
-      - Bad allocation string
-      - Signature is incorrect
-      - Make sure bytes taken from pool are returned (to defend against DOS attack)
-
-TODO: Test case for: if signature validation fails, return fragment to pool
-
-TODO: Test cases for delete share after timeout
+TODO: Additional test cases (coverage closer to 100%):
+      - Start hub later than client
+      - Invalid signatures (also return allocation to pool)
+      - Pool allocations after give-backs

common/tests/test_fragment.py

@@ -114,10 +114,6 @@ def test_from_api_bad_block_uuid():
         _fragment = Fragment.from_api(api_fragment, pool)
 
 
-# TODO: Add test case: start outside of block
-# TODO: Add test case: size too large for block
-
-
 def test_to_enc_str():
     """
     Create an encoded string from a Fragment.

common/tests/test_pool.py

@@ -125,9 +125,6 @@ def test_allocate_success_empty_pool_first_block_full_second_block_full():
     assert pool.nr_used_bytes == 9
 
 
-# TODO: Also test re-allocation after giving back
-
-
 def test_allocate_failure_insufficient_space():
     """
     Attempt to allocate an allocation from a pool. There is not enough unused data in the pool.

common/tests/test_shamir.py

@@ -39,5 +39,3 @@ def test_shamir_split_reconstruct_all_scenarios():
             (10, 3),
         ]:
             shamir_split_reconstruct_scenario(size, nr_shares, min_shares)
-
-    # TODO: Key length 3 (< MIN_KEY_LENGTH) raises exception

system_tests/test_share_storage.py

@@ -2,8 +2,10 @@
 Test share storage at hubs.
 """
 
+from time import sleep
 import httpx
 import pytest
+from common.configuration import DEFAULT_SHARE_TIMEOUT_SECS
 from . import system_test_common
 
 
@@ -19,7 +21,9 @@ def setup_and_teardown():
 
 def test_shares_on_hubs():
     """
-    Test that getting a key on the master client results in shares being created on the hubs.
+    Test that getting a key on the master client results in shares being created on the hubs,
+    and that getting a with with key IDs on the slave client removes the shares (normally, unless
+    the slave SAE ID is incorrect).
     """
     # Pylint complains this looks too much like test_etsi_qkd
     # pylint: disable=duplicate-code
@@ -57,3 +61,29 @@ def test_shares_on_hubs():
     assert "shares" in status
     shares = status["shares"]
     assert len(shares) == 0
+
+
+def test_shares_on_hubs_timeout():
+    """
+    Test that shares are removed from hubs after their timeout expires (i.e. the slave client
+    does not get the key with key IDs in time).
+    """
+    # Get key on master client
+    key_id = system_test_common.get_key("sam", "sunny")
+    # There should be a share stored one each hub (we only check hank)
+    status = system_test_common.status_node("hub", "hank")
+    assert "shares" in status
+    shares = status["shares"]
+    assert len(shares) == 1
+    share = shares[0]
+    assert share["key_id"] == key_id
+    assert share["master_sae_id"] == "sam"
+    assert share["slave_sae_id"] == "sunny"
+    assert share["share_index"] == 0
+    # Wait for share to timeout
+    sleep(DEFAULT_SHARE_TIMEOUT_SECS + 10)
+    # The share should be removed from the hub
+    status = system_test_common.status_node("hub", "hank")
+    assert "shares" in status
+    shares = status["shares"]
+    assert len(shares) == 0