Remove update time from ACL delta class

lib/SampleService/core/acls.py

@@ -112,12 +112,10 @@ class SampleACLDelta():
     :ivar remove: the list of usernames to have all privileges removed.
     :ivar public_read: a boolean designating whether the sample should be made publically readable.
         None signifies no change.
-    :ivar lastupdate: the date and time the delta was applied to the ACLs.
     '''
 
     def __init__(
             self,
-            lastupdate: datetime.datetime,
             admin: Sequence[UserID] = None,
             write: Sequence[UserID] = None,
             read: Sequence[UserID] = None,
@@ -126,7 +124,6 @@ def __init__(
         '''
         Create the ACLs.
 
-        :param lastupdate: the date and time the delta was applied to the ACLs.
         :param admin: the list of usernames to be granted admin privileges.
         :param write: the list of usernames to be granted write privileges.
         :param read: the list of usernames to be granted read privileges.
@@ -135,7 +132,6 @@ def __init__(
             None signifies no change.
         :raises IllegalParameterError: If a user appears in more than one ACL
         '''
-        self.lastupdate = _check_timestamp(lastupdate, 'lastupdate')
         self.admin = _to_tuple(admin, 'admin')
         self.write = _to_tuple(write, 'write')
         self.read = _to_tuple(read, 'read')
@@ -149,17 +145,15 @@ def __init__(
 
     def __eq__(self, other):
         if type(other) is type(self):
-            return (other.lastupdate == self.lastupdate
-                    and other.admin == self.admin
+            return (other.admin == self.admin
                     and other.write == self.write
                     and other.read == self.read
                     and other.remove == self.remove
                     and other.public_read is self.public_read)
         return NotImplemented
 
     def __hash__(self):
-        return hash((self.lastupdate, self.admin, self.write, self.read, self.remove,
-                     self.public_read))
+        return hash((self.admin, self.write, self.read, self.remove, self.public_read))
 
 
 class SampleACL(SampleACLOwnerless):

lib/SampleService/core/samples.py

@@ -270,14 +270,6 @@ def update_sample_acls(
 
         self._check_perms(id_, user, _SampleAccessType.ADMIN, as_admin=as_admin)
 
-        update = SampleACLDelta(
-            self._now(),
-            update.admin,
-            update.write,
-            update.read,
-            update.remove,
-            update.public_read
-        )
         self._storage.update_sample_acls(id_, update, self._now())
         if self._kafka:
             self._kafka.notify_sample_acl_change(id_)

test/core/acls_test.py

@@ -188,46 +188,46 @@ def test_is_update():
         [u('r1'), u('r2')],
         True)
 
-    assert s.is_update(SampleACLDelta(dt(2))) is False
+    assert s.is_update(SampleACLDelta()) is False
 
-    assert s.is_update(SampleACLDelta(dt(2), [u('a1')])) is False
-    assert s.is_update(SampleACLDelta(dt(2), [u('a3')])) is True
+    assert s.is_update(SampleACLDelta([u('a1')])) is False
+    assert s.is_update(SampleACLDelta([u('a3')])) is True
 
-    assert s.is_update(SampleACLDelta(dt(2), write=[u('w2')])) is False
-    assert s.is_update(SampleACLDelta(dt(2), write=[u('w4')])) is True
+    assert s.is_update(SampleACLDelta(write=[u('w2')])) is False
+    assert s.is_update(SampleACLDelta(write=[u('w4')])) is True
 
-    assert s.is_update(SampleACLDelta(dt(2), read=[u('r1')])) is False
-    assert s.is_update(SampleACLDelta(dt(2), read=[u('r3')])) is True
+    assert s.is_update(SampleACLDelta(read=[u('r1')])) is False
+    assert s.is_update(SampleACLDelta(read=[u('r3')])) is True
 
-    assert s.is_update(SampleACLDelta(dt(2), remove=[u('a1')])) is True
-    assert s.is_update(SampleACLDelta(dt(2), remove=[u('a3')])) is False
+    assert s.is_update(SampleACLDelta(remove=[u('a1')])) is True
+    assert s.is_update(SampleACLDelta(remove=[u('a3')])) is False
 
-    assert s.is_update(SampleACLDelta(dt(2), remove=[u('w2')])) is True
-    assert s.is_update(SampleACLDelta(dt(2), remove=[u('w4')])) is False
+    assert s.is_update(SampleACLDelta(remove=[u('w2')])) is True
+    assert s.is_update(SampleACLDelta(remove=[u('w4')])) is False
 
-    assert s.is_update(SampleACLDelta(dt(2), remove=[u('r1')])) is True
-    assert s.is_update(SampleACLDelta(dt(2), remove=[u('r3')])) is False
+    assert s.is_update(SampleACLDelta(remove=[u('r1')])) is True
+    assert s.is_update(SampleACLDelta(remove=[u('r3')])) is False
 
-    assert s.is_update(SampleACLDelta(dt(2), public_read=False)) is True
-    assert s.is_update(SampleACLDelta(dt(2), public_read=None)) is False
-    assert s.is_update(SampleACLDelta(dt(2), public_read=True)) is False
+    assert s.is_update(SampleACLDelta(public_read=False)) is True
+    assert s.is_update(SampleACLDelta(public_read=None)) is False
+    assert s.is_update(SampleACLDelta(public_read=True)) is False
 
 
 def test_is_update_fail():
     s = SampleACL(u('u'), dt(1))
 
     _is_update_fail(s, None, ValueError('update cannot be a value that evaluates to false'))
     _is_update_fail(
-        s, SampleACLDelta(dt(2), [u('a'), u('u')], [u('v')]),
+        s, SampleACLDelta([u('a'), u('u')], [u('v')]),
         UnauthorizedError('ACLs for the sample owner u may not be modified by a delta update.'))
     _is_update_fail(
-        s, SampleACLDelta(dt(2), [u('a')], write=[u('v'), u('u')]),
+        s, SampleACLDelta([u('a')], write=[u('v'), u('u')]),
         UnauthorizedError('ACLs for the sample owner u may not be modified by a delta update.'))
     _is_update_fail(
-        s, SampleACLDelta(dt(2), [u('a')], read=[u('v'), u('u')]),
+        s, SampleACLDelta([u('a')], read=[u('v'), u('u')]),
         UnauthorizedError('ACLs for the sample owner u may not be modified by a delta update.'))
     _is_update_fail(
-        s, SampleACLDelta(dt(2), [u('a')], remove=[u('v'), u('u')]),
+        s, SampleACLDelta([u('a')], remove=[u('v'), u('u')]),
         UnauthorizedError('ACLs for the sample owner u may not be modified by a delta update.'))
 
 
@@ -291,38 +291,33 @@ def test_hash():
 
 
 def test_delta_build():
-    a = SampleACLDelta(dt(30))
-    assert a.lastupdate == dt(30)
+    a = SampleACLDelta()
     assert a.admin == ()
     assert a.write == ()
     assert a.read == ()
     assert a.remove == ()
     assert a.public_read is None
 
     a = SampleACLDelta(
-        dt(-56),
         [u('baz'), u('bat'), u('baz')],
         read=[u('wheee'), u('wheee')],
         write=[u('wugga'), u('a'), u('b'), u('wugga')],
         remove=[u('bleah'), u('ffs'), u('c')],
         public_read=True)
-    assert a.lastupdate == dt(-56)
     assert a.admin == (u('bat'), u('baz'))
     assert a.write == (u('a'), u('b'), u('wugga'))
     assert a.read == (u('wheee'),)
     assert a.remove == (u('bleah'), u('c'), u('ffs'))
     assert a.public_read is True
 
-    a = SampleACLDelta(dt(30), public_read=False)
-    assert a.lastupdate == dt(30)
+    a = SampleACLDelta(public_read=False)
     assert a.admin == ()
     assert a.write == ()
     assert a.read == ()
     assert a.remove == ()
     assert a.public_read is False
 
-    a = SampleACLDelta(dt(30), public_read=None)
-    assert a.lastupdate == dt(30)
+    a = SampleACLDelta(public_read=None)
     assert a.admin == ()
     assert a.write == ()
     assert a.read == ()
@@ -331,102 +326,84 @@ def test_delta_build():
 
 
 def test_delta_build_fail():
-    t = dt(1)
-    _build_delta_fail(None, None, None, None, None, ValueError(
-        'lastupdate cannot be a value that evaluates to false'))
-    _build_delta_fail(datetime.datetime.fromtimestamp(1), None, None, None, None, ValueError(
-        'lastupdate cannot be a naive datetime'))
-    _build_delta_fail(t, [u('a'), None], None, None, None, ValueError(
+    _build_delta_fail([u('a'), None], None, None, None, ValueError(
         'Index 1 of iterable admin cannot be a value that evaluates to false'))
-    _build_delta_fail(t, None, [None, None], None, None, ValueError(
+    _build_delta_fail(None, [None, None], None, None, ValueError(
         'Index 0 of iterable write cannot be a value that evaluates to false'))
-    _build_delta_fail(t, None, None, [u('a'), u('b'), None], None, ValueError(
+    _build_delta_fail(None, None, [u('a'), u('b'), None], None, ValueError(
         'Index 2 of iterable read cannot be a value that evaluates to false'))
-    _build_delta_fail(t, None, None, None, [None], ValueError(
+    _build_delta_fail(None, None, None, [None], ValueError(
         'Index 0 of iterable remove cannot be a value that evaluates to false'))
 
     # test that you cannot have a user in 2 acls
     _build_delta_fail(
-        t, [u('a'), u('z')], [u('a'), u('c')], [u('w'), u('b')], None,
+        [u('a'), u('z')], [u('a'), u('c')], [u('w'), u('b')], None,
         IllegalParameterError('User a appears in two ACLs'))
     _build_delta_fail(
-        t, [u('a'), u('z')], [u('b'), u('c')], [u('w'), u('a')], None,
+        [u('a'), u('z')], [u('b'), u('c')], [u('w'), u('a')], None,
         IllegalParameterError('User a appears in two ACLs'))
     _build_delta_fail(
-        t, [u('x'), u('z')], [u('b'), u('c'), u('w')], [u('w'), u('a')], None,
+        [u('x'), u('z')], [u('b'), u('c'), u('w')], [u('w'), u('a')], None,
         IllegalParameterError('User w appears in two ACLs'))
 
     # test that you cannot have a user in the remove list and an acl
     _build_delta_fail(
-        t, [u('f'), u('z')], [u('b'), u('c'), u('g')], [u('w'), u('a')], [u('m'), u('f')],
+        [u('f'), u('z')], [u('b'), u('c'), u('g')], [u('w'), u('a')], [u('m'), u('f')],
         IllegalParameterError('Users in the remove list cannot be in any other ACL'))
     _build_delta_fail(
-        t, [u('a'), u('z')], [u('x'), u('c')], [u('w'), u('b')], [u('m'), u('x')],
+        [u('a'), u('z')], [u('x'), u('c')], [u('w'), u('b')], [u('m'), u('x')],
         IllegalParameterError('Users in the remove list cannot be in any other ACL'))
     _build_delta_fail(
-        t, [u('a'), u('z')], [u('b'), u('c')], [u('w'), u('y')], [u('y')],
+        [u('a'), u('z')], [u('b'), u('c')], [u('w'), u('y')], [u('y')],
         IllegalParameterError('Users in the remove list cannot be in any other ACL'))
 
 
-def _build_delta_fail(lastchanged, admin, write, read, remove, expected):
+def _build_delta_fail(admin, write, read, remove, expected):
     with raises(Exception) as got:
-        SampleACLDelta(lastchanged, admin, write, read, remove)
+        SampleACLDelta(admin, write, read, remove)
     assert_exception_correct(got.value, expected)
 
 
 def test_delta_eq():
-    t = dt(3)
-    assert SampleACLDelta(t) == SampleACLDelta(t)
-    assert SampleACLDelta(t) != SampleACLDelta(dt(7))
+    assert SampleACLDelta() == SampleACLDelta()
 
-    assert SampleACLDelta(t, [u('bar')]) == SampleACLDelta(t, [u('bar')])
-    assert SampleACLDelta(t, [u('bar')]) != SampleACLDelta(t, [u('baz')])
+    assert SampleACLDelta([u('bar')]) == SampleACLDelta([u('bar')])
+    assert SampleACLDelta([u('bar')]) != SampleACLDelta([u('baz')])
 
-    assert SampleACLDelta(t, write=[u('bar')]) == SampleACLDelta(t, write=[u('bar')])
-    assert SampleACLDelta(t, write=[u('bar')]) != SampleACLDelta(t, write=[u('baz')])
+    assert SampleACLDelta(write=[u('bar')]) == SampleACLDelta(write=[u('bar')])
+    assert SampleACLDelta(write=[u('bar')]) != SampleACLDelta(write=[u('baz')])
 
-    assert SampleACLDelta(t, read=[u('bar')]) == SampleACLDelta(t, read=[u('bar')])
-    assert SampleACLDelta(t, read=[u('bar')]) != SampleACLDelta(t, read=[u('baz')])
+    assert SampleACLDelta(read=[u('bar')]) == SampleACLDelta(read=[u('bar')])
+    assert SampleACLDelta(read=[u('bar')]) != SampleACLDelta(read=[u('baz')])
 
-    assert SampleACLDelta(t, remove=[u('bar')]) == SampleACLDelta(t, remove=[u('bar')])
-    assert SampleACLDelta(t, remove=[u('bar')]) != SampleACLDelta(t, remove=[u('baz')])
+    assert SampleACLDelta(remove=[u('bar')]) == SampleACLDelta(remove=[u('bar')])
+    assert SampleACLDelta(remove=[u('bar')]) != SampleACLDelta(remove=[u('baz')])
 
-    assert SampleACLDelta(t, public_read=True) == SampleACLDelta(t, public_read=True)
-    assert SampleACLDelta(t, public_read=True) != SampleACLDelta(t, public_read=False)
+    assert SampleACLDelta(public_read=True) == SampleACLDelta(public_read=True)
+    assert SampleACLDelta(public_read=True) != SampleACLDelta(public_read=False)
 
-    assert SampleACLDelta(t) != 1
-    assert t != SampleACLDelta(t)
+    assert SampleACLDelta() != 1
+    assert [] != SampleACLDelta()
 
 
 def test_delta_hash():
     # hashes will change from instance to instance of the python interpreter, and therefore
     # tests can't be written that directly test the hash value. See
     # https://docs.python.org/3/reference/datamodel.html#object.__hash__
 
-    t = dt(56)
-
-    assert hash(SampleACLDelta(t)) == hash(SampleACLDelta(t))
-    assert hash(SampleACLDelta(t)) != hash(SampleACLDelta(dt(55)))
+    assert hash(SampleACLDelta()) == hash(SampleACLDelta())
 
-    assert hash(SampleACLDelta(t, [u('bar')])) == hash(SampleACLDelta(t, [u('bar')]))
-    assert hash(SampleACLDelta(t, [u('bar')])) != hash(SampleACLDelta(t, [u('baz')]))
+    assert hash(SampleACLDelta([u('bar')])) == hash(SampleACLDelta([u('bar')]))
+    assert hash(SampleACLDelta([u('bar')])) != hash(SampleACLDelta([u('baz')]))
 
-    assert hash(SampleACLDelta(t, write=[u('bar')])) == hash(
-        SampleACLDelta(t, write=[u('bar')]))
-    assert hash(SampleACLDelta(t, write=[u('bar')])) != hash(
-        SampleACLDelta(t, write=[u('baz')]))
+    assert hash(SampleACLDelta(write=[u('bar')])) == hash(SampleACLDelta(write=[u('bar')]))
+    assert hash(SampleACLDelta(write=[u('bar')])) != hash(SampleACLDelta(write=[u('baz')]))
 
-    assert hash(SampleACLDelta(t, read=[u('bar')])) == hash(
-        SampleACLDelta(t, read=[u('bar')]))
-    assert hash(SampleACLDelta(t, read=[u('bar')])) != hash(
-        SampleACLDelta(t, read=[u('baz')]))
+    assert hash(SampleACLDelta(read=[u('bar')])) == hash(SampleACLDelta(read=[u('bar')]))
+    assert hash(SampleACLDelta(read=[u('bar')])) != hash(SampleACLDelta(read=[u('baz')]))
 
-    assert hash(SampleACLDelta(t, remove=[u('bar')])) == hash(
-        SampleACLDelta(t, remove=[u('bar')]))
-    assert hash(SampleACLDelta(t, remove=[u('bar')])) != hash(
-        SampleACLDelta(t, remove=[u('baz')]))
+    assert hash(SampleACLDelta(remove=[u('bar')])) == hash(SampleACLDelta(remove=[u('bar')]))
+    assert hash(SampleACLDelta(remove=[u('bar')])) != hash(SampleACLDelta(remove=[u('baz')]))
 
-    assert hash(SampleACLDelta(t, public_read=True)) == hash(
-        SampleACLDelta(t, public_read=True))
-    assert hash(SampleACLDelta(t, public_read=True)) != hash(
-        SampleACLDelta(t, public_read=False))
+    assert hash(SampleACLDelta(public_read=True)) == hash(SampleACLDelta(public_read=True))
+    assert hash(SampleACLDelta(public_read=True)) != hash(SampleACLDelta(public_read=False))

test/core/samples_test.py

@@ -868,7 +868,7 @@ def _update_sample_acls(user: UserID, public_read):
         [u('Fungus J. Pustule Jr.'), u('x')])
 
     samples.update_sample_acls(id_, user, SampleACLDelta(
-        dt(-100), [u('x'), u('y')], [u('z'), u('a')], [u('b'), u('c')], [u('r'), u('q')],
+        [u('x'), u('y')], [u('z'), u('a')], [u('b'), u('c')], [u('r'), u('q')],
         public_read))
 
     lu.invalid_users.assert_called_once_with(
@@ -879,7 +879,6 @@ def _update_sample_acls(user: UserID, public_read):
     storage.update_sample_acls.assert_called_once_with(
         UUID('1234567890abcdef1234567890abcde0'),
         SampleACLDelta(
-            dt(6),
             [u('x'), u('y')],
             [u('z'), u('a')],
             [u('b'), u('c')],
@@ -913,7 +912,7 @@ def test_update_sample_acls_as_admin_without_notifier():
         [u('Fungus J. Pustule Jr.'), u('x')])
 
     samples.update_sample_acls(id_, UserID('someguy'), SampleACLDelta(
-        dt(-100), [u('x'), u('y')], [u('z'), u('a')], [u('b'), u('c')], [u('r'), u('q')],
+        [u('x'), u('y')], [u('z'), u('a')], [u('b'), u('c')], [u('r'), u('q')],
         None),
         as_admin=True)
 
@@ -923,7 +922,6 @@ def test_update_sample_acls_as_admin_without_notifier():
     storage.update_sample_acls.assert_called_once_with(
         UUID('1234567890abcdef1234567890abcde0'),
         SampleACLDelta(
-            dt(6),
             [u('x'), u('y')],
             [u('z'), u('a')],
             [u('b'), u('c')],
@@ -942,9 +940,9 @@ def test_update_sample_acls_fail_bad_input():
     id_ = UUID('1234567890abcdef1234567890abcde0')
     u = UserID('u')
 
-    _update_sample_acls_fail(samples, None, u, SampleACLDelta(dt(1)), ValueError(
+    _update_sample_acls_fail(samples, None, u, SampleACLDelta(), ValueError(
         'id_ cannot be a value that evaluates to false'))
-    _update_sample_acls_fail(samples, id_, None, SampleACLDelta(dt(1)), ValueError(
+    _update_sample_acls_fail(samples, id_, None, SampleACLDelta(), ValueError(
         'user cannot be a value that evaluates to false'))
     _update_sample_acls_fail(samples, id_, u, None, ValueError(
         'update cannot be a value that evaluates to false'))
@@ -962,7 +960,6 @@ def test_update_sample_acls_fail_nonexistent_user_5_users():
     lu.invalid_users.return_value = [u('whoo'), u('yay'), u('bugga'), u('w'), u('c')]
 
     acls = SampleACLDelta(
-        dt(1),
         [u('x'), u('whoo')],
         [u('yay'), u('fwew')],
         [u('y'), u('bugga'), u('z'), u('w'), u('c')],
@@ -988,7 +985,6 @@ def test_update_sample_acls_fail_nonexistent_user_6_users():
     lu.invalid_users.return_value = [u('whoo'), u('yay'), u('bugga'), u('w'), u('c'), u('whee')]
 
     acls = SampleACLDelta(
-        dt(6),
         [u('x'), u('whoo')],
         [u('yay'), u('fwew')],
         [u('y'), u('bugga'), u('z'), u('w'), u('c'), u('whee')],
@@ -1014,7 +1010,6 @@ def test_update_sample_acls_fail_invalid_user():
     lu.invalid_users.side_effect = user_lookup.InvalidUserError('o shit waddup')
 
     acls = SampleACLDelta(
-        dt(67),
         [u('o shit waddup'), u('whoo')],
         [u('yay'), u('fwew')],
         [u('y'), u('bugga'), u('z')])
@@ -1037,7 +1032,6 @@ def test_update_sample_acls_fail_invalid_token():
     lu.invalid_users.side_effect = user_lookup.InvalidTokenError('you big dummy')
 
     acls = SampleACLDelta(
-        dt(1),
         [u('x'), u('whoo')],
         [u('yay'), u('fwew')],
         [u('y'), u('bugga'), u('z')])
@@ -1074,7 +1068,7 @@ def _update_sample_acls_fail_unauthorized(user: UserID):
         [u('Fungus J. Pustule Jr.'), u('x')],
         public_read=True)  # public read shouldn't grant privs.
 
-    _update_sample_acls_fail(samples, id_, user, SampleACLDelta(dt(1)), UnauthorizedError(
+    _update_sample_acls_fail(samples, id_, user, SampleACLDelta(), UnauthorizedError(
         f'User {user} cannot administrate sample 12345678-90ab-cdef-1234-567890abcde0'))
 
     assert lu.invalid_users.call_args_list == [(([],), {})]