Skip to content
This repository has been archived by the owner on Jul 28, 2022. It is now read-only.

fix(message-order): consume message by creation order (fifo) #362

Merged

Conversation

mazsudo
Copy link
Contributor

@mazsudo mazsudo commented Jan 23, 2019

Subject

I found out that with backend: sonata.notification.backend.doctrine, messages are consumed in the wrong order for me that should be first in first consumed. It seems that combination of :

For me it should be a First In First Out, but i may be wrong :)

I am targeting this branch, because it's changing the order of message consumption and may have some unwanted side effects on existing projects.

Changelog

### Fixed
Fix order of message handling with doctrine backend (older first)

@mazsudo mazsudo force-pushed the handle-message-by-creation-order branch 2 times, most recently from cbadd67 to c94db70 Compare January 23, 2019 16:23
@OskarStark
Copy link
Member

Thank you for the detailed description of this behavior and I share your thoughts! 👍

@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 23, 2019

Thanks for your feedback @OskarStark. FI, i'm going to rework this PR to do an orderBy ASC and an array_shift rather than orderBy DESC and array_pop that sounds more like a double negation.
Currently trying to write an explicit unit test to validate the message processing order. Updates will come soon ;)

Copy link
Member

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Thank you, I will prevent merging by "Requesting Changes", please ping me when you want an review again 👍 🚀

@mazsudo mazsudo force-pushed the handle-message-by-creation-order branch 2 times, most recently from af45b09 to 86edef9 Compare January 24, 2019 09:38
@mazsudo
Copy link
Contributor Author

mazsudo commented Jan 24, 2019

should be good now @OskarStark ;) orderBy ASC + array_shift is clearer for everyone i hope

tests/Entity/MessageManagerTest.php Outdated Show resolved Hide resolved
tests/Iterator/MessageManagerMessageIteratorTest.php Outdated Show resolved Hide resolved
tests/Iterator/MessageManagerMessageIteratorTest.php Outdated Show resolved Hide resolved
@mazsudo mazsudo force-pushed the handle-message-by-creation-order branch from 86edef9 to db3a1df Compare January 24, 2019 14:05
@core23 core23 merged commit 24edcdb into sonata-project:master Feb 9, 2019
@mazsudo mazsudo deleted the handle-message-by-creation-order branch January 28, 2020 07:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants