fix: apns-expiration header properly encoded (#123)

* fix: apns-expiration header properly encoded Also: added support for apns-collapse-id header (with corresponding new attribute on the Notification struct) * test: cleanup APNSTest a bit * chore: bump to v1.2.2 and update CHANGELOG * docs: update for collapse_id
codedge-llc · Jul 8, 2018 · ac3d32c · ac3d32c
1 parent 6b2d4bb
commit ac3d32c
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 74 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## v1.2.2
+* Fixed APNS handling of notification `expiration`
+* Added APNS support for `collapse_id`
+
 ## v1.2.1
 * FCM notifications can now handle `time_to_live`, `collapse_key`, `restricted_package_name`
   and `dry_run` keys

diff --git a/README.md b/README.md
@@ -13,7 +13,7 @@ Add pigeon and kadabra as `mix.exs` dependencies:
   ```elixir
   def deps do
     [
-      {:pigeon, "~> 1.2.0"},
+      {:pigeon, "~> 1.2.2"},
       {:kadabra, "~> 0.4.2"}
     ]
   end

diff --git a/docs/APNS Apple iOS.md b/docs/APNS Apple iOS.md
@@ -58,8 +58,9 @@ redeliver it.
 
   ```elixir
   %Pigeon.APNS.Notification{
+    collapse_id: String.t() | nil,
     device_token: String.t() | nil,
-    expiration: String.t() | nil,
+    expiration: non_neg_integer | nil,
     id: String.t() | nil,
     payload: %{String.t() => String.t()},
     response: atom,

diff --git a/lib/pigeon/apns/notification.ex b/lib/pigeon/apns/notification.ex
@@ -3,7 +3,8 @@ defmodule Pigeon.APNS.Notification do
   Defines APNS notification struct and constructor functions.
   """
 
-  defstruct device_token: nil,
+  defstruct collapse_id: nil,
+            device_token: nil,
             expiration: nil,
             id: nil,
             payload: %{"aps" => %{}},
@@ -18,6 +19,7 @@ defmodule Pigeon.APNS.Notification do
   ## Examples
 
       %Pigeon.APNS.Notification{
+          collapse_id: nil,
           device_token: "device token",
           expiration: nil,
           id: nil, # Set on push response if nil
@@ -27,8 +29,9 @@ defmodule Pigeon.APNS.Notification do
       }
   """
   @type t :: %__MODULE__{
+          collapse_id: String.t() | nil,
           device_token: String.t() | nil,
-          expiration: String.t() | nil,
+          expiration: non_neg_integer | nil,
           id: String.t() | nil,
           payload: %{String.t() => String.t()},
           response: response,
@@ -82,6 +85,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.new("push message", "device token", "topic", "id_1234")
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: "device token",
         expiration: nil,
         id: "id_1234",
@@ -108,6 +112,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.put_alert(%Pigeon.APNS.Notification{}, "push message")
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: nil,
         expiration: nil,
         id: nil,
@@ -128,6 +133,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.put_badge(%Pigeon.APNS.Notification{}, 5)
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: nil,
         expiration: nil,
         id: nil,
@@ -149,6 +155,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.put_sound(%Pigeon.APNS.Notification{}, "custom.aiff")
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: nil,
         expiration: nil,
         id: nil,
@@ -170,6 +177,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.put_content_available(%Pigeon.APNS.Notification{})
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: nil,
         expiration: nil,
         id: nil,
@@ -188,6 +196,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.put_category(%Pigeon.APNS.Notification{}, "category")
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: nil,
         expiration: nil,
         id: nil,
@@ -208,6 +217,7 @@ defmodule Pigeon.APNS.Notification do
 
       iex> Pigeon.APNS.Notification.put_mutable_content(%Pigeon.APNS.Notification{})
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: nil,
         expiration: nil,
         id: nil,
@@ -237,6 +247,7 @@ defmodule Pigeon.APNS.Notification do
       iex> n = Pigeon.APNS.Notification.new("test message", "device token")
       iex> Pigeon.APNS.Notification.put_custom(n, %{"custom-key" => 1234})
       %Pigeon.APNS.Notification{
+        collapse_id: nil,
         device_token: "device token",
         expiration: nil,
         id: nil,

diff --git a/lib/pigeon/apns/shared.ex b/lib/pigeon/apns/shared.ex
@@ -9,6 +9,11 @@ defmodule Pigeon.APNS.Shared do
 
   @type headers :: [{binary(), any()}]
 
+  @apns_id "apns-id"
+  @apns_topic "apns-topic"
+  @apns_expiration "apns-expiration"
+  @apns_collapse_id "apns-collapse-id"
+
   @spec worker_name(any) :: atom | nil
   def worker_name(%{name: name}), do: name
 
@@ -24,8 +29,10 @@ defmodule Pigeon.APNS.Shared do
       {":path", "/3/device/#{notification.device_token}"},
       {"content-length", "#{byte_size(json)}"}
     ]
-    |> put_apns_id(notification)
-    |> put_apns_topic(notification)
+    |> put_header(@apns_id, notification.id)
+    |> put_header(@apns_topic, notification.topic)
+    |> put_header(@apns_expiration, notification.expiration)
+    |> put_header(@apns_collapse_id, notification.collapse_id)
   end
 
   @spec push_payload(config, Notification.t(), Keyword.t()) ::
@@ -39,7 +46,11 @@ defmodule Pigeon.APNS.Shared do
 
     case status do
       200 ->
-        n = %{notification | id: get_apns_id(headers), response: :success}
+        n =
+          notification
+          |> Map.put(:id, get_header(headers, @apns_id))
+          |> Map.put(:response, :success)
+
         process_on_response(on_response, n)
 
       _error ->
@@ -58,23 +69,19 @@ defmodule Pigeon.APNS.Shared do
   def close(_config) do
   end
 
-  def put_apns_id(headers, notification) do
-    case notification.id do
-      nil -> headers
-      id -> headers ++ [{"apns-id", id}]
-    end
+  def put_header(headers, _key, nil), do: headers
+
+  def put_header(headers, key, val) when is_binary(val) do
+    headers ++ [{key, val}]
   end
 
-  def put_apns_topic(headers, notification) do
-    case notification.topic do
-      nil -> headers
-      topic -> headers ++ [{"apns-topic", topic}]
-    end
+  def put_header(headers, key, val) do
+    put_header(headers, key, to_string(val))
   end
 
-  def get_apns_id(headers) do
-    case Enum.find(headers, fn {key, _val} -> key == "apns-id" end) do
-      {"apns-id", id} -> id
+  def get_header(headers, key) do
+    case Enum.find(headers, fn {k, _val} -> k == key end) do
+      {^key, val} -> val
       nil -> nil
     end
   end

diff --git a/mix.exs b/mix.exs
@@ -1,7 +1,7 @@
 defmodule Pigeon.Mixfile do
   use Mix.Project
 
-  @version "1.2.1"
+  @version "1.2.2"
 
   def project do
     [

diff --git a/test/apns_test.exs b/test/apns_test.exs
@@ -3,17 +3,30 @@ defmodule Pigeon.APNSTest do
   doctest Pigeon.APNS
   doctest Pigeon.APNS.Notification
 
-  def test_message(msg),
-    do: "#{DateTime.to_string(DateTime.utc_now())} - #{msg}"
+  def test_message(msg) do
+    "#{DateTime.to_string(DateTime.utc_now())} - #{msg}"
+  end
 
   def test_topic, do: Application.get_env(:pigeon, :test)[:apns_topic]
   def test_token, do: Application.get_env(:pigeon, :test)[:valid_apns_token]
 
-  def bad_token,
-    do: "00fc13adff785122b4ad28809a3420982341241421348097878e577c991de8f0"
+  def bad_token do
+    "00fc13adff785122b4ad28809a3420982341241421348097878e577c991de8f0"
+  end
 
   def bad_id, do: "123e4567-e89b-12d3-a456-42665544000"
 
+  def test_notification(msg) do
+    msg
+    |> test_message()
+    |> Pigeon.APNS.Notification.new(
+      test_token(),
+      test_topic()
+    )
+    |> Map.put(:expiration, 0)
+    |> Map.put(:collapse_id, "test")
+  end
+
   describe "start_connection/1" do
     test "starts connection with opts keyword list" do
       opts = [
@@ -57,13 +70,7 @@ defmodule Pigeon.APNSTest do
 
   describe "push/1" do
     test "returns notification with :success on successful push" do
-      n =
-        Pigeon.APNS.Notification.new(
-          test_message("push/1"),
-          test_token(),
-          test_topic()
-        )
-
+      n = test_notification("push/1")
       assert Pigeon.APNS.push(n).response == :success
     end
 
@@ -79,12 +86,7 @@ defmodule Pigeon.APNSTest do
     end
 
     test "returns list for multiple notifications" do
-      n =
-        Pigeon.APNS.Notification.new(
-          test_message("push/1"),
-          test_token(),
-          test_topic()
-        )
+      n = test_notification("push/1")
 
       bad_n =
         Pigeon.APNS.Notification.new(
@@ -105,14 +107,8 @@ defmodule Pigeon.APNSTest do
 
   describe "push/1 with custom worker" do
     test "pushes to worker pid" do
-      n =
-        Pigeon.APNS.Notification.new(
-          test_message("push/1, custom worker"),
-          test_token(),
-          test_topic()
-        )
+      n = test_notification("push/1, custom_worker")
 
-      # Pigeon.APNS.stop_connection(:apns_default)
       opts = [
         cert: Application.get_env(:pigeon, :test)[:apns_cert],
         key: Application.get_env(:pigeon, :test)[:apns_key],
@@ -126,19 +122,11 @@ defmodule Pigeon.APNSTest do
       {:ok, worker_pid} = Pigeon.APNS.start_connection(opts)
 
       assert Pigeon.APNS.push(n, to: worker_pid).response == :success
-
-      # Pigeon.APNS.start_connection(:apns_default)
     end
 
     test "pushes to worker's atom name" do
-      n =
-        Pigeon.APNS.Notification.new(
-          test_message("push/1, custom worker"),
-          test_token(),
-          test_topic()
-        )
+      n = test_notification("push/1, custom_worker")
 
-      # Pigeon.APNS.stop_connection(:default)
       opts = [
         cert: Application.get_env(:pigeon, :test)[:apns_cert],
         key: Application.get_env(:pigeon, :test)[:apns_key],
@@ -153,20 +141,14 @@ defmodule Pigeon.APNSTest do
       {:ok, _worker_pid} = Pigeon.APNS.start_connection(opts)
 
       assert Pigeon.APNS.push(n, to: :custom).response == :success
-
-      # Pigeon.APNS.start_connection(:apns_default)
     end
   end
 
   describe "push/2 with :on_response" do
     test "returns :success response on successful push" do
       pid = self()
       on_response = fn x -> send(pid, x) end
-
-      n =
-        "push/2 :ok"
-        |> test_message()
-        |> Pigeon.APNS.Notification.new(test_token(), test_topic())
+      n = test_notification("push/2 :ok")
 
       assert Pigeon.APNS.push(n, on_response: on_response) == :ok
 
@@ -226,11 +208,7 @@ defmodule Pigeon.APNSTest do
     test "sends to pid if specified" do
       pid = self()
       on_response = fn x -> send(pid, x) end
-
-      n =
-        "push/2 :ok, custom worker"
-        |> test_message()
-        |> Pigeon.APNS.Notification.new(test_token(), test_topic())
+      n = test_notification("push/2 :ok, custom worker")
 
       Pigeon.APNS.stop_connection(:default)
 
@@ -257,13 +235,8 @@ defmodule Pigeon.APNSTest do
     test "sends to worker's atom name if specified" do
       pid = self()
       on_response = fn x -> send(pid, x) end
+      n = test_notification("push/2 :ok, custom worker")
 
-      n =
-        "push/2 :ok, custom worker"
-        |> test_message()
-        |> Pigeon.APNS.Notification.new(test_token(), test_topic())
-
-      # Pigeon.APNS.stop_connection(:default)
       opts = [
         cert: Application.get_env(:pigeon, :test)[:apns_cert],
         key: Application.get_env(:pigeon, :test)[:apns_key],
@@ -280,8 +253,6 @@ defmodule Pigeon.APNSTest do
       assert Pigeon.APNS.push(n, on_response: on_response, to: :custom) == :ok
 
       assert_receive(%Pigeon.APNS.Notification{response: :success}, 5_000)
-
-      # Pigeon.APNS.start_connection(:apns_default)
     end
   end
 end