Merge pull request #73 from mattsb42-aws/fix-72

lizroth · web-flow · commit a5680f03884e · 2018-05-21T16:27:39.000-07:00
Fix bugs identified in #72, #74, and #75
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -2,12 +2,30 @@
 Changelog
 *********
 
+1.0.4 -- 2018-05-22
+===================
+
+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
diff --git a/src/dynamodb_encryption_sdk/identifiers.py b/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__)
diff --git a/src/dynamodb_encryption_sdk/material_providers/most_recent.py b/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()
diff --git a/src/dynamodb_encryption_sdk/structures.py b/src/dynamodb_encryption_sdk/structures.py
@@ -17,7 +17,7 @@
 import six
 
 try:  # Python 3.5.0 and 3.5.1 have incompatible typing modules
-    from typing import Dict, Iterable, Optional, Set, Text  # noqa pylint: disable=unused-import
+    from typing import Dict, Iterable, List, Optional, Set, Text  # noqa pylint: disable=unused-import
 except ImportError:  # pragma: no cover
     # We only actually need these imports when running the mypy checks
     pass
@@ -295,7 +295,7 @@ class TableInfo(object):
     :param bool all_encrypting_secondary_indexes: Should we allow secondary index attributes to be encrypted?
     :param TableIndex primary_index: Description of primary index
     :param secondary_indexes: Set of TableIndex objects describing any secondary indexes
-    :type secondary_indexes: set(TableIndex)
+    :type secondary_indexes: list(TableIndex)
     """
 
     name = attr.ib(validator=attr.validators.instance_of(six.string_types))
@@ -304,15 +304,15 @@ 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
     )
 
     def __init__(
             self,
             name,  # type: Text
             primary_index=None,  # type: Optional[TableIndex]
-            secondary_indexes=None  # type: Optional[Set[TableIndex]]
+            secondary_indexes=None  # type: Optional[List[TableIndex]]
     ):  # noqa=D107
         # type: (...) -> None
         # Workaround pending resolution of attrs/mypy interaction.
@@ -338,7 +338,7 @@ def primary_index(self):
 
     @property
     def secondary_indexes(self):
-        # type: () -> Set[TableIndex]
+        # type: () -> List[TableIndex]
         """Return the primary TableIndex.
 
         :returns: secondary index descriptions
@@ -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.
diff --git a/test/functional/functional_test_utils.py b/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:
diff --git a/test/functional/material_providers/store/test_meta.py b/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
 
diff --git a/test/functional/material_providers/test_most_recent.py b/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
diff --git a/test/functional/test_structures.py b/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']))
diff --git a/tox.ini b/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
