Repository: getmoto/moto
Pull request #6722 (2023-08-24 20:07:38)

SQS: perform validation on all messages before attempting a send
Follow up to https://github.com/getmoto/moto/pull/6721

I extracted the individual SQS message validation logic into a new `_validate_message` method and I am calling that from `send_message_batch` for all messages before attempting to send them via `send_message`. This replicates boto3 behavior. All tests are passing locally without changes required.

Tagging @bblommers for visibility.


Patch:
diff --git a/moto/identitystore/models.py b/moto/identitystore/models.py
index 456c1f7fc..8cc968675 100644
--- a/moto/identitystore/models.py
+++ b/moto/identitystore/models.py
@@ -77,6 +77,12 @@ class IdentityStoreBackend(BaseBackend):
             "limit_default": 100,
             "unique_attribute": "GroupId",
         },
+        "list_users": {
+            "input_token": "next_token",
+            "limit_key": "max_results",
+            "limit_default": 100,
+            "unique_attribute": "UserId",
+        },
     }
 
     def __init__(self, region_name: str, account_id: str) -> None:
@@ -248,6 +254,32 @@ class IdentityStoreBackend(BaseBackend):
 
         return [m for m in identity_store.groups.values()]
 
+    @paginate(pagination_model=PAGINATION_MODEL)  # type: ignore
+    def list_users(
+        self, identity_store_id: str, filters: List[Dict[str, str]]
+    ) -> List[Dict[str, str]]:
+        identity_store = self.__get_identity_store(identity_store_id)
+
+        users = []
+        if filters:
+            if filters[0].get("AttributePath") == "UserName":
+                username = filters[0].get("AttributeValue")
+
+                for m in identity_store.users.values():
+                    if m.UserName == username:
+                        user = m._asdict()
+                        if user.get("Name"):
+                            user["Name"] = m.Name._asdict()  # type: ignore
+                        users.append(user)
+                return users
+
+        for m in identity_store.users.values():
+            user = m._asdict()
+            if user.get("Name"):
+                user["Name"] = m.Name._asdict()  # type: ignore
+            users.append(user)
+        return users
+
     def delete_group_membership(
         self, identity_store_id: str, membership_id: str
     ) -> None:
diff --git a/moto/identitystore/responses.py b/moto/identitystore/responses.py
index ce18dd06a..9348f43f6 100644
--- a/moto/identitystore/responses.py
+++ b/moto/identitystore/responses.py
@@ -150,6 +150,20 @@ class IdentityStoreResponse(BaseResponse):
 
         return json.dumps(dict(Groups=groups, NextToken=next_token))
 
+    def list_users(self) -> str:
+        identity_store_id = self._get_param("IdentityStoreId")
+        max_results = self._get_param("MaxResults")
+        next_token = self._get_param("NextToken")
+        filters = self._get_param("Filters")
+        (users, next_token,) = self.identitystore_backend.list_users(
+            identity_store_id=identity_store_id,
+            max_results=max_results,
+            next_token=next_token,
+            filters=filters,
+        )
+
+        return json.dumps(dict(Users=users, NextToken=next_token))
+
     def delete_group(self) -> str:
         identity_store_id = self._get_param("IdentityStoreId")
         group_id = self._get_param("GroupId")
diff --git a/moto/sqs/models.py b/moto/sqs/models.py
index 06c27f7fd..05d03e583 100644
--- a/moto/sqs/models.py
+++ b/moto/sqs/models.py
@@ -101,7 +101,6 @@ class Message(BaseModel):
 
     @property
     def attribute_md5(self) -> str:
-
         md5 = md5_hash()
 
         for attrName in sorted(self.message_attributes.keys()):
@@ -552,7 +551,6 @@ class Queue(CloudFormationModel):
 
     def add_message(self, message: Message) -> None:
         if self.fifo_queue:
-
             # the cases in which we dedupe fifo messages
             # from https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/using-messagededuplicationid-property.html
             # https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_SendMessage.html
