Skip to content

Commit

Permalink
Merge pull request #808 from eclipse/stop-exposing-private-function
Browse files Browse the repository at this point in the history
Stop exposing private function
  • Loading branch information
PierreF authored Feb 6, 2024
2 parents 47ab9b9 + 49eb1c7 commit 1da5ba5
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 115 deletions.
1 change: 1 addition & 0 deletions ChangeLog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ This release include breaking change. See migrations.md for more details on how
* `max_packets` argument in loop(), loop_write() and loop_forever() is removed
* `force` argument in loop_stop() is removed
* method `message_retry_set()` is removed
- **BREAKING** Remove the base62, WebsocketWrapper and ConnectionState, as user shouldn't directly use them.
- Possible breaking change: Add properties to access most Client attribute. Closes #764.
Since this add new properties like `logger`, if a sub-class defined `logger`, the two `logger`
will conflict.
Expand Down
21 changes: 19 additions & 2 deletions migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ The list of breaking change (detailed below) are:
tl; dr: add `mqtt.CallbackAPIVersion.VERSION1` as first argument to `Client()`
* Drop support for older Python.
* Dropped some deprecated and no used argument or method. If you used them, you can just drop them.
* Removed from public interface few function/class
* Renamed ReasonCodes to ReasonCode
* Improved typing which resulted in few type change. It might no affect you, see below for detail.
* Fixed connect_srv, which changed its signature.
* Added new properties, which could conflict with sub-class
Expand Down Expand Up @@ -63,7 +65,7 @@ def on_connect(client, userdata, flags, reason_code, properties):
# error processing
```

Be careful that for MQTTv3, `rc` (an integer) changed to `reason_code` (an instance of ReasonCodes), and the numeric value changed.
Be careful that for MQTTv3, `rc` (an integer) changed to `reason_code` (an instance of ReasonCode), and the numeric value changed.
The numeric value 0 means success for both, so as in above example, using `reason_code == 0`, `reason_code != 0` or other comparison with zero
is fine.
But if you had comparison with other value, you will need to update the code. It's recommended to compare to string value:
Expand Down Expand Up @@ -148,7 +150,7 @@ def on_unsubscribe(client, userdata, mid):
# OLD code for MQTTv5
def on_unsubscribe(client, userdata, mid, properties, reason_codes):
# In OLD version, reason_codes could be a list or a single ReasonCodes object
# In OLD version, reason_codes could be a list or a single ReasonCode object
if isinstance(reason_codes, list):
for unsub_result in reason_codes:
# Any reason code >= 128 is a failure.
Expand Down Expand Up @@ -207,6 +209,21 @@ The following is dropped:

They were not used in previous version, so you can just remove them if you used them.

### Stop exposing private function/class

Some private function or class are not longer exposed. The following are removed:
* function base62
* class WebsocketWrapper
* enum ConnectionState

### Renamed ReasonCodes to ReasonCode

The class ReasonCodes that was used to represent one reason code response from
broker or generated by the library is now named ReasonCode.

This should work without any change as ReasonCodes (plural, the old name) is still
present but deprecated.

### Improved typing

Version 2.0 improved typing, but this would be compatible with existing code.
Expand Down
197 changes: 107 additions & 90 deletions src/paho/mqtt/client.py

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/paho/mqtt/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class ConnackCode(enum.IntEnum):
CONNACK_REFUSED_NOT_AUTHORIZED = 5


class ConnectionState(enum.Enum):
class _ConnectionState(enum.Enum):
MQTT_CS_NEW = enum.auto()
MQTT_CS_CONNECT_ASYNC = enum.auto()
MQTT_CS_CONNECTING = enum.auto()
Expand Down
4 changes: 2 additions & 2 deletions src/paho/mqtt/publish.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

from paho.mqtt.enums import CallbackAPIVersion
from paho.mqtt.properties import Properties
from paho.mqtt.reasoncodes import ReasonCodes
from paho.mqtt.reasoncodes import ReasonCode

from .. import mqtt
from . import client as paho
Expand Down Expand Up @@ -92,7 +92,7 @@ def _on_connect(client: paho.Client, userdata: MessagesList, flags, reason_code,


def _on_publish(
client: paho.Client, userdata: collections.deque[MessagesList], mid: int, reason_codes: ReasonCodes, properties: Properties,
client: paho.Client, userdata: collections.deque[MessagesList], mid: int, reason_codes: ReasonCode, properties: Properties,
) -> None:
"""Internal callback"""
#pylint: disable=unused-argument
Expand Down
24 changes: 20 additions & 4 deletions src/paho/mqtt/reasoncodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@
"""

import functools
import warnings
from typing import Any

from .packettypes import PacketTypes


@functools.total_ordering
class ReasonCodes:
class ReasonCode:
"""MQTT version 5.0 reason codes class.
See ReasonCodes.names for a list of possible numeric values along with their
See ReasonCode.names for a list of possible numeric values along with their
names and the packets to which they apply.
"""
Expand Down Expand Up @@ -176,14 +178,14 @@ def __eq__(self, other):
return self.value == other
if isinstance(other, str):
return other == str(self)
if isinstance(other, ReasonCodes):
if isinstance(other, ReasonCode):
return self.value == other.value
return False

def __lt__(self, other):
if isinstance(other, int):
return self.value < other
if isinstance(other, ReasonCodes):
if isinstance(other, ReasonCode):
return self.value < other.value
return NotImplemented

Expand All @@ -195,3 +197,17 @@ def json(self):

def pack(self):
return bytearray([self.value])


class _CompatibilityIsInstance(type):
def __instancecheck__(self, other: Any) -> bool:
return isinstance(other, ReasonCode)


class ReasonCodes(ReasonCode, metaclass=_CompatibilityIsInstance):
def __init__(self, *args, **kwargs):
warnings.warn("ReasonCodes is deprecated, use ReasonCode (singular) instead",
category=DeprecationWarning,
stacklevel=2,
)
super().__init__(*args, **kwargs)
14 changes: 7 additions & 7 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from paho.mqtt.enums import CallbackAPIVersion, MQTTErrorCode, MQTTProtocolVersion
from paho.mqtt.packettypes import PacketTypes
from paho.mqtt.properties import Properties
from paho.mqtt.reasoncodes import ReasonCodes
from paho.mqtt.reasoncodes import ReasonCode

import tests.paho_test as paho_test

Expand Down Expand Up @@ -78,7 +78,7 @@ def on_connect(mqttc, obj, flags, rc_or_reason_code, properties_or_none=None):
if callback_version == CallbackAPIVersion.VERSION1:
assert rc_or_reason_code == 1
else:
assert rc_or_reason_code == ReasonCodes(PacketTypes.CONNACK, "Unsupported protocol version")
assert rc_or_reason_code == ReasonCode(PacketTypes.CONNACK, "Unsupported protocol version")

mqttc.on_connect = on_connect

Expand Down Expand Up @@ -221,7 +221,7 @@ def test_01_broker_no_support(self, fake_broker):

def on_connect(mqttc, obj, flags, reason, properties):
assert reason == 132
assert reason == ReasonCodes(client.CONNACK >> 4, aName="Unsupported protocol version")
assert reason == ReasonCode(client.CONNACK >> 4, aName="Unsupported protocol version")
mqttc.disconnect()

mqttc.on_connect = on_connect
Expand Down Expand Up @@ -828,7 +828,7 @@ def test_callback_v2_mqtt3(self, fake_broker):
def on_connect(cl, userdata, flags, reason, properties):
assert isinstance(cl, client.Client)
assert isinstance(flags, client.ConnectFlags)
assert isinstance(reason, ReasonCodes)
assert isinstance(reason, ReasonCode)
assert isinstance(properties, Properties)
assert reason == 0
assert properties.isEmpty()
Expand All @@ -839,7 +839,7 @@ def on_subscribe(cl, userdata, mid, reason_code_list, properties):
assert isinstance(cl, client.Client)
assert isinstance(mid, int)
assert isinstance(reason_code_list, list)
assert isinstance(reason_code_list[0], ReasonCodes)
assert isinstance(reason_code_list[0], ReasonCode)
assert isinstance(properties, Properties)
assert properties.isEmpty()
userdata.append("on_subscribe")
Expand All @@ -848,7 +848,7 @@ def on_subscribe(cl, userdata, mid, reason_code_list, properties):
def on_publish(cl, userdata, mid, reason_code, properties):
assert isinstance(cl, client.Client)
assert isinstance(mid, int)
assert isinstance(reason_code, ReasonCodes)
assert isinstance(reason_code, ReasonCode)
assert isinstance(properties, Properties)
assert properties.isEmpty()
userdata.append("on_publish")
Expand All @@ -872,7 +872,7 @@ def on_unsubscribe(cl, userdata, mid, reason_code_list, properties):
def on_disconnect(cl, userdata, flags, reason_code, properties):
assert isinstance(cl, client.Client)
assert isinstance(flags, client.DisconnectFlags)
assert isinstance(reason_code, ReasonCodes)
assert isinstance(reason_code, ReasonCode)
assert isinstance(properties, Properties)
assert properties.isEmpty()
userdata.append("on_disconnect")
Expand Down
35 changes: 28 additions & 7 deletions tests/test_reasoncodes.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,47 @@
import pytest
from paho.mqtt.packettypes import PacketTypes
from paho.mqtt.reasoncodes import ReasonCodes
from paho.mqtt.reasoncodes import ReasonCode, ReasonCodes


class TestReasonCode:
def test_equality(self):
rc_success = ReasonCodes(PacketTypes.CONNACK, "Success")
rc_success = ReasonCode(PacketTypes.CONNACK, "Success")
assert rc_success == 0
assert rc_success == "Success"
assert rc_success != "Protocol error"
assert rc_success == ReasonCodes(PacketTypes.CONNACK, "Success")
assert rc_success == ReasonCode(PacketTypes.CONNACK, "Success")

rc_protocol_error = ReasonCodes(PacketTypes.CONNACK, "Protocol error")
rc_protocol_error = ReasonCode(PacketTypes.CONNACK, "Protocol error")
assert rc_protocol_error == 130
assert rc_protocol_error == "Protocol error"
assert rc_protocol_error != "Success"
assert rc_protocol_error == ReasonCodes(PacketTypes.CONNACK, "Protocol error")
assert rc_protocol_error == ReasonCode(PacketTypes.CONNACK, "Protocol error")

def test_comparison(self):
rc_success = ReasonCodes(PacketTypes.CONNACK, "Success")
rc_protocol_error = ReasonCodes(PacketTypes.CONNACK, "Protocol error")
rc_success = ReasonCode(PacketTypes.CONNACK, "Success")
rc_protocol_error = ReasonCode(PacketTypes.CONNACK, "Protocol error")

assert not rc_success > 0
assert rc_protocol_error > 0
assert not rc_success != 0
assert rc_protocol_error != 0

def test_compatibility(self):
rc_success = ReasonCode(PacketTypes.CONNACK, "Success")
with pytest.deprecated_call():
rc_success_old = ReasonCodes(PacketTypes.CONNACK, "Success")
assert rc_success == rc_success_old

assert isinstance(rc_success, ReasonCode)
assert isinstance(rc_success_old, ReasonCodes)
# User might use isinstance with the old name (plural)
# while the library give them a ReasonCode (singular) in the callbacks
assert isinstance(rc_success, ReasonCodes)
# The other way around is probably never used... but still support it
assert isinstance(rc_success_old, ReasonCode)

# Check that isinstance implementation don't always return True
assert not isinstance(rc_success, dict)
assert not isinstance(rc_success_old, dict)
assert not isinstance({}, ReasonCode)
assert not isinstance({}, ReasonCodes)
4 changes: 2 additions & 2 deletions tests/test_websockets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from unittest.mock import Mock

import pytest
from paho.mqtt.client import WebsocketConnectionError, WebsocketWrapper
from paho.mqtt.client import WebsocketConnectionError, _WebsocketWrapper


class TestHeaders:
Expand Down Expand Up @@ -121,7 +121,7 @@ def fakerecv(*args):
wargs_with_socket["socket"] = mocksock

with pytest.raises(WebsocketConnectionError) as exc:
WebsocketWrapper(**wargs_with_socket)
_WebsocketWrapper(**wargs_with_socket)

# We're not creating the response hash properly so it should raise this
# error
Expand Down

0 comments on commit 1da5ba5

Please sign in to comment.