Merge d8c762a into 527fca2

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- As according to the specification, an unsubscribe package can no
+  longer be encoded if it has no topic filters defined.
+
 ## 0.8.1 - 2018-08-25
 
 ### Added

lib/tortoise/package/subscribe.ex

@@ -45,8 +45,14 @@ defmodule Tortoise.Package.Subscribe do
 
   # PROTOCOLS ==========================================================
   defimpl Tortoise.Encodable do
-    def encode(%Package.Subscribe{identifier: identifier} = t)
-        when identifier in 0x0001..0xFFFF do
+    def encode(
+          %Package.Subscribe{
+            identifier: identifier,
+            # a valid subscribe package has at least one topic/qos pair
+            topics: [{<<_topic_filter::binary>>, qos} | _]
+          } = t
+        )
+        when identifier in 0x0001..0xFFFF and qos in 0..2 do
       [
         Package.Meta.encode(t.__META__),
         Package.variable_length_encode([
@@ -87,13 +93,9 @@ defmodule Tortoise.Package.Subscribe do
       {Enum.into(topics, %{}),
        fn
          acc, {:cont, {<<topic::binary>>, qos}} when qos in 0..2 ->
-           Map.update(acc, topic, qos, fn
-             current_qos when qos > current_qos ->
-               qos
-
-             current_qos ->
-               current_qos
-           end)
+           # if a topic filter repeat in the input we will pick the
+           # biggest one
+           Map.update(acc, topic, qos, &max(&1, qos))
 
          acc, {:cont, <<topic::binary>>} ->
            Map.put_new(acc, topic, 0)

lib/tortoise/package/unsubscribe.ex

@@ -43,7 +43,13 @@ defmodule Tortoise.Package.Unsubscribe do
 
   # Protocols ----------------------------------------------------------
   defimpl Tortoise.Encodable do
-    def encode(%Package.Unsubscribe{identifier: identifier} = t)
+    def encode(
+          %Package.Unsubscribe{
+            identifier: identifier,
+            # a valid unsubscribe package has at least one topic filter
+            topics: [_topic_filter | _]
+          } = t
+        )
         when identifier in 0x0001..0xFFFF do
       [
         Package.Meta.encode(t.__META__),

test/test_helper.exs

@@ -208,7 +208,7 @@ defmodule Tortoise.TestGenerators do
   def gen_unsubscribe() do
     let unsubscribe <- %Package.Unsubscribe{
           identifier: gen_identifier(),
-          topics: list(gen_topic_filter())
+          topics: non_empty(list(gen_topic_filter()))
         } do
       unsubscribe
     end

test/tortoise/package/subscribe_test.exs

@@ -6,6 +6,7 @@ defmodule Tortoise.Package.SubscribeTest do
   import Tortoise.TestGenerators, only: [gen_subscribe: 0]
 
   alias Tortoise.Package
+  alias Tortoise.Package.Subscribe
 
   property "encoding and decoding subscribe messages" do
     forall subscribe <- gen_subscribe() do
@@ -17,4 +18,20 @@ defmodule Tortoise.Package.SubscribeTest do
       )
     end
   end
+
+  describe "Collectable" do
+    test "Pick the largest QoS when topic filters repeat in input" do
+      topic_filters = [{"a", 2}, {"a", 1}, {"a", 0}]
+      assert %Subscribe{topics: [{"a", 2}]} = Enum.into(topic_filters, %Subscribe{})
+
+      topic_filters = [{"a", 0}, {"a", 1}, {"a", 2}]
+      assert %Subscribe{topics: [{"a", 2}]} = Enum.into(topic_filters, %Subscribe{})
+
+      topic_filters = [{"a", 1}, {"a", 0}]
+      assert %Subscribe{topics: [{"a", 1}]} = Enum.into(topic_filters, %Subscribe{})
+
+      topic_filters = [{"a", 0}, {"a", 0}]
+      assert %Subscribe{topics: [{"a", 0}]} = Enum.into(topic_filters, %Subscribe{})
+    end
+  end
 end