@@ -786,19 +784,13 @@ class SQSBackend(BaseBackend):
         queue._set_attributes(attributes)
         return queue
 
-    def send_message(
+    def _validate_message(
         self,
-        queue_name: str,
+        queue: Queue,
         message_body: str,
-        message_attributes: Optional[Dict[str, Any]] = None,
-        delay_seconds: Optional[int] = None,
         deduplication_id: Optional[str] = None,
         group_id: Optional[str] = None,
-        system_attributes: Optional[Dict[str, Any]] = None,
-    ) -> Message:
-
-        queue = self.get_queue(queue_name)
-
+    ) -> None:
         if queue.fifo_queue:
             if (
                 queue.attributes.get("ContentBasedDeduplication") == "false"
@@ -822,6 +814,33 @@ class SQSBackend(BaseBackend):
             msg = f"One or more parameters are invalid. Reason: Message must be shorter than {queue.maximum_message_size} bytes."  # type: ignore
             raise InvalidParameterValue(msg)
 
+        if group_id is None:
+            if queue.fifo_queue:
+                # MessageGroupId is a mandatory parameter for all
+                # messages in a fifo queue
+                raise MissingParameter("MessageGroupId")
+        else:
+            if not queue.fifo_queue:
+                msg = (
+                    f"Value {group_id} for parameter MessageGroupId is invalid. "
+                    "Reason: The request include parameter that is not valid for this queue type."
+                )
+                raise InvalidParameterValue(msg)
+
+    def send_message(
+        self,
+        queue_name: str,
+        message_body: str,
+        message_attributes: Optional[Dict[str, Any]] = None,
+        delay_seconds: Optional[int] = None,
+        deduplication_id: Optional[str] = None,
+        group_id: Optional[str] = None,
+        system_attributes: Optional[Dict[str, Any]] = None,
+    ) -> Message:
+        queue = self.get_queue(queue_name)
+
+        self._validate_message(queue, message_body, deduplication_id, group_id)
+
         if delay_seconds:
             delay_seconds = int(delay_seconds)
         else:
@@ -844,18 +863,7 @@ class SQSBackend(BaseBackend):
                 random.choice(string.digits) for _ in range(20)
             )
 
-        if group_id is None:
-            # MessageGroupId is a mandatory parameter for all
-            # messages in a fifo queue
-            if queue.fifo_queue:
-                raise MissingParameter("MessageGroupId")
-        else:
-            if not queue.fifo_queue:
-                msg = (
-                    f"Value {group_id} for parameter MessageGroupId is invalid. "
-                    "Reason: The request include parameter that is not valid for this queue type."
-                )
-                raise InvalidParameterValue(msg)
+        if group_id is not None:
             message.group_id = group_id
 
         if message_attributes:
@@ -877,7 +885,7 @@ class SQSBackend(BaseBackend):
     def send_message_batch(
         self, queue_name: str, entries: Dict[str, Dict[str, Any]]
     ) -> Tuple[List[Message], List[Dict[str, Any]]]:
