Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing array of messages to send_messages #140

Merged
merged 2 commits into from
Oct 19, 2015
Merged

Allow passing array of messages to send_messages #140

merged 2 commits into from
Oct 19, 2015

Conversation

farski
Copy link
Contributor

@farski farski commented Oct 10, 2015

I can't come up with a good reason why this would be the intended behavior, so I think it's a bug.

If a set of messages is passed to a queue like queue.send_messages(batch), where batch is:

[
    {
        message_body: "foo",
        delay_seconds: 1,
        message_attributes: {...}
    }, {
        message_body: "foo",
        delay_seconds: 1,
        message_attributes: {...}
    }
]

the messages are being corrupted before they actually get sent by send_message_batch on the client.

When sanitize_messages! tries to reformat the value from an Array to a Hash that is properly wrapped and ID'd, it doesn't take into account that the members of the array may be hashes. When hashes like in the example are used, the entire hash ends up getting serialized and shoved into message_body.

This change properly maintains the messages and adds an index, or behaves the old way if a string body is all that's getting passed in.

delay_seconds: 1,
message_attributes: { attr: 'attr1' }
}, {
message_body: "msg2",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@phstc
Copy link
Collaborator

phstc commented Oct 19, 2015

Thanks @farski. Before merging it, could you review houndci feedbacks?

@farski
Copy link
Contributor Author

farski commented Oct 19, 2015

@phstc Fixed a couple things. Line length in specs doesn't seem like a priority, but I can try to fix it if you'd prefer.

phstc added a commit that referenced this pull request Oct 19, 2015
Allow passing array of messages to send_messages
@phstc phstc merged commit cf1c52c into ruby-shoryuken:master Oct 19, 2015
@phstc
Copy link
Collaborator

phstc commented Oct 19, 2015

Added to master 🍻

@farski farski deleted the fix/send_messages_array branch October 19, 2015 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants