[Storage][STG73]Address API Review Comments

sdk/storage/azure-storage-blob/azure/storage/blob/__init__.py

@@ -53,7 +53,9 @@
     CustomerProvidedEncryptionKey,
     ContainerEncryptionScope,
     QuickQueryError,
-    DelimitedTextConfiguration
+    DelimitedTextConfiguration,
+    ObjectReplicationPolicy,
+    ObjectReplicationRule
 )
 
 __version__ = VERSION
@@ -214,5 +216,7 @@ def download_blob_from_url(
     'ContainerEncryptionScope',
     'QuickQueryError',
     'DelimitedTextConfiguration',
-    'QuickQueryReader'
+    'QuickQueryReader',
+    'ObjectReplicationPolicy',
+    'ObjectReplicationRule'
 ]

sdk/storage/azure-storage-blob/azure/storage/blob/_deserialize.py

@@ -11,7 +11,7 @@
 
 from ._shared.response_handlers import deserialize_metadata
 from ._models import BlobProperties, ContainerProperties, BlobAnalyticsLogging, Metrics, CorsRule, RetentionPolicy, \
-    StaticWebsite
+    StaticWebsite, ObjectReplicationPolicy, ObjectReplicationRule
 
 if TYPE_CHECKING:
     from azure.storage.blob._generated.models import PageList
@@ -48,13 +48,13 @@ def deserialize_ors_policies(response):
         policy_id = policy_and_rule_ids[0]
         rule_id = policy_and_rule_ids[1]
 
-        try:
-            parsed_result[policy_id][rule_id] = val
-        except KeyError:
-            # we are seeing this policy for the first time, so a new rule_id -> result dict is needed
-            parsed_result[policy_id] = {rule_id: val}
+        # If we are seeing this policy for the first time, create a new list to store rule_id -> result
+        parsed_result[policy_id] = parsed_result.get(policy_id) or list()
+        parsed_result[policy_id].append(ObjectReplicationRule(rule_id=rule_id, status=val))
 
-    return parsed_result
+    result_list = [ObjectReplicationPolicy(policy_id=k, rules=v) for k, v in parsed_result.items()]
+
+    return result_list
 
 
 def deserialize_blob_stream(response, obj, headers):

sdk/storage/azure-storage-blob/azure/storage/blob/_models.py

@@ -483,9 +483,8 @@ class BlobProperties(DictMixin):
         container-level scope is configured to allow overrides. Otherwise an error will be raised.
     :ivar bool request_server_encrypted:
         Whether this blob is encrypted.
-    :ivar dict(str, dict(str, str)) object_replication_source_properties:
+    :ivar list(~azure.storage.blob.ObjectReplicationPolicy) object_replication_source_properties:
         Only present for blobs that have policy ids and rule ids applied to them.
-        Dictionary<policy_id, Dictionary<rule_id, status of replication(complete,failed)>
     :ivar str object_replication_destination_policy:
         Represents the Object Replication Policy Id that created this blob.
     :ivar int tag_count:
@@ -657,16 +656,17 @@ def _build_item(self, item):
         return item
 
 
-class FilteredBlob(FilterBlobItem):
+class FilteredBlob(DictMixin):
     """Blob info from a Filter Blobs API call.
 
     :ivar name: Blob name
     :type name: str
     :ivar container_name: Container name.
     :type container_name: str
-    :ivar tag_value: tag value filtered by the expression.
-    :type tag_value: str
     """
+    def __init__(self, **kwargs):
+        self.name = kwargs.get('name', None)
+        self.container_name = kwargs.get('container_name', None)
 
 
 class FilteredBlobPaged(PageIterator):
@@ -732,7 +732,7 @@ def _extract_data_cb(self, get_next_return):
     @staticmethod
     def _build_item(item):
         if isinstance(item, FilterBlobItem):
-            blob = FilteredBlob(name=item.name, container_name=item.container_name, tag_value=item.tag_value)  # pylint: disable=protected-access
+            blob = FilteredBlob(name=item.name, container_name=item.container_name)  # pylint: disable=protected-access
             return blob
         return item
 
@@ -1238,6 +1238,35 @@ def __init__(self, **kwargs):
             headers_present=headers_present)
 
 
+class ObjectReplicationPolicy(DictMixin):
+    """Policy id and rule ids applied to a blob.
+
+    :ivar str policy_id:
+        Policy id for the blob. A replication policy gets created (policy id) when creating a source/destination pair.
+    :ivar list(~azure.storage.blob.ObjectReplicationRule) rules:
+        Within each policy there may be multiple replication rules.
+        e.g. rule 1= src/container/.pdf to dst/container2/; rule2 = src/container1/.jpg to dst/container3
+    """
+
+    def __init__(self, **kwargs):
+        self.policy_id = kwargs.pop('policy_id', None)
+        self.rules = kwargs.pop('rules', None)
+
+
+class ObjectReplicationRule(DictMixin):
+    """Policy id and rule ids applied to a blob.
+
+    :ivar str rule_id:
+        Rule id.
+    :ivar str status:
+        The status of the rule. It could be "Complete" or "Failed"
+    """
+
+    def __init__(self, **kwargs):
+        self.rule_id = kwargs.pop('rule_id', None)
+        self.status = kwargs.pop('status', None)
+
+
 class QuickQueryError(object):
     """The error happened during quick query operation.
 

sdk/storage/azure-storage-blob/azure/storage/blob/_shared_access_signature.py

@@ -481,7 +481,6 @@ def generate_blob_sas(
         start=None,  # type: Optional[Union[datetime, str]]
         policy_id=None,  # type: Optional[str]
         ip=None,  # type: Optional[str]
-        version_id=None,  # type: Optional[str]
         **kwargs # type: Any
     ):
     # type: (...) -> Any
@@ -498,12 +497,6 @@ def generate_blob_sas(
         The name of the blob.
     :param str snapshot:
         An optional blob snapshot ID.
-    :param str version_id:
-        An optional blob version ID. This parameter is only for versioning enabled account
-
-        .. versionadded:: 12.4.0
-            This keyword argument was introduced in API version '2019-12-12'.
-
     :param str account_key:
         The account key, also called shared key or access key, to generate the shared access signature.
         Either `account_key` or `user_delegation_key` must be specified.
@@ -545,6 +538,11 @@ def generate_blob_sas(
         or address range specified on the SAS token, the request is not authenticated.
         For example, specifying ip=168.1.5.65 or ip=168.1.5.60-168.1.5.70 on the SAS
         restricts the request to those IP addresses.
+    :keyword str version_id:
+        An optional blob version ID. This parameter is only for versioning enabled account
+
+        .. versionadded:: 12.4.0
+            This keyword argument was introduced in API version '2019-12-12'.
     :keyword str protocol:
         Specifies the protocol permitted for a request made. The default value is https.
     :keyword str cache_control:
@@ -567,6 +565,7 @@ def generate_blob_sas(
     """
     if not user_delegation_key and not account_key:
         raise ValueError("Either user_delegation_key or account_key must be provided.")
+    version_id = kwargs.pop('version_id', None)
     if version_id and snapshot:
         raise ValueError("snapshot and version_id cannot be set at the same time.")
     if user_delegation_key:

sdk/storage/azure-storage-blob/azure/storage/blob/aio/_blob_client_async.py

@@ -33,7 +33,6 @@
 
 if TYPE_CHECKING:
     from datetime import datetime