-        self.get_queue(queue_name)
+        queue = self.get_queue(queue_name)
 
         if any(
             not re.match(r"^[\w-]{1,80}$", entry["Id"]) for entry in entries.values()
@@ -899,6 +907,16 @@ class SQSBackend(BaseBackend):
 
         messages = []
         failedInvalidDelay = []
+
+        for entry in entries.values():
+            # validate ALL messages before trying to send any
+            self._validate_message(
+                queue,
+                entry["MessageBody"],
+                entry.get("MessageDeduplicationId"),
+                entry.get("MessageGroupId"),
+            )
+
         for entry in entries.values():
             try:
                 # Loop through looking for messages
@@ -954,7 +972,6 @@ class SQSBackend(BaseBackend):
 
         # queue.messages only contains visible messages
         while True:
-
             if result or (wait_seconds_timeout and unix_time() > polling_end):
                 break
 
@@ -1045,7 +1062,6 @@ class SQSBackend(BaseBackend):
         queue = self.get_queue(queue_name)
         for message in queue._messages:
             if message.had_receipt_handle(receipt_handle):
-
                 visibility_timeout_msec = int(visibility_timeout) * 1000
                 given_visibility_timeout = unix_time_millis() + visibility_timeout_msec
                 if given_visibility_timeout - message.sent_timestamp > 43200 * 1000:  # type: ignore
diff --git a/moto/utilities/paginator.py b/moto/utilities/paginator.py
index ce411e49d..313605843 100644
--- a/moto/utilities/paginator.py
+++ b/moto/utilities/paginator.py
@@ -149,7 +149,7 @@ class Paginator:
             token_dict["parameterChecksum"] = self._param_checksum
         range_keys = []
         for attr in self._unique_attributes:
-            if type(next_item) == dict:
+            if isinstance(next_item, dict):
                 range_keys.append(str(next_item[attr]))
             else:
                 range_keys.append(str(getattr(next_item, attr)))


Test patch:
diff --git a/tests/test_identitystore/test_identitystore.py b/tests/test_identitystore/test_identitystore.py
index 73c626d15..b7e17bb19 100644
--- a/tests/test_identitystore/test_identitystore.py
+++ b/tests/test_identitystore/test_identitystore.py
@@ -91,7 +91,7 @@ def test_create_group_membership():
         Description="description",
     )["GroupId"]
 
