Skip to content

Commit

Permalink
trigger_payload: accept trigger_name key
Browse files Browse the repository at this point in the history
Astarte >= 1.2.0 sends an additional trigger_name key in the trigger payload.
This was making Edgehog crash since Ash doesn't accept additional input keys
by default. Fix this allowing unknown keys to be accepted. This should
future-proof the trigger handler in case further additional keys are added in
the future.

Since 1.2.0 is the current stable version of Astarte, add the trigger_name key
to all existing tests and explicitly test support for the key missing to avoid
regressions with old Astarte versions.

Fix edgehog-device-manager#664

Signed-off-by: Riccardo Binetti <[email protected]>
  • Loading branch information
rbino committed Oct 28, 2024
1 parent 6845d45 commit ffb3486
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.9.1] - Unreleased
### Fixed
- Allow receiving `trigger_name` key in trigger payload, which is sent by Astarte >= 1.2.0.

## [0.9.0] - 2024-10-25
### Fixed
- Correctly support automatic login attempts on the frontend regardless of existing auth sessions ([#596](https://github.com/edgehog-device-manager/edgehog/pull/596)).
Expand Down
10 changes: 10 additions & 0 deletions backend/lib/edgehog/triggers/trigger_payload.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ defmodule Edgehog.Triggers.TriggerPayload do

alias Edgehog.Triggers.Event

actions do
defaults [:read, :update, :destroy]

create :create do
primary? true
accept [:device_id, :timestamp, :event]
skip_unknown_inputs :*
end
end

attributes do
# TODO: add Device ID validation
attribute :device_id, :string do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,38 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do
} = device
end

test "accepts trigger payload without `trigger_name` key (Astarte < 1.2.0)", ctx do
%{conn: conn, path: path} = ctx

stub(DeviceStatusMock, :get, fn _client, _device_id -> {:ok, device_status_fixture()} end)

device_id = random_device_id()
timestamp = utc_now_second()

astarte_pre_1_2_0_event =
device_id
|> connection_trigger(timestamp)
|> Map.delete(:trigger_name)

assert conn |> post(path, astarte_pre_1_2_0_event) |> response(200)
end

test "accepts arbitrary additional keys in the trigger payload", ctx do
%{conn: conn, path: path} = ctx

stub(DeviceStatusMock, :get, fn _client, _device_id -> {:ok, device_status_fixture()} end)

device_id = random_device_id()
timestamp = utc_now_second()

extra_key_event =
device_id
|> connection_trigger(timestamp)
|> Map.put(:random, "key")

assert conn |> post(path, extra_key_event) |> response(200)
end

test "disconnection events update an existing device, not calling Astarte", ctx do
%{
conn: conn,
Expand Down Expand Up @@ -395,6 +427,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do
path = Routes.astarte_trigger_path(conn, :process_event, tenant.slug)

ota_event = %{
trigger_name: "edgehog-ota-event",
device_id: device.device_id,
event: %{
type: "incoming_data",
Expand Down Expand Up @@ -479,6 +512,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do
path = Routes.astarte_trigger_path(conn, :process_event, tenant.slug)

ota_event = %{
trigger_name: "edgehog-ota-event",
device_id: device.device_id,
event: %{
type: "incoming_data",
Expand Down Expand Up @@ -516,6 +550,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do

defp connection_trigger(device_id, timestamp) do
%{
trigger_name: "edgehog-connection",
device_id: device_id,
event: %{
type: "device_connected",
Expand All @@ -527,6 +562,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do

defp disconnection_trigger(device_id, timestamp) do
%{
trigger_name: "edgehog-disconnection",
device_id: device_id,
event: %{
type: "device_disconnected"
Expand All @@ -539,6 +575,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do

defp part_number_trigger(device_id, part_number) do
%{
trigger_name: "edgehog-system-info",
device_id: device_id,
event: %{
type: "incoming_data",
Expand All @@ -552,6 +589,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do

defp serial_number_trigger(device_id, serial_number) do
%{
trigger_name: "edgehog-system-info",
device_id: device_id,
event: %{
type: "incoming_data",
Expand All @@ -565,6 +603,7 @@ defmodule EdgehogWeb.Controllers.AstarteTriggerControllerTest do

defp unknown_trigger(device_id) do
%{
trigger_name: "other-trigger",
device_id: device_id,
event: %{
type: "incoming_data",
Expand Down

0 comments on commit ffb3486

Please sign in to comment.