fix(dup): fix possible crash on interface-specific volatile triggers

previously, when
1. installing a volatile trigger for a specific interface
2. the interface results in a cache miss but is valid for the device

the volatile trigger would be installed but the device status would not
be updated.

this results in a crash when the trigger generated events, as the logic
expects the interfaces to be in the data updater state.

by just loading the interfaces normally, we avoid this possible crash

Signed-off-by: Francesco Noacco <francesco.noacco@secomind.com>

Author: noaccOS

apps/astarte_data_updater_plant/lib/astarte_data_updater_plant/data_updater/core/trigger.ex

@@ -35,7 +35,6 @@ defmodule Astarte.DataUpdaterPlant.DataUpdater.Core.Trigger do
   alias Astarte.Core.Triggers.DataTrigger
   alias Astarte.Core.InterfaceDescriptor
   alias Astarte.Core.Mapping.EndpointsAutomaton
-  alias Astarte.DataAccess.Interface, as: InterfaceQueries
   alias Astarte.DataUpdaterPlant.TriggerPolicy.Queries, as: PolicyQueries
   alias Astarte.Core.Triggers.SimpleTriggersProtobuf.DeviceTrigger, as: ProtobufDeviceTrigger
   alias Astarte.Core.Triggers.SimpleTriggersProtobuf.DataTrigger, as: ProtobufDataTrigger
@@ -376,38 +375,17 @@ defmodule Astarte.DataUpdaterPlant.DataUpdater.Core.Trigger do
         {:data_trigger, %ProtobufDataTrigger{interface_name: "*"}} ->
           {:ok, load_trigger(new_state, trigger, target)}
 
-        {:data_trigger,
-         %ProtobufDataTrigger{
-           interface_name: interface_name,
-           interface_major: major,
-           match_path: "/*"
-         }} ->
-          with :ok <-
-                 InterfaceQueries.check_if_interface_exists(state.realm, interface_name, major) do
-            {:ok, new_state}
-          else
-            {:error, reason} ->
-              # State rollback here
-              {{:error, reason}, state}
-          end
-
         {:data_trigger,
          %ProtobufDataTrigger{
            interface_name: interface_name,
            interface_major: major,
            match_path: match_path
          }} ->
-          with {:ok, %InterfaceDescriptor{automaton: automaton}} <-
-                 InterfaceQueries.fetch_interface_descriptor(state.realm, interface_name, major),
-               {:ok, _endpoint_id} <- EndpointsAutomaton.resolve_path(match_path, automaton) do
-            {:ok, new_state}
+          with {:ok, descriptor, new_state} <- handle_cache_miss(new_state, interface_name),
+               :ok <- check_interface_major_version(descriptor, major),
+               :ok <- check_trigger_path(match_path, descriptor.automaton) do
+            {:ok, load_trigger(new_state, trigger, target)}
           else
-            {:error, :not_found} ->
-              {{:error, :invalid_match_path}, state}
-
-            {:guessed, _} ->
-              {{:error, :invalid_match_path}, state}
-
             {:error, reason} ->
               # State rollback here
               {{:error, reason}, state}
@@ -419,6 +397,32 @@ defmodule Astarte.DataUpdaterPlant.DataUpdater.Core.Trigger do
     end
   end
 