-    user_id = __create_and_verify_sparse_user(client, identity_store_id)
+    user_id = __create_and_verify_sparse_user(client, identity_store_id)["UserId"]
 
     create_response = client.create_group_membership(
         IdentityStoreId=identity_store_id,
@@ -376,7 +376,6 @@ def test_get_group_id():
 
     # Make sure we can get their ID
     for name, group_id in groups.items():
-
         response = client.get_group_id(
             IdentityStoreId=identity_store_id,
             AlternateIdentifier={
@@ -522,7 +521,7 @@ def test_list_group_memberships():
     )["GroupId"]
 
     for _ in range(end):
-        user_id = __create_and_verify_sparse_user(client, identity_store_id)
+        user_id = __create_and_verify_sparse_user(client, identity_store_id)["UserId"]
         create_response = client.create_group_membership(
             IdentityStoreId=identity_store_id,
             GroupId=group_id,
@@ -567,6 +566,89 @@ def __check_membership_list_values(members, expected):
         assert members[i]["MemberId"]["UserId"] == expected[i][1]
 
 
+@mock_identitystore
+def test_list_users():
+    client = boto3.client("identitystore", region_name="us-east-2")
+    identity_store_id = get_identity_store_id()
+
+    start = 0
+    end = 240
+    batch_size = 100
+    next_token = None
+
+    expected_users = list()
+    for _ in range(end):
+        dummy_user = __create_and_verify_sparse_user(client, identity_store_id)
+        expected_users.append(dummy_user)
+
+    users = list()
+    for iteration in range(start, end, batch_size):
+        last_iteration = end - iteration <= batch_size
+        expected_size = batch_size if not last_iteration else end - iteration
+
+        if next_token is not None:
+            list_response = client.list_users(
+                IdentityStoreId=identity_store_id,
+                MaxResults=batch_size,
+                NextToken=next_token,
+            )
+        else:
+            list_response = client.list_users(
+                IdentityStoreId=identity_store_id,
+                MaxResults=batch_size,
+            )
+
+        assert len(list_response["Users"]) == expected_size
+        users.extend(list_response["Users"])
+        if last_iteration:
+            assert "NextToken" not in list_response
+        else:
+            assert "NextToken" in list_response
+            next_token = list_response["NextToken"]
+
+    assert users == expected_users
+
+
+@mock_identitystore
+def test_list_users_filter():
+    client = boto3.client("identitystore", region_name="us-east-2")
+    identity_store_id = get_identity_store_id()
+
+    client.create_user(
+        IdentityStoreId=identity_store_id,
+        UserName="test_username_1",
+        DisplayName="test_display_name_1",
+        Name={"GivenName": "given_name", "FamilyName": "family_name"},
+    )
+
+    # Create a second user to see if it is not returned
+    client.create_user(
+        IdentityStoreId=identity_store_id,
+        UserName="test_username_2",
+        DisplayName="test_display_name_2",
+        Name={"GivenName": "given_name", "FamilyName": "family_name"},
+    )
+
+    users = client.list_users(
+        IdentityStoreId=identity_store_id,
+        Filters=[
+            {"AttributePath": "UserName", "AttributeValue": "test_username_1"},
+        ],
+    )["Users"]
+
+    assert len(users) == 1
+    assert users[0]["UserName"] == "test_username_1"
+
+    no_users = client.list_users(
+        IdentityStoreId=identity_store_id,
+        Filters=[
+            {"AttributePath": "UserName", "AttributeValue": "non_existant_user"},
+        ],
+    )["Users"]
+
+    assert len(no_users) == 0
+
+
 @mock_identitystore
 def test_delete_group():
     client = boto3.client("identitystore", region_name="us-east-2")
@@ -600,7 +682,7 @@ def test_delete_group_doesnt_exist():
 def test_delete_group_membership():
     client = boto3.client("identitystore", region_name="eu-west-1")
     identity_store_id = get_identity_store_id()
-    user_id = __create_and_verify_sparse_user(client, identity_store_id)
+    user_id = __create_and_verify_sparse_user(client, identity_store_id)["UserId"]
     _, _, group_id = __create_test_group(client, identity_store_id)
 
     membership = client.create_group_membership(
@@ -630,7 +712,7 @@ def test_delete_group_membership():
 def test_delete_user():
     client = boto3.client("identitystore", region_name="us-east-2")
     identity_store_id = get_identity_store_id()
-    user_id = __create_and_verify_sparse_user(client, identity_store_id)
+    user_id = __create_and_verify_sparse_user(client, identity_store_id)["UserId"]
 
     client.delete_user(IdentityStoreId=identity_store_id, UserId=user_id)
 
@@ -684,7 +766,8 @@ def __group_exists(client, group_name: str, store_id: str) -> bool:
 
 
 def __create_and_verify_sparse_user(client, store_id: str):
-    rand = random.choices(string.ascii_lowercase, k=8)
+    """Creates a user, verifies it is unique and returns the response of describe_user"""
+    rand = "".join(random.choices(string.ascii_lowercase, k=8))
     username = f"the_username_{rand}"
     response = client.create_user(
         IdentityStoreId=store_id,
@@ -699,4 +782,7 @@ def __create_and_verify_sparse_user(client, store_id: str):
     )
 
     assert user_resp["UserName"] == username
-    return user_resp["UserId"]
+    del user_resp[
+        "ResponseMetadata"
+    ]  # strip response metadata and just return user info
+    return user_resp
diff --git a/tests/test_utilities/test_paginator.py b/tests/test_utilities/test_paginator.py
index 57ac62c67..5a5b5a830 100644
--- a/tests/test_utilities/test_paginator.py
+++ b/tests/test_utilities/test_paginator.py
@@ -1,3 +1,4 @@
+from collections import OrderedDict
 import unittest
 
 import pytest
@@ -38,6 +39,12 @@ def test_paginator__paginate_with_just_max_results():
     assert page == results
 
 
+def test_paginator__ordered_dict():
+    p = Paginator(max_results=1, unique_attribute="id")
+    page, _ = p.paginate([OrderedDict(x) for x in results])
+    assert len(page) == 1
+
+
 def test_paginator__paginate_without_range_key__throws_error():
     p = Paginator(max_results=2)
     with pytest.raises(KeyError):