Merge pull request #65 from shinnokdisengir/fix/individual-mapping-interface

shinnokdisengir · web-flow · commit eeefbfce3bfe · 2025-08-22T09:43:16.000+02:00
fix: aggregation individual are now more diversified + test on :object
diff --git a/lib/astarte/core/generators/interface.ex b/lib/astarte/core/generators/interface.ex
@@ -27,11 +27,14 @@ defmodule Astarte.Core.Generators.Interface do
 
   alias Astarte.Core.CQLUtils
   alias Astarte.Core.Interface
+  alias Astarte.Core.Mapping
 
   alias Astarte.Core.Generators.Mapping, as: MappingGenerator
 
   alias Astarte.Utilities.Map, as: MapUtilities
 
+  @interface_max_mappings 10
+
   @doc """
   Generates a valid Astarte Interface.
 
@@ -46,8 +49,7 @@ defmodule Astarte.Core.Generators.Interface do
                    type <- type(),
                    ownership <- ownership(),
                    aggregation <- aggregation(type),
-                   prefix <- prefix(),
-                   mappings <- mappings(type, name, major_version, prefix),
+                   mappings <- mappings(aggregation, type, name, major_version),
                    description <- description(),
                    doc <- doc(),
                    params: params do
@@ -59,7 +61,6 @@ defmodule Astarte.Core.Generators.Interface do
           minor_version: minor_version,
           type: type,
           ownership: ownership,
-          prefix: prefix,
           aggregation: aggregation,
           mappings: mappings,
           description: description,
@@ -131,7 +132,7 @@ defmodule Astarte.Core.Generators.Interface do
 
   https://docs.astarte-platform.org/astarte/latest/030-interface.html#interface-type
   """
-  @spec type() :: StreamData.t(String.t())
+  @spec type() :: StreamData.t(:datastream | :properties)
   def type, do: member_of([:datastream, :properties])
 
   @doc false
@@ -182,40 +183,89 @@ defmodule Astarte.Core.Generators.Interface do
 
   defp ownership, do: member_of([:device, :server])
 
-  defp prefix, do: MappingGenerator.endpoint()
+  defp interface_mapping(params, endpoint),
+    do: MappingGenerator.mapping(params ++ [endpoint: endpoint])
 
-  defp mappings(interface_type, interface_name, interface_major, prefix) do
-    gen all retention <- MappingGenerator.retention(interface_type),
-            reliability <- MappingGenerator.reliability(interface_type),
-            expiry <- MappingGenerator.expiry(interface_type),
-            allow_unset <- MappingGenerator.allow_unset(interface_type),
-            explicit_timestamp <- MappingGenerator.explicit_timestamp(interface_type),
-            mappings <-
-              MappingGenerator.endpoint_segment()
-              |> list_of(min_length: 1, max_length: 10)
-              |> map(&Enum.uniq_by(&1, fn endpoint -> String.downcase(endpoint) end))
-              |> bind(fn list ->
-                list
-                |> Enum.map(fn postfix ->
-                  MappingGenerator.mapping(
-                    interface_type: interface_type,
-                    interface_name: interface_name,
-                    interface_major: interface_major,
-                    retention: retention,
-                    reliability: reliability,
-                    expiry: expiry,
-                    allow_unset: allow_unset,
-                    explicit_timestamp: explicit_timestamp,
-                    endpoint: prefix <> "/" <> postfix
-                  )
-                end)
-                |> fixed_list()
-              end) do
+  defp interface_mappings(:individual, params) do
+    MappingGenerator.endpoint()
+    |> bind(&interface_mapping(params, &1))
+    |> list_of(min_length: 1, max_length: @interface_max_mappings)
+    |> map(&uniq_endpoints/1)
+  end
+
+  defp interface_mappings(:object, params) do
+    MappingGenerator.endpoint()
+    |> bind(fn endpoint ->
+      MappingGenerator.endpoint_segment()
+      |> bind(&interface_mapping(params, endpoint <> &1))
+      |> list_of(min_length: 1, max_length: @interface_max_mappings)
+      |> map(&uniq_endpoints/1)
+    end)
+  end
+
+  defp mappings(aggregation, interface_type, interface_name, interface_major) do
+    common =
+      gen all retention <- MappingGenerator.retention(interface_type),
+              reliability <- MappingGenerator.reliability(interface_type),
+              expiry <- MappingGenerator.expiry(interface_type),
+              allow_unset <- MappingGenerator.allow_unset(interface_type),
+              explicit_timestamp <- MappingGenerator.explicit_timestamp(interface_type) do
+        [
+          retention: retention,
+          reliability: reliability,
+          expiry: expiry,
+          allow_unset: allow_unset,
+          explicit_timestamp: explicit_timestamp
+        ]
+      end
+
+    gen all common_params <- common,
+            other_params = [
+              interface_type: interface_type,
+              interface_name: interface_name,
+              interface_major: interface_major
+            ],
+            params = common_params ++ other_params,
+            mappings <- interface_mappings(aggregation, params) do
       mappings
     end
   end
 
   defp description, do: one_of([nil, string(:ascii, min_length: 1, max_length: 1000)])
 
   defp doc, do: one_of([nil, string(:ascii, min_length: 1, max_length: 100_000)])
