Skip to content

[WIP] Add new RedisRepository #40

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 59 commits into from
Closed

Conversation

Namoshek
Copy link
Collaborator

@Namoshek Namoshek commented Sep 1, 2020

This PR re-introduces the RedisRepository deleted by #33, but with the new Repository interface and slightly different logic.

The new implementation utilizes hash maps for pending messages instead of sets. Most operations only involve one call to the Redis database, since we can use commands which terminate gracefully on failure (which we can then forward as exception or negative result to the caller).

Todos:

  • Test implementation

Namoshek and others added 30 commits July 26, 2020 20:31
This change allows us to guarantee that message ids are never
used twice at the same time. It also allows for different
implementations, e.g. with persistence.
The connection settings do now provide a fluent setter interface
instead of a large constructor. Besides this change, the configurable
caFile has been moved from the MQTTClient class to the connection
settings. A bunch of new TLS settings have been added as well.
All logging statements have been reworked to utilize the log context interpolation provided by the PSR-3 log standard. Some log statements have been reduced in severity (from info to debug). Most log statements now contain the broker (referenced by host and port).

Co-Authored-By: Giovanni Giacobbi <thg2k@users.noreply.github.com>
New log helper methods are used to add common information and context to log messages. All log messages are now prepended with broker information (host and port) as well as the client id. Also the word "MQTT" is now part of the common log output, making it obsolete for individual messages.

Co-Authored-By: Giovanni Giacobbi <thg2k@users.noreply.github.com>
Whenever registered callbacks are called, we catch all exceptions thrown by user provided code to avoid crashes of the client. We still log such exceptions as errors though.

Co-Authored-By: Giovanni Giacobbi <thg2k@users.noreply.github.com>
We also create a socket context without any settings if none are
configured. This prevents accidental socket context pollution by
other parts of an application.
…on (#28)

* Implement basic MQTT v3 message processor
The current implementation does parse messages, but it does
not handle them just yet.

* Refactor more functionality into message processor

* Log sending/receiving data

* Implement new loop logic

* Split message parsing and processing

* Throw more correct exceptions

* Fix required bytes calculation

* Improve data readin / connection logic

* Fix wrong change in readFromSocket()

* Add phpunit/phpunit as dev dependency

* Only pass clientId to message processor

* Add initial test for message processor and fix issue

* Fix typo

* Wait for first byte in connection logic
Doing so avoids polling over and over again. Since we know,
there must be at least one byte in the response.

* Add an abstract BaseMessageProcessor

* Fix protocol version in message processor docs
Since there are different protocol versions like 3, 3.1, 3.1.1 of which some use the same protocol level (like 3 and 3.1, which both use `3` as protocol level), it makes more sense to represent the protocol version as string. This also makes it less error prone if more protocol version levels are added.
Having immutable connection settings prevents the settings from being altered after passing them to the connect() method. This is important since some settings are only used during the life time of a session. And if auto-reconnect is being implemented at some point, using the same settings for the reconnect will be essential as well.
The $qualityOfService and $retain settings are actually intended for the last will. We now transport this information through the name.
The latest documentation improvements of the master branch have been added, but updated for the refactoring branch details.
Since we decide based on the protocol logic whether to block for data on a per-call-basis or not, it is not necessary to set a global blocking mode on the socket itself.
Namoshek and others added 23 commits August 1, 2020 15:28
A blocking stream causes connection abortions, and we don't want that.
This fix has been taken from the master branch, see #30.
and buildDisconnectMessage()
In case multiple subscriptions of different topics matching the topic
of a published message with different QoS exist, we also deliver higher
QoS messages to subscribers with a lower QoS. Actually, the QoS doesn't
matter at all in this case, since the broker handles it accordingly.
…iptions management (#33)

* Refactored Repository interface and pending message handling.

Details:

* Refactored the Repository interface.
  The new interface stores a generic serializable PendingMessage without
  knowing which type it is, using two different queues one for incoming and
  one for outgoing messages. There are currently three types of outgoing
  messages (PublishedMessage, SubscribeRequest, UnsubscribeRequest) and one
  incoming message (PublishedMessaged) handled.
* The `TopicSubscription` class has been renamed to `Subscription` and now
  represents only the subscription itself, separating it from the concept of
  SubscribeRequest which might include several topic filters in one message.
* Removed exception `UnexpectedAcknowledgementException`.
  It is expected to have duplicate packets over the wire, so there is no need
  for an expection in this case, a notice to the logger is enough.
* Added `ProtocolViolationException`.
  This explicitly marks situations where the broker is misbehaving and the
  connection should be terminated.
* Fixed some problems with the MessageProcessor.

* Removed Redis repository implementation until updated to the new Repository interface

* Fix MessageProcessor, exceptions and documentation

* Change visibility and description of DTOs

* Simplify acknowledgement parsing logic

* Change code formatting

* Change field visibility and formatting

Co-authored-by: Marvin Mall <marvin-mall@msn.com>
The repository is inspired by the previous implementation but uses hash maps over sets for pending messages (due to message identifiers as keys).
@Namoshek Namoshek added the enhancement New feature or request label Sep 1, 2020
@Namoshek
Copy link
Collaborator Author

Namoshek commented Sep 1, 2020

@thg2k If you could have a look as well, it would be great!

Base automatically changed from refactoring to master January 2, 2021 17:48
@Namoshek Namoshek mentioned this pull request Jan 2, 2021
1 task
@Namoshek Namoshek closed this Jan 2, 2021
@Namoshek Namoshek deleted the feature-redis-repository branch January 2, 2021 18:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants