Skip to content

Refactoring and overhaul of the package in preparation of v1 #51

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

Merged
merged 62 commits into from
Jan 2, 2021

Conversation

Namoshek
Copy link
Collaborator

@Namoshek Namoshek commented Jan 2, 2021

This PR adds a long list of anticipated changes with the goal of releasing a v1.0.0 of the MQTT client soon. A lot has changed from the current version v0.3.0. Noteworthy are the following things:

  • The MQTTClient class has been renamed to MqttClient to follow PSR-2.
  • The MqttClient class has been split into the MQTT client itself as well as a so called MessageProcessor, which is what implements the actual protocol and protocol specific parsing and formatting (in preparation of MQTT 5).
  • Some parameters of MQTTClient::__construct() and MQTTClient::connect() have been removed and relocated to the ConnectionSettings. The ConnectionSettings are now immutable and can be configured using fluent setters (see README).
  • The support for TLS options provided by the PHP SSL context has been improved.
  • A few unnoticed issues and bugs have been fixed along the way.

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 26 commits August 1, 2020 19:21
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>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 2, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Namoshek Namoshek merged commit 27bda19 into master Jan 2, 2021
@Namoshek Namoshek deleted the refactoring branch January 2, 2021 17:48
@Namoshek Namoshek added this to the v1.0.0 milestone Jan 3, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants