Skip to content

Message publishing optimization #75

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.

Sign up for GitHub

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

Jump to bottom
Closed
viktorprogger wants to merge 8 commits into from
Closed

Message publishing optimization #75

viktorprogger wants to merge 8 commits into from

Conversation

@viktorprogger
Copy link
Contributor

@viktorprogger viktorprogger commented Apr 27, 2023

Q A
Is bugfix? ❌
New feature? ✔️
Breaks BC? ✔️
Fixed issues #58

@viktorprogger viktorprogger added the status:under development Someone is working on a pull request. label Apr 27, 2023
@viktorprogger viktorprogger requested a review from s1lver April 27, 2023 09:11
@viktorprogger viktorprogger self-assigned this Apr 27, 2023
@viktorprogger viktorprogger linked an issue Apr 27, 2023 that may be closed by this pull request
Optimize message publishing #58
Closed
@what-the-diff
Copy link

what-the-diff bot commented Apr 27, 2023

PR Summary

  • Added private property to Adapter class
    Introduces a new private property for message handling within the Adapter class.
  • Improved push method in the adapter
    Enhanced efficiency by reusing AMQPMessage instances, and ensures message IDs are set when provided by the MessageInterface object.
  • Middleware for generating unique message IDs
    Created middleware to generate unique IDs for messages without one, ensuring all messages pushed into the queue have an ID.

vjik
vjik reviewed Apr 28, 2023
View reviewed changes
@samdark
Copy link
Member

samdark commented May 3, 2023

Tests?

s1lver
s1lver suggested changes Jun 15, 2023
View reviewed changes
src/Adapter.php
$this->message = new AMQPMessage('', $this->queueProvider->getMessageProperties());
}

return $this->message;
Copy link
Member

@s1lver s1lver Jun 15, 2023

Choose a reason for hiding this comment

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

Scrutinizer review

The expression return $this->message could return the type null which is incompatible with the type-hinted return PhpAmqpLib\Message\AMQPMessage. Consider adding an additional type-check to rule them out.

@viktorprogger
Copy link
Contributor Author

viktorprogger commented Jul 13, 2025

Done in #121

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vjik vjik vjik left review comments

+1 more reviewer

@s1lver s1lver s1lver requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@viktorprogger viktorprogger

Labels

status:under development Someone is working on a pull request.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Optimize message publishing

5 participants

@viktorprogger @samdark @vjik @s1lver @xepozz