+  defp handle_cache_miss(state, interface_name) do
+    case Core.Interface.maybe_handle_cache_miss(nil, interface_name, state) do
+      {:ok, _interface_descriptor, _new_state} = ok -> ok
+      {:error, :interface_loading_failed} -> {:error, :interface_not_found}
+    end
+  end
+
+  defp check_trigger_path("/*", _automaton) do
+    :ok
+  end
+
+  defp check_trigger_path(path, automaton) do
+    case EndpointsAutomaton.resolve_path(path, automaton) do
+      {:ok, _endpoint_id} -> :ok
+      {:guessed, _} -> {:error, :invalid_match_path}
+      {:error, :not_found} -> {:error, :invalid_match_path}
+    end
+  end
+
+  defp check_interface_major_version(descriptor, major) do
+    case descriptor.major_version do
+      ^major -> :ok
+      _ -> {:error, :interface_major_version_mismatch}
+    end
+  end
+
   def handle_delete_volatile_trigger(state, trigger_id) do
     {new_volatile, maybe_trigger} =
       Enum.reduce(state.volatile_triggers, {[], nil}, fn item, {acc, found} ->

apps/astarte_data_updater_plant/test/astarte_data_updater_plant/data_updater/core/trigger_handler_test.exs

@@ -68,18 +68,24 @@ defmodule Astarte.DataUpdaterPlant.DataUpdater.Core.TriggerHandlerTest do
   property "successfully install volatile data trigger for specific interface", %{
     state: state,
     realm_name: realm_name,
-    device: device
+    device: device,
+    individual_datastream_device_interface: interface,
+    registered_paths: registered_paths
   } do
+    path =
+      registered_paths[{interface.name, interface.major_version}]
+      |> Enum.random()
+
     simple_trigger =
       %SimpleTriggerContainer{
         simple_trigger: {
           :data_trigger,
           %DataTrigger{
             version: 1,
-            interface_name: "com.test.SimpleStreamTest",
-            interface_major: 1,
+            interface_name: interface.name,
+            interface_major: interface.major_version,
             data_trigger_type: :INCOMING_DATA,
-            match_path: "/0/value",
+            match_path: path,
             value_match_operator: :LESS_THAN,
             known_value: Cyanide.encode!(%{v: 100})
           }

apps/astarte_data_updater_plant/test/astarte_data_updater_plant/data_updater/time_based_actions_test.exs

@@ -949,6 +949,23 @@ defmodule Astarte.DataUpdaterPlant.TimeBasedActionsTest do
          timestamp,
          trigger_opts \\ []
        ) do
+    match_path = trigger_opts[:match_path] || "/0/value"
+
+    interface_params = %{
+      interface_name: trigger_opts[:interface_name] || "com.test.SimpleStreamTest",
+      version_major: trigger_opts[:interface_major] || 1,
+      version_minor: 1,
+      type: :datastream,
+      ownership: :device,
+      aggregation: :individual,
+      mappings: [
+        %{
+          endpoint: trigger_opts[:endpoint] || match_path,
+          value_type: :integer
+        }
+      ]
+    }
+
     trigger_data =
       case trigger_type do
         :device_trigger ->
@@ -966,10 +983,10 @@ defmodule Astarte.DataUpdaterPlant.TimeBasedActionsTest do
             simple_trigger: {
               :data_trigger,
               %DataTrigger{
-                interface_name: trigger_opts[:interface_name] || "com.test.SimpleStreamTest",
-                interface_major: trigger_opts[:interface_major] || 1,
+                interface_name: interface_params.interface_name,
+                interface_major: interface_params.version_major,
                 data_trigger_type: trigger_opts[:data_trigger_type] || :INCOMING_DATA,
-                match_path: trigger_opts[:match_path] || "/0/value",
+                match_path: match_path,
                 value_match_operator: trigger_opts[:value_match_operator] || :LESS_THAN,
                 known_value: trigger_opts[:known_value] || Cyanide.encode!(%{v: 100})
               }
@@ -993,7 +1010,11 @@ defmodule Astarte.DataUpdaterPlant.TimeBasedActionsTest do
     volatile_trigger_id = :crypto.strong_rand_bytes(16)
     ref = if trigger_type == :data_trigger, do: 2, else: 1
 
-    insert_device_and_start_data_updater(realm, device_id)
+    ensure_interface_exists(realm, interface_params)
+
+    insert_device_and_start_data_updater(realm, device_id,
+      introspection: %{interface_params.interface_name => interface_params.version_major}
+    )
 
     :ok =
       DataUpdater.handle_install_volatile_trigger(
@@ -1025,4 +1046,19 @@ defmodule Astarte.DataUpdaterPlant.TimeBasedActionsTest do
     setup_data_updater(realm_name, encoded_device_id)
     :ok
   end
+
+  defp ensure_interface_exists(realm_name, interface) do
+    case Astarte.DataAccess.Interface.check_if_interface_exists(
+           realm_name,
+           interface.interface_name,
+           interface.version_major
+         ) do
+      :ok ->
+        :ok
+
+      {:error, :interface_not_found} ->
+        interface_json = Jason.encode!(interface)
+        Astarte.RealmManagement.Engine.install_interface(realm_name, interface_json)
+    end
+  end
 end

apps/astarte_data_updater_plant/test/astarte_data_updater_plant/data_updater_test.exs

@@ -1710,10 +1710,6 @@ defmodule Astarte.DataUpdaterPlant.DataUpdaterTest do
     |> SimpleTriggerContainer.encode()
   end
 
-  defp generate_trigger_target() do
-    generate_trigger_target(nil)
-  end
-
   defp generate_trigger_target(realm) do
     %TriggerTargetContainer{
       trigger_target: {