From 9dbfe954b5b522c126a581e84d571c33191afa4f Mon Sep 17 00:00:00 2001 From: speeddragon Date: Thu, 3 Jul 2025 22:18:14 +0100 Subject: [PATCH 1/5] Fix missing :p256dh atom --- lib/web_push/subscription.ex | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/web_push/subscription.ex b/lib/web_push/subscription.ex index 844c0d0..920a207 100644 --- a/lib/web_push/subscription.ex +++ b/lib/web_push/subscription.ex @@ -31,10 +31,12 @@ defmodule WebPushEx.Subscription do key = String.to_existing_atom(key) value = - if key == :endpoint and is_binary(value) do - URI.parse(value) - else - value + cond do + key == :endpoint and is_binary(value) -> + URI.parse(value) + + key in [:p256dh, :auth, :keys] -> + value end [{key, value} | acc] From ec5127433f219c833af4f6f4fc479d9f30a161e6 Mon Sep 17 00:00:00 2001 From: speeddragon Date: Thu, 3 Jul 2025 22:25:36 +0100 Subject: [PATCH 2/5] Fix test --- lib/web_push/subscription.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/web_push/subscription.ex b/lib/web_push/subscription.ex index 920a207..722fbb9 100644 --- a/lib/web_push/subscription.ex +++ b/lib/web_push/subscription.ex @@ -37,6 +37,9 @@ defmodule WebPushEx.Subscription do key in [:p256dh, :auth, :keys] -> value + + true -> + value end [{key, value} | acc] From 80f2c6013eb132987ee2f51bf712702ef9af4d90 Mon Sep 17 00:00:00 2001 From: Kenichi Nakamura Date: Tue, 8 Jul 2025 05:12:19 +0000 Subject: [PATCH 3/5] add atom test script, run from ci --- .github/workflows/ci.yml | 5 +++++ test/support/atom.exs | 9 +++++++++ 2 files changed, 14 insertions(+) create mode 100644 test/support/atom.exs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5cfde91..7611db4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,11 @@ jobs: - name: test run: mix test + - name: atom + env: + MIX_ENV: test + run: mix run test/support/atom.exs + - name: credo env: MIX_ENV: test diff --git a/test/support/atom.exs b/test/support/atom.exs new file mode 100644 index 0000000..9fff2f4 --- /dev/null +++ b/test/support/atom.exs @@ -0,0 +1,9 @@ +WebPushEx.Subscription.from_json(""" +{ + "endpoint": "https://push.example.com/123", + "keys": { + "p256dh": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", + "auth": "BTBZMqHH6r4Tts7J_aSIgg" + } +} +""") From b8d0b66809d534b806206a0e13e30c58ef0fda05 Mon Sep 17 00:00:00 2001 From: Kenichi Nakamura Date: Tue, 8 Jul 2025 05:17:29 +0000 Subject: [PATCH 4/5] refactor WebPushEx.Subscription.from_json/1 * remove usage of String.to_existing_atom/1 * update tests for KeyError vs. ArgumentError --- lib/web_push/subscription.ex | 34 ++++++++++++---------------------- test/web_push_ex_test.exs | 32 ++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/lib/web_push/subscription.ex b/lib/web_push/subscription.ex index 722fbb9..d595da8 100644 --- a/lib/web_push/subscription.ex +++ b/lib/web_push/subscription.ex @@ -21,27 +21,17 @@ defmodule WebPushEx.Subscription do """ @spec from_json(String.t()) :: t() def from_json(json) do - {decoded, :ok, ""} = :json.decode(json, :ok, %{object_push: &object_push/3}) - - struct!(__MODULE__, decoded) - end - - @spec object_push(String.t(), :json.decode_value(), :ok) :: list() - defp object_push(key, value, acc) do - key = String.to_existing_atom(key) - - value = - cond do - key == :endpoint and is_binary(value) -> - URI.parse(value) - - key in [:p256dh, :auth, :keys] -> - value - - true -> - value - end - - [{key, value} | acc] + with decoded <- :json.decode(json), + endpoint <- Map.fetch!(decoded, "endpoint"), + uri <- URI.parse(endpoint), + keys <- Map.fetch!(decoded, "keys") do + %__MODULE__{ + endpoint: uri, + keys: %{ + p256dh: Map.fetch!(keys, "p256dh"), + auth: Map.fetch!(keys, "auth") + } + } + end end end diff --git a/test/web_push_ex_test.exs b/test/web_push_ex_test.exs index 3c402da..eedba6a 100644 --- a/test/web_push_ex_test.exs +++ b/test/web_push_ex_test.exs @@ -47,18 +47,42 @@ defmodule WebPushExTest do assert observed.keys == expected.keys end - test "raises with invalid keys" do - assert_raise ArgumentError, fn -> + test "raises KeyError with invalid keys" do + assert_raise KeyError, fn -> WebPushEx.Subscription.from_json(""" { - "end": "https://push.example.com/123", - "key": { + "endpoint": "https://push.example.com/123", + "keys": { "p256": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", "au": "BTBZMqHH6r4Tts7J_aSIgg" } } """) end + + assert_raise KeyError, fn -> + WebPushEx.Subscription.from_json(""" + { + "endpoint": "https://push.example.com/123", + "key": { + "p256dh": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", + "auth": "BTBZMqHH6r4Tts7J_aSIgg" + } + } + """) + end + + assert_raise KeyError, fn -> + WebPushEx.Subscription.from_json(""" + { + "end": "https://push.example.com/123", + "keys": { + "p256dh": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", + "auth": "BTBZMqHH6r4Tts7J_aSIgg" + } + } + """) + end end end end From b3308394e544b8c86210a3a4f73d98951b883093 Mon Sep 17 00:00:00 2001 From: Kenichi Nakamura Date: Tue, 8 Jul 2025 15:06:08 +0000 Subject: [PATCH 5/5] add test for auth key and comments, simplify fixture --- test/support/web_push_ex_fixtures.ex | 8 +++----- test/web_push_ex_test.exs | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/test/support/web_push_ex_fixtures.ex b/test/support/web_push_ex_fixtures.ex index 834f82a..c99911c 100644 --- a/test/support/web_push_ex_fixtures.ex +++ b/test/support/web_push_ex_fixtures.ex @@ -11,17 +11,15 @@ defmodule WebPushExFixtures do WebPushSubscription fixture with default `p256dh` and `auth` values from the RFC example. """ - def web_push_subscription_fixture(attrs \\ %{}) do - attrs - |> Enum.into(%{ + def web_push_subscription_fixture do + %WebPushEx.Subscription{ endpoint: URI.parse("https://push.example.com/123"), keys: %{ p256dh: "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", auth: "BTBZMqHH6r4Tts7J_aSIgg" } - }) - |> then(&struct!(WebPushEx.Subscription, &1)) + } end @doc """ diff --git a/test/web_push_ex_test.exs b/test/web_push_ex_test.exs index eedba6a..9fd294a 100644 --- a/test/web_push_ex_test.exs +++ b/test/web_push_ex_test.exs @@ -48,18 +48,33 @@ defmodule WebPushExTest do end test "raises KeyError with invalid keys" do + # auth key assert_raise KeyError, fn -> WebPushEx.Subscription.from_json(""" { "endpoint": "https://push.example.com/123", "keys": { - "p256": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", + "p256dh": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", "au": "BTBZMqHH6r4Tts7J_aSIgg" } } """) end + # p256dh key + assert_raise KeyError, fn -> + WebPushEx.Subscription.from_json(""" + { + "endpoint": "https://push.example.com/123", + "keys": { + "p256": "BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw4", + "auth": "BTBZMqHH6r4Tts7J_aSIgg" + } + } + """) + end + + # keys key assert_raise KeyError, fn -> WebPushEx.Subscription.from_json(""" { @@ -72,6 +87,7 @@ defmodule WebPushExTest do """) end + # endpoint key assert_raise KeyError, fn -> WebPushEx.Subscription.from_json(""" {