Fix bugs identified in #72, #74, and #75

CHANGELOG.rst

@@ -2,12 +2,30 @@
 Changelog
 *********
 
+1.0.4 -- 2018-05-xx
+===================
+
+Bugfixes
+--------
+* Fix :class:`MostRecentProvider` behavior when lock cannot be acquired.
+  `#72 <https://github.com/awslabs/aws-dynamodb-encryption-python/issues/72>`_
+* Fix :class:`MostRecentProvider` lock acquisition for Python 2.7.
+  `#74 <https://github.com/awslabs/aws-dynamodb-encryption-python/issues/74>`_
+* Fix :class:`TableInfo` secondary index storage.
+  `#75 <https://github.com/awslabs/aws-dynamodb-encryption-python/issues/75>`_
+
 1.0.3 -- 2018-05-03
 ===================
+
+Bugfixes
+--------
 * Finish fixing ``MANIFEST.in``.
 
 1.0.2 -- 2018-05-03
 ===================
+
+Bugfixes
+--------
 * Fill out ``MANIFEST.in`` to correctly include necessary files in source build.
 
 1.0.1 -- 2018-05-02

src/dynamodb_encryption_sdk/identifiers.py

@@ -14,7 +14,7 @@
 from enum import Enum
 
 __all__ = ('LOGGER_NAME', 'CryptoAction', 'EncryptionKeyType', 'KeyEncodingType')
-__version__ = '1.0.3'
+__version__ = '1.0.4'
 
 LOGGER_NAME = 'dynamodb_encryption_sdk'
 USER_AGENT_SUFFIX = 'DynamodbEncryptionSdkPython/{}'.format(__version__)

src/dynamodb_encryption_sdk/material_providers/most_recent.py

@@ -182,7 +182,7 @@ def decryption_materials(self, encryption_context):
         return provider.decryption_materials(encryption_context)
 
     def _ttl_action(self):
-        # type: () -> bool
+        # type: () -> TtlActions
         """Determine the correct action to take based on the local resources and TTL.
 
         :returns: decision
@@ -240,14 +240,14 @@ def _get_most_recent_version(self, allow_local):
         :returns: version and corresponding cryptographic materials provider
         :rtype: CryptographicMaterialsProvider
         """
-        acquired = self._lock.acquire(blocking=not allow_local)
+        acquired = self._lock.acquire(not allow_local)
 
         if not acquired:
             # We failed to acquire the lock.
             # If blocking, we will never reach this point.
             # If not blocking, we want whatever the latest local version is.
             version = self._version
-            return version, self._cache.get(version)
+            return self._cache.get(version)
 
         try:
             max_version = self._get_max_version()

src/dynamodb_encryption_sdk/structures.py

@@ -304,7 +304,7 @@ class TableInfo(object):
         default=None
     )
     _secondary_indexes = attr.ib(
-        validator=attr.validators.optional(iterable_validator(set, TableIndex)),
+        validator=attr.validators.optional(iterable_validator(list, TableIndex)),
         default=None
     )
 
@@ -378,10 +378,10 @@ def refresh_indexed_attributes(self, client):
         table = client.describe_table(TableName=self.name)['Table']
         self._primary_index = TableIndex.from_key_schema(table['KeySchema'])
 
-        self._secondary_indexes = set()
+        self._secondary_indexes = []
         for group in ('LocalSecondaryIndexes', 'GlobalSecondaryIndexes'):
             try:
                 for index in table[group]:
-                    self._secondary_indexes.add(TableIndex.from_key_schema(index['KeySchema']))
+                    self._secondary_indexes.append(TableIndex.from_key_schema(index['KeySchema']))
             except KeyError:
                 pass  # Not all tables will have secondary indexes.

test/functional/functional_test_utils.py

@@ -48,6 +48,16 @@
         'value': Decimal('99.233')
     }
 }