-    from azure.core.pipeline.policies import HTTPPolicy
     from .._models import (  # pylint: disable=unused-import
         ContentSettings,
         PremiumPageBlobTier,
@@ -61,12 +60,6 @@ class BlobClient(AsyncStorageAccountHostsMixin, BlobClientBase):  # pylint: disa
         account URL already has a SAS token. The value can be a SAS token string, an account
         shared access key, or an instance of a TokenCredentials class from azure.identity.
         If the URL already has a SAS token, specifying an explicit credential will take priority.
-    :keyword str version_id:
-        The version id parameter is an opaque DateTime
-        value that, when present, specifies the version of the blob to download.
-
-        .. versionadded:: 12.4.0
-        This keyword argument was introduced in API version '2019-12-12'.
     :keyword str api_version:
         The Storage API version to use for requests. Default value is '2019-07-07'.
         Setting to an older version may result in reduced feature compatibility.

sdk/storage/azure-storage-blob/tests/test_blob_tags.py

@@ -278,10 +278,5 @@ def test_filter_blobs(self, resource_group, location, storage_account, storage_a
         items_on_page2 = list(second_page)
 
         self.assertEqual(2, len(items_on_page1))
-
-        for blob in items_on_page1:
-            self.assertEqual(blob.tag_value, "firsttag")
-        for blob in items_on_page2:
-            self.assertEqual(blob.tag_value, "firsttag")
-
+        self.assertEqual(2, len(items_on_page2))
 #------------------------------------------------------------------------------

sdk/storage/azure-storage-blob/tests/test_blob_tags_async.py

@@ -285,10 +285,5 @@ async def test_filter_blobs(self, resource_group, location, storage_account, sto
             items_on_page2.append(item)
 
         self.assertEqual(2, len(items_on_page1))
-
-        for blob in items_on_page1:
-            self.assertEqual(blob.tag_value, "firsttag")
-        for blob in items_on_page2:
-            self.assertEqual(blob.tag_value, "firsttag")
-
+        self.assertEqual(2, len(items_on_page2))
 #------------------------------------------------------------------------------

sdk/storage/azure-storage-blob/tests/test_ors.py

@@ -10,7 +10,6 @@
 
 from azure.storage.blob import (
     BlobServiceClient,
-    BlobType,
     BlobProperties,
 )
 
@@ -43,14 +42,14 @@ class StubHTTPResponse:
 
         result = deserialize_ors_policies(response)
         self.assertEqual(len(result), 2)  # 2 policies
-        self.assertEqual(len(result.get('111')), 2)  # 2 rules for policy 111
-        self.assertEqual(len(result.get('222')), 2)  # 2 rules for policy 222
+        self.assertEqual(len(result[0].rules), 2)  # 2 rules for policy 111
+        self.assertEqual(len(result[1].rules), 2)  # 2 rules for policy 222
 
         # check individual result
-        self.assertEqual(result.get('111').get('111'), 'Completed')
-        self.assertEqual(result.get('111').get('222'), 'Failed')
-        self.assertEqual(result.get('222').get('111'), 'Completed')
-        self.assertEqual(result.get('222').get('222'), 'Failed')
+        self.assertEqual(result[0].rules[0].status, 'Completed' if result[0].rules[0].rule_id == '111' else 'Failed')
+        self.assertEqual(result[0].rules[1].status, 'Failed' if result[0].rules[1].rule_id == '222' else 'Completed')
+        self.assertEqual(result[1].rules[0].status, 'Completed' if result[1].rules[0].rule_id == '111' else 'Failed')
+        self.assertEqual(result[1].rules[1].status, 'Failed' if result[1].rules[1].rule_id == '222' else 'Completed')
 
     @pytest.mark.playback_test_only
     @GlobalStorageAccountPreparer()
@@ -67,14 +66,14 @@ def test_ors_source(self, resource_group, location, storage_account, storage_acc
         # Assert
         self.assertIsInstance(props, BlobProperties)
         self.assertIsNotNone(props.object_replication_source_properties)
-        for policy, rule_result in props.object_replication_source_properties.items():
-            self.assertNotEqual(policy, '')
-            self.assertIsNotNone(rule_result)
-
-            for rule_id, result in rule_result.items():
-                self.assertNotEqual(rule_id, '')
-                self.assertIsNotNone(result)
-                self.assertNotEqual(result, '')
+        for replication_policy in props.object_replication_source_properties:
+            self.assertNotEqual(replication_policy.policy_id, '')
+            self.assertIsNotNone(replication_policy.rules)
+
+            for rule in replication_policy.rules:
+                self.assertNotEqual(rule.rule_id, '')
+                self.assertIsNotNone(rule.status)
+                self.assertNotEqual(rule.status, '')
 
         # Check that the download function gives back the same result
         stream = blob.download_blob()

sdk/storage/azure-storage-blob/tests/test_ors_async.py

@@ -54,14 +54,14 @@ async def test_ors_source(self, resource_group, location, storage_account, stora
         # Assert
         self.assertIsInstance(props, BlobProperties)
         self.assertIsNotNone(props.object_replication_source_properties)
-        for policy, rule_result in props.object_replication_source_properties.items():
-            self.assertNotEqual(policy, '')
-            self.assertIsNotNone(rule_result)
-
-            for rule_id, result in rule_result.items():
-                self.assertNotEqual(rule_id, '')
-                self.assertIsNotNone(result)
-                self.assertNotEqual(result, '')
+        for replication_policy in props.object_replication_source_properties:
+            self.assertNotEqual(replication_policy.policy_id, '')
+            self.assertIsNotNone(replication_policy.rules)
+
+            for rule in replication_policy.rules:
+                self.assertNotEqual(rule.rule_id, '')
+                self.assertIsNotNone(rule.status)
+                self.assertNotEqual(rule.status, '')
 
         # Check that the download function gives back the same result
         stream = await blob.download_blob()