+
+  # Utilities
+
+  @normalized_param ""
+
+  @doc false
+  @spec uniq_endpoints(list(Mapping.t())) :: list(Mapping.t())
+  def uniq_endpoints([%Mapping{} | _] = mappings),
+    do: mappings |> uniq_endpoints(MapSet.new(), MapSet.new())
+
+  defp uniq_endpoints([], _endpoints, acc), do: MapSet.to_list(acc)
+
+  defp uniq_endpoints([%Mapping{endpoint: endpoint} = mapping | rest], prefixes, acc) do
+    prefix = endpoint |> uniform_param() |> tokenize_endpoint()
+
+    if conflict_exists?(prefixes, prefix) do
+      uniq_endpoints(rest, prefixes, acc)
+    else
+      uniq_endpoints(rest, MapSet.put(prefixes, prefix), MapSet.put(acc, mapping))
+    end
+  end
+
+  defp tokenize_endpoint(endpoint), do: endpoint |> String.split("/") |> Enum.drop(1)
+
+  defp conflict?([], _), do: true
+  defp conflict?(_, []), do: true
+  defp conflict?([@normalized_param | ta], [_ | tb]), do: conflict?(ta, tb)
+  defp conflict?([_ | ta], [@normalized_param | tb]), do: conflict?(ta, tb)
+  defp conflict?([h | ta], [h | tb]), do: conflict?(ta, tb)
+  defp conflict?(_, _), do: false
+
+  defp conflict_exists?(prefixes, test), do: Enum.any?(prefixes, &conflict?(&1, test))
+
+  defp uniform_param(endpoint), do: Mapping.normalize_endpoint(endpoint) |> String.downcase()
 end
diff --git a/lib/astarte/core/generators/mapping.ex b/lib/astarte/core/generators/mapping.ex
@@ -101,34 +101,35 @@ defmodule Astarte.Core.Generators.Mapping do
   """
   @spec endpoint() :: StreamData.t(String.t())
   def endpoint do
-    gen all prefix <- endpoint_segment(),
-            segments <-
+    gen all segments <-
               frequency([
                 {3, endpoint_segment()},
                 {1, endpoint_segment_param()}
               ])
               |> list_of(min_length: 1, max_length: 5) do
-      "/" <> prefix <> "/" <> Enum.join(segments, "/")
+      Enum.join(segments, "")
     end
   end
 
   @doc """
   Generates a generic endpoint segment.
   """
   @spec endpoint_segment() :: StreamData.t(StreamData.t(String.t()))
-  def endpoint_segment do
-    gen all prefix <- string(@unix_prefix_path_chars, length: 1),
-            rest <- string(@unix_path_chars, max_length: 19) do
-      prefix <> rest
-    end
-  end
+  def endpoint_segment, do: endpoint_segment_content() |> map(fn content -> "/" <> content end)
 
   @doc """
   Generates a parametrized endpoint segment.
   """
   @spec endpoint_segment_param() :: StreamData.t(StreamData.t(String.t()))
   def endpoint_segment_param,
-    do: endpoint_segment() |> map(fn segment -> "%{" <> segment <> "}" end)
+    do: endpoint_segment_content() |> map(fn content -> "/%{" <> content <> "}" end)
+
+  defp endpoint_segment_content do
+    gen all prefix <- string(@unix_prefix_path_chars, length: 1),
+            rest <- string(@unix_path_chars, max_length: 19) do
+      prefix <> rest
+    end
+  end
 
   defp endpoint_id(interface_name, interface_major, endpoint),
     do: constant(CQLUtils.endpoint_id(interface_name, interface_major, endpoint))
@@ -154,12 +155,12 @@ defmodule Astarte.Core.Generators.Mapping do
 
   @doc false
   @spec reliability(:datastream | :properties) ::
-          StreamData.t(:unreliable | :guaranteed | :unique)
+          StreamData.t(:unreliable | :guaranteed | :unique | nil)
   def reliability(:datastream), do: member_of([:unreliable, :guaranteed, :unique])
   def reliability(_), do: constant(nil)
 
   @doc false
-  @spec explicit_timestamp(:datastream | :properties) :: StreamData.t(nil | boolean())
+  @spec explicit_timestamp(:datastream | :properties) :: StreamData.t(boolean())
   def explicit_timestamp(:datastream), do: boolean()
   def explicit_timestamp(_), do: constant(false)
 
@@ -169,18 +170,19 @@ defmodule Astarte.Core.Generators.Mapping do
   def retention(_), do: constant(:discard)
 
   @doc false
-  @spec expiry(:datastream | :properties) :: StreamData.t(0 | pos_integer())
+  @spec expiry(:datastream | :properties) :: StreamData.t(pos_integer() | 0)
   def expiry(:datastream), do: one_of([constant(0), integer(1..10_000)])
   def expiry(_), do: constant(0)
 
   @doc false
-  @spec database_retention_policy(:datastream | :properties) :: StreamData.t(:no_ttl | :use_ttl)
+  @spec database_retention_policy(:datastream | :properties) ::
+          StreamData.t(:no_ttl | :use_ttl | nil)
   def database_retention_policy(:datastream), do: member_of([:no_ttl, :use_ttl])
   def database_retention_policy(_), do: constant(nil)
 
   @doc false
   @spec database_retention_ttl(:datastream | :properties, :use_ttl | :no_ttl) ::
-          StreamData.t(nil | non_neg_integer())
+          StreamData.t(non_neg_integer() | nil)
   def database_retention_ttl(:datastream, :use_ttl), do: integer(60..1_048_576)
   def database_retention_ttl(_, _), do: constant(nil)
 
diff --git a/test/astarte/core/generators/interface_test.exs b/test/astarte/core/generators/interface_test.exs
@@ -23,12 +23,68 @@ defmodule Astarte.Core.Generators.InterfaceTest do
   use ExUnit.Case, async: true
   use ExUnitProperties
 
-  alias Astarte.Core.Generators.Interface, as: InterfaceGenerator
   alias Astarte.Core.Interface
+  alias Astarte.Core.Mapping
+
+  alias Astarte.Core.Generators.Interface, as: InterfaceGenerator
 
   @moduletag :core
   @moduletag :interface
 
+  @endpoint_cases [
+    {["/AbCde", "/AbCde/QwEr", "/XyZ"], ["/AbCde", "/XyZ"]},
+    {["/shop/Prd45", "/shop/Prd45/Details"], ["/shop/Prd45"]},
+    {["/alpha/Beta", "/alpha/Beta/Gamma", "/alpha/Delta"], ["/alpha/Beta", "/alpha/Delta"]},
+    {["/%{RootB}/Xy", "/%{RootA}", "/%{RootA}/Xy", "/Abc"], ["/%{RootB}/Xy"]},
+    {["/%{RootB}", "/%{RootB}/Seg1", "/%{RootB}/Seg1/Seg2"], ["/%{RootB}"]},
+    {["/api/%{CA}", "/api/%{CA}/status", "/api/%{VA}/status"], ["/api/%{CA}"]},
+    {["/api/%{V_1}", "/api/%{V_1}/ping", "/api/%{V_2}"], ["/api/%{V_1}"]},
+    {["/api/%{V_1}/detail", "/%{mask}/detail", "/api/%{V_2}"], ["/api/%{V_1}/detail"]},
+    {["/Ux9/Za2", "/Ux9/Za2/Rt7"], ["/Ux9/Za2"]},
+    {["/foo/%{idA}", "/foo/%{idB}/bar"], ["/foo/%{idA}"]},
+    {["/foo/%{Ab1}", "/foo/%{Ab1}/X", "/bar/Baz"], ["/foo/%{Ab1}", "/bar/Baz"]},
+    {["/mmNn/ooPP", "/mmNn/ooPP/qqRR"], ["/mmNn/ooPP"]},
+    {["/docs/%{Slug_1}", "/docs/%{Slug_1}/v1", "/docs/%{Slug_2}"], ["/docs/%{Slug_1}"]},
+    {["/AA/BB", "/AA/BB/CC", "/DD"], ["/AA/BB", "/DD"]},
+    {["/AA/BB/CC", "/AA/BB"], ["/AA/BB/CC"]},
+    {["/xY/Za", "/xY/Za/Q1", "/xY/Zb"], ["/xY/Za", "/xY/Zb"]},
+    {["/p1q2/RS", "/p1q2/RS/TT/UU"], ["/p1q2/RS"]},
+    {["/user/%{U_123}", "/user/%{U_123}/orders", "/user/%{U_456}"], ["/user/%{U_123}"]},
+    {["/calc/%{ModeA}/run", "/calc/%{ModeB}/run"], ["/calc/%{ModeA}/run"]},
+    {["/calc/%{ModeC}/run/now", "/calc/%{ModeC}/run"], ["/calc/%{ModeC}/run/now"]},
+    {["/A1b2C3", "/A1b2C3/D4e5"], ["/A1b2C3"]},
+    {["/AABB", "/AABBCC"], ["/AABB", "/AABBCC"]},
+    {["/srv/%{EnvX}/cfg", "/srv/%{EnvX}/cfg/edit"], ["/srv/%{EnvX}/cfg"]},
+    {["/srv/%{Env_1}/cfg", "/srv/%{Env_2}/cfg"], ["/srv/%{Env_1}/cfg"]},
+    {["/%{ROOT_X}", "/%{ROOT_Y}", "/%{ROOT_Y}/x"], ["/%{ROOT_X}"]},
+    {["/root/%{Id1}/x", "/root/%{Id2}/x/y"], ["/root/%{Id1}/x"]},
+    {["/root/%{Abc}/x/y", "/root/%{Abc}/x"], ["/root/%{Abc}/x/y"]},
+    {["/kLm/Nop", "/kLm/Nop/Qrs", "/kLm/Tuv"], ["/kLm/Nop", "/kLm/Tuv"]},
+    {["/data/%{KeyA}/val", "/data/%{KeyB}/val"], ["/data/%{KeyA}/val"]},
+    {["/data/%{KeyA}/val", "/data/%{KeyA}/val/x"], ["/data/%{KeyA}/val"]},
+    {["/zZz/%{Pid}/q", "/zZz/%{Pid}/q/r"], ["/zZz/%{Pid}/q"]},
+    {["/zZz/%{Pid1}/q", "/zZz/%{Pid2}/w"], ["/zZz/%{Pid1}/q", "/zZz/%{Pid2}/w"]},
+    {["/Mix/Aa", "/Mix/Aa/Bb", "/Mix/Cc"], ["/Mix/Aa", "/Mix/Cc"]},
+    {["/auth/Login", "/auth/Login/Step2"], ["/auth/Login"]},
+    {["/auth/%{FlowA}/step/next", "/auth/%{FlowA}/step"], ["/auth/%{FlowA}/step/next"]},
+    {["/auth/%{FlowA}/step", "/auth/%{FlowB}"], ["/auth/%{FlowA}/step"]},
+    {["/r1/r2/r3", "/r1/r2"], ["/r1/r2/r3"]},
+    {["/r1/%{VarA}", "/r1/%{VarA}/x", "/r2"], ["/r1/%{VarA}", "/r2"]},
+    {["/A_/B_", "/A_/B_/C_"], ["/A_/B_"]},
+    {["/A_/B_/C_", "/A_/B_/C_/D_"], ["/A_/B_/C_"]},
+    {["/node/%{N1}/leaf/%{L2}", "/node/%{N1}/leaf"], ["/node/%{N1}/leaf/%{L2}"]},
+    {["/node/%{N1}/leaf/%{L2}", "/node/%{N2}/leaf/%{L3}"], ["/node/%{N1}/leaf/%{L2}"]},
+    {["/Sx/Tx", "/Sx/Tx/Ux", "/Sx/Vx"], ["/Sx/Tx", "/Sx/Vx"]},
+    {["/G1/H2", "/G1/H2/I3/J4"], ["/G1/H2"]},
+    {["/lib/%{Pkg}/v", "/lib/%{Pkg}/v/1"], ["/lib/%{Pkg}/v"]},
+    {["/lib/%{PkgA}/v", "/lib/%{PkgB}/v"], ["/lib/%{PkgA}/v"]},
+    {["/cat/%{C1}/dog/%{D2}", "/cat/%{C1}/dog/%{D2}/x"], ["/cat/%{C1}/dog/%{D2}"]},
+    {["/alpha/%{A1}/beta", "/alpha/%{A1}/beta/gamma"], ["/alpha/%{A1}/beta"]},
+    {["/alpha/%{A1}/beta", "/alpha/%{A2}"], ["/alpha/%{A1}/beta"]},
+    {["/xLong/SegName", "/xLong/SegName/Next"], ["/xLong/SegName"]},
+    {["/Ping", "/Ping/Pong", "/Pang"], ["/Ping", "/Pang"]}
+  ]
+
   @doc false
   describe "interface generator" do
     @describetag :success
@@ -51,6 +107,35 @@ defmodule Astarte.Core.Generators.InterfaceTest do
       end
     end
 
+    test "validate endpoints in :individual" do
+      for {endpoints, results} <- @endpoint_cases do
+        mappings =
+          for endpoint <- endpoints do
+            %Mapping{endpoint: endpoint}
+          end
+
+        uniq_mappings = InterfaceGenerator.uniq_endpoints(mappings)
+
+        uniq_endpoints =
+          for %Mapping{endpoint: endpoint} <- uniq_mappings do
+            endpoint
+          end
+
+        assert MapSet.new(results) == MapSet.new(uniq_endpoints)
+      end
+    end
+
+    property "validate endpoints in aggregation :object must be the same" do
+      check all %Interface{mappings: mappings} <-
+                  InterfaceGenerator.interface(aggregation: :object),
+                endpoints =
+                  mappings
+                  |> Enum.map(fn %Mapping{endpoint: endpoint} -> endpoint end)
+                  |> Enum.map(&Regex.replace(~r"/[^/]+$", &1, "")) do
+        assert 1 == endpoints |> Enum.uniq() |> length()
+      end
+    end
+
     @tag issue: 45
     property "custom interface creation" do
       gen_interface_changes =