+SECONARY_INDEX = {
+    'secondary_index_1': {
+        'type': 'B',
+        'value': Binary(b'\x00\x01\x02')
+    },
+    'secondary_index_1': {
+        'type': 'S',
+        'value': 'another_value'
+    }
+}
 TEST_KEY = {name: value['value'] for name, value in TEST_INDEX.items()}
 TEST_BATCH_INDEXES = [
     {
@@ -130,6 +140,132 @@ def example_table():
     mock_dynamodb2().stop()
 
 
+@pytest.fixture
+def table_with_local_seconary_indexes():
+    mock_dynamodb2().start()
+    ddb = boto3.client('dynamodb', region_name='us-west-2')
+    ddb.create_table(
+        TableName=TEST_TABLE_NAME,
+        KeySchema=[
+            {
+                'AttributeName': 'partition_attribute',
+                'KeyType': 'HASH'
+            },
+            {
+                'AttributeName': 'sort_attribute',
+                'KeyType': 'RANGE'
+            }
+        ],
+        LocalSecondaryIndexes=[
+            {
+                'IndexName': 'lsi-1',
+                'KeySchema': [
+                    {
+                        'AttributeName': 'secondary_index_1',
+                        'KeyType': 'HASH'
+                    }
+                ],
+                'Projection': {
+                    'ProjectionType': 'ALL'
+                }
+            },
+            {
+                'IndexName': 'lsi-2',
+                'KeySchema': [
+                    {
+                        'AttributeName': 'secondary_index_2',
+                        'KeyType': 'HASH'
+                    }
+                ],
+                'Projection': {
+                    'ProjectionType': 'ALL'
+                }
+            }
+        ],
+        AttributeDefinitions=[
+            {
+                'AttributeName': name,
+                'AttributeType': value['type']
+            }
+            for name, value in list(TEST_INDEX.items()) + list(SECONARY_INDEX.items())
+        ],
+        ProvisionedThroughput={
+            'ReadCapacityUnits': 100,
+            'WriteCapacityUnits': 100
+        }
+    )
+    yield
+    ddb.delete_table(TableName=TEST_TABLE_NAME)
+    mock_dynamodb2().stop()
+
+
+@pytest.fixture
+def table_with_global_seconary_indexes():
+    mock_dynamodb2().start()
+    ddb = boto3.client('dynamodb', region_name='us-west-2')
+    ddb.create_table(
+        TableName=TEST_TABLE_NAME,
+        KeySchema=[
+            {
+                'AttributeName': 'partition_attribute',
+                'KeyType': 'HASH'
+            },
+            {
+                'AttributeName': 'sort_attribute',
+                'KeyType': 'RANGE'
+            }
+        ],
+        GlobalSecondaryIndexes=[
+            {
+                'IndexName': 'gsi-1',
+                'KeySchema': [
+                    {
+                        'AttributeName': 'secondary_index_1',
+                        'KeyType': 'HASH'
+                    }
+                ],
+                'Projection': {
+                    'ProjectionType': 'ALL'
+                },
+                'ProvisionedThroughput': {
+                    'ReadCapacityUnits': 100,
+                    'WriteCapacityUnits': 100
+                }
+            },
+            {
+                'IndexName': 'gsi-2',
+                'KeySchema': [
+                    {
+                        'AttributeName': 'secondary_index_2',
+                        'KeyType': 'HASH'
+                    }
+                ],
+                'Projection': {
+                    'ProjectionType': 'ALL'
+                },
+                'ProvisionedThroughput': {
+                    'ReadCapacityUnits': 100,
+                    'WriteCapacityUnits': 100
+                }
+            }
+        ],
+        AttributeDefinitions=[
+            {
+                'AttributeName': name,
+                'AttributeType': value['type']
+            }
+            for name, value in list(TEST_INDEX.items()) + list(SECONARY_INDEX.items())
+        ],
+        ProvisionedThroughput={
+            'ReadCapacityUnits': 100,
+            'WriteCapacityUnits': 100
+        }
+    )
+    yield
+    ddb.delete_table(TableName=TEST_TABLE_NAME)
+    mock_dynamodb2().stop()
+
+
 def _get_from_cache(dk_class, algorithm, key_length):
     """Don't generate new keys every time. All we care about is that they are valid keys, not that they are unique."""
     try:

test/functional/material_providers/store/test_meta.py

@@ -10,7 +10,7 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
-"""Integration tests for ``dynamodb_encryption_sdk.material_providers.store.meta``."""
+"""Functional tests for ``dynamodb_encryption_sdk.material_providers.store.meta``."""
 import base64
 import os
 

test/functional/material_providers/test_most_recent.py

@@ -0,0 +1,37 @@
+# Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+"""Functional tests for ``dynamodb_encryption_sdk.material_providers.most_recent``."""
+from mock import MagicMock, sentinel
+import pytest
+
+from dynamodb_encryption_sdk.material_providers.most_recent import MostRecentProvider
+from dynamodb_encryption_sdk.material_providers.store import ProviderStore
+
+pytestmark = [pytest.mark.functional, pytest.mark.local]
+
+
+def test_failed_lock_acquisition():
+    store = MagicMock(__class__=ProviderStore)
+    provider = MostRecentProvider(
+        provider_store=store,
+        material_name='my material',
+        version_ttl=10.0
+    )
+    provider._version = 9
+    provider._cache.put(provider._version, sentinel.nine)
+
+    with provider._lock:
+        test = provider._get_most_recent_version(allow_local=True)
+
+    assert test is sentinel.nine
+    assert not store.mock_calls

test/functional/test_structures.py

@@ -11,15 +11,42 @@
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
 """Functional tests for ``dynamodb_encryption_sdk.structures``."""
+import boto3
 import pytest
 
 from dynamodb_encryption_sdk.exceptions import InvalidArgumentError
 from dynamodb_encryption_sdk.identifiers import CryptoAction
-from dynamodb_encryption_sdk.structures import AttributeActions, TableIndex
+from dynamodb_encryption_sdk.structures import AttributeActions, TableIndex, TableInfo
+from .functional_test_utils import (
+    example_table, table_with_global_seconary_indexes, table_with_local_seconary_indexes, TEST_TABLE_NAME
+)
 
 pytestmark = [pytest.mark.functional, pytest.mark.local]
 
 
+# TODO: There is a way to parameterize fixtures, but the existing docs on that are very unclear.
+# This will get us what we need for now, but we should come back to this to clean this up later.
+def test_tableinfo_refresh_indexes_no_secondary_indexes(example_table):
+    client = boto3.client('dynamodb', region_name='us-west-2')
+    table = TableInfo(name=TEST_TABLE_NAME)
+
+    table.refresh_indexed_attributes(client)
+
+
+def test_tableinfo_refresh_indexes_with_gsis(table_with_global_seconary_indexes):
+    client = boto3.client('dynamodb', region_name='us-west-2')
+    table = TableInfo(name=TEST_TABLE_NAME)
+
+    table.refresh_indexed_attributes(client)
+
+
+def test_tableinfo_refresh_indexes_with_lsis(table_with_local_seconary_indexes):
+    client = boto3.client('dynamodb', region_name='us-west-2')
+    table = TableInfo(name=TEST_TABLE_NAME)
+
+    table.refresh_indexed_attributes(client)
+
+
 @pytest.mark.parametrize('kwargs, expected_attributes', (
     (dict(partition='partition_name'), set(['partition_name'])),
     (dict(partition='partition_name', sort='sort_name'), set(['partition_name', 'sort_name']))

tox.ini

@@ -259,7 +259,8 @@ commands =
 basepython = python3
 deps = -rdoc/requirements.txt
 commands =
-    sphinx-build -E -c doc/ -b html -b linkcheck doc/ doc/build/html
+    sphinx-build -E -c doc/ -b html doc/ doc/build/html
+    sphinx-build -E -c doc/ -b linkcheck doc/ doc/build/html
 
 [testenv:serve-docs]
 basepython = python3