Merge pull request #22 from rbino/add-unset

Add unset_property function

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## Unreleased
 ### Added
 - Make the HTTP client follow redirects when interacting with Pairing API
+- Add `unset_property` function to unset a path on a property interface.
 
 ### Changed
 - Increase the max certificate chain length to 10.

lib/astarte_device.ex

@@ -276,6 +276,22 @@ defmodule Astarte.Device do
     :gen_statem.call(pid, {:set_property, interface_name, path, value})
   end
 
+  @doc """
+  Unset a property value to Astarte.
+
+  This call is blocking and waits for the message to be ACKed at the MQTT level.
+  """
+  @spec unset_property(
+          pid :: pid(),
+          interface_name :: String.t(),
+          path :: String.t()
+        ) ::
+          :ok
+          | {:error, reason :: term()}
+  def unset_property(pid, interface_name, path) do
+    :gen_statem.call(pid, {:unset_property, interface_name, path})
+  end
+
   @doc """
   Blocks until the device succesfully connects to the broker, then returns `:ok`.
 
@@ -544,6 +560,20 @@ defmodule Astarte.Device do
     {:keep_state_and_data, actions}
   end
 
+  def connected({:call, from}, {:unset_property, interface_name, path}, data) do
+    publish_params = [
+      publish_type: :properties,
+      interface_name: interface_name,
+      path: path,
+      value: nil,
+      opts: [qos: 2]
+    ]
+
+    reply = Impl.publish(publish_params, data)
+    actions = [{:reply, from, reply}]
+    {:keep_state_and_data, actions}
+  end
+
   def connected(_event_type, _event, _data) do
     :keep_state_and_data
   end

lib/astarte_device/impl.ex

@@ -263,8 +263,7 @@ defmodule Astarte.Device.Impl do
          :ok <- validate_publish_type(publish_type, interface),
          :ok <- validate_ownership(:device, interface),
          :ok <- validate_path_and_type(interface, path, value),
-         payload_map = build_payload_map(value, opts),
-         {:ok, bson_payload} <- Cyanide.encode(payload_map) do
+         {:ok, payload} <- build_payload(value, opts) do
       publish_opts = Keyword.take(opts, [:qos])
       "/" <> bare_path = path
 
@@ -276,7 +275,7 @@ defmodule Astarte.Device.Impl do
           Enum.join([client_id, interface_name, bare_path], "/")
         end
 
-      @connection.publish_sync(client_id, topic, bson_payload, publish_opts)
+      @connection.publish_sync(client_id, topic, payload, publish_opts)
     end
   end
 
@@ -452,6 +451,16 @@ defmodule Astarte.Device.Impl do
     end
   end
 
+  defp build_payload(nil, _opts) do
+    # If value is nil (which can only happen for an unset), we send an empty binary
+    {:ok, <<>>}
+  end
+
+  defp build_payload(value, opts) do
+    payload_map = build_payload_map(value, opts)
+    Cyanide.encode(payload_map)
+  end
+
   defp validate_publish_type(publish_type, %Interface{type: publish_type}) do
     :ok
   end
@@ -477,10 +486,13 @@ defmodule Astarte.Device.Impl do
   end
 
   defp validate_path_and_type(%Interface{aggregation: :individual} = interface, path, value) do
-    mappings = interface.mappings
+    %Interface{
+      mappings: mappings,
+      type: type
+    } = interface
 
-    with {:ok, %Mapping{value_type: expected_type}} <- find_mapping(path, mappings) do
-      Mapping.ValueType.validate_value(expected_type, value)
+    with {:ok, mapping} <- find_mapping(path, mappings) do
+      validate_value(type, mapping, value)
     end
   end
 
@@ -494,6 +506,19 @@ defmodule Astarte.Device.Impl do
     {:error, :cannot_resolve_path}
   end
 
+  defp validate_value(:properties, %Mapping{allow_unset: allow_unset}, nil) do
+    # If value is nil on a properties interface, check if we can unset
+    if allow_unset do
+      :ok
+    else
+      {:error, :unset_not_allowed}
+    end
+  end
+
+  defp validate_value(_type, %Mapping{value_type: expected_type}, value) do
+    Mapping.ValueType.validate_value(expected_type, value)
+  end
+
   defp classify_error({:api, {:error, reason}}, log_tag)
        when reason in [:econnrefused, :closed, :timeout] do
     # Temporary errors

test/astarte_device_test.exs

@@ -237,6 +237,48 @@ defmodule Astarte.DeviceTest do
     end
   end
 
+  describe "unset_property" do
+    setup [:set_mox_from_context, :verify_on_exit!, :init_device]
+
+    test "fails on a datastream interface", %{device: device} do
+      interface = "org.astarteplatform.test.DeviceDatastream"
+      path = "/realValue"
+      value = 42.0
+
+      :ok = wait_for_connection()
+
+      assert Device.unset_property(device, interface, path) == {:error, :datastream_interface}
+    end
+
+    test "fails on a property interface without allow_unset", %{device: device} do
+      interface = "org.astarteplatform.test.DeviceProperties"
+      path = "/intValue"
+      value = 42
+
+      :ok = wait_for_connection()
+
+      assert Device.unset_property(device, interface, path) == {:error, :unset_not_allowed}
+    end
+
+    test "succeeds on a property interface", %{device: device} do
+      interface = "org.astarteplatform.test.DeviceProperties"
+      path = "/stringValue"
+      value = 42
+
+      full_path = "#{@realm}/#{@device_id}/#{interface}#{path}"
+
+      ConnectionMock
+      |> expect(:publish_sync, fn _client_id, ^full_path, payload, [qos: 2] ->
+        assert payload == <<>>
+        :ok
+      end)
+
+      :ok = wait_for_connection()
+
+      assert Device.unset_property(device, interface, path) == :ok
+    end
+  end
+
   defp init_device(_) do
     test_process = self()
     introspection_topic = @client_id

test/interfaces/org.astarteplatform.DeviceProperties.json

@@ -8,6 +8,11 @@
        {
            "endpoint": "/intValue",
            "type": "integer"
+       },
+       {
+           "endpoint": "/stringValue",
+           "type": "string",
+           "allow_unset": true
        }
     ]
 }