improvement: Added sort direction support to custom_indexes fields. (#775)

Author: ThomaseLucas

documentation/dsls/DSL-AshPostgres.DataLayer.md

@@ -103,7 +103,7 @@ index ["column", "column2"], unique: true, where: "thing = TRUE"
 
 | Name | Type | Default | Docs |
 |------|------|---------|------|
-| [`fields`](#postgres-custom_indexes-index-fields){: #postgres-custom_indexes-index-fields } | `atom \| String.t \| list(atom \| String.t)` |  | The fields to include in the index. |
+| [`fields`](#postgres-custom_indexes-index-fields){: #postgres-custom_indexes-index-fields } | `atom \| String.t \| {:asc \| :desc, atom \| String.t} \| list(atom \| String.t \| {:asc \| :desc, atom \| String.t})` |  | The fields to include in the index. Each entry can be an atom, string, or a tuple with an order using :desc or :asc and a field. |
 ### Options
 
 | Name | Type | Default | Docs |

lib/custom_index.ex

@@ -3,7 +3,11 @@
 # SPDX-License-Identifier: MIT
 
 defmodule AshPostgres.CustomIndex do
-  @moduledoc "Represents a custom index on the table backing a resource"
+  @moduledoc """
+  Represents a custom index on the table backing a resource.
+
+  Each entry in `fields` can be an atom, string, or a tuple with an order and a field.
+  """
   @fields [
     :table,
     :fields,
@@ -26,8 +30,19 @@ defmodule AshPostgres.CustomIndex do
 
   @schema [
     fields: [
-      type: {:wrap_list, {:or, [:atom, :string]}},
-      doc: "The fields to include in the index."
+      type:
+        {:wrap_list,
+         {:or,
+          [
+            :atom,
+            :string,
+            {:tuple, [{:one_of, [:asc, :desc]}, {:or, [:atom, :string]}]}
+          ]}},
+      doc: """
+      The fields to include in the index.
+
+      Each entry can be an atom, string, or a tuple with an order using :desc or :asc and a field.
+      """
     ],
     error_fields: [
       type: {:list, :atom},
@@ -83,6 +98,41 @@ defmodule AshPostgres.CustomIndex do
 
   def schema, do: @schema
 
+  def column_name(field) when is_atom(field) or is_binary(field), do: field
+
+  def column_name({order, field})
+      when order in [:asc, :desc] and (is_atom(field) or is_binary(field)),
+      do: field
+
+  def field_to_snapshot(field) when is_atom(field), do: %{type: "atom", value: field}
+  def field_to_snapshot(field) when is_binary(field), do: %{type: "string", value: field}
+
+  def field_to_snapshot({order, field})
+      when order in [:asc, :desc] and (is_atom(field) or is_binary(field)) do
+    %{
+      type: "directed",
+      order: to_string(order),
+      value: if(is_atom(field), do: to_string(field), else: field)
+    }
+  end
+
+  def field_comparison_key(field) when is_atom(field), do: to_string(field)
+  def field_comparison_key(field) when is_binary(field), do: field
+
+  def field_comparison_key({order, field})
+      when order in [:asc, :desc] and (is_atom(field) or is_binary(field)) do
+    "#{order}:#{field}"
+  end
+
+  @doc false
+  def field_for_migration(field) when is_atom(field), do: inspect(field)
+  def field_for_migration(field) when is_binary(field), do: inspect(field)
+
+  def field_for_migration({order, field})
+      when order in [:asc, :desc] and (is_atom(field) or is_binary(field)) do
+    "#{order}: #{inspect(field)}"
+  end
+
   def transform(index) do
     with {:ok, index} <- set_name(index) do
       set_error_fields(index)
@@ -99,11 +149,13 @@ defmodule AshPostgres.CustomIndex do
          index
          | error_fields:
              Enum.flat_map(index.fields, fn field ->
-               if Regex.match?(~r/^[0-9a-zA-Z_]+$/, to_string(field)) do
-                 if is_binary(field) do
-                   [String.to_atom(field)]
+               column = column_name(field)
+
+               if Regex.match?(~r/^[0-9a-zA-Z_]+$/, to_string(column)) do
+                 if is_binary(column) do
+                   [String.to_atom(column)]
                  else
-                   [field]
+                   [column]
                  end
                else
                  []
@@ -125,7 +177,8 @@ defmodule AshPostgres.CustomIndex do
 
       mismatched_field =
           Enum.find(index.fields, fn field ->
-            !Regex.match?(~r/^[0-9a-zA-Z_]+$/, to_string(field))
+            column = column_name(field)
+            !Regex.match?(~r/^[0-9a-zA-Z_]+$/, to_string(column))
           end) ->
         {:error,
          """
@@ -149,7 +202,7 @@ defmodule AshPostgres.CustomIndex do
 
   # sobelow_skip ["DOS.StringToAtom"]
   def name(table, %{fields: fields}) do
-    [table, fields, "index"]
+    [table, Enum.map(fields, &column_name/1), "index"]
     |> List.flatten()
     |> Enum.map(&to_string(&1))
     |> Enum.map(&String.replace(&1, ~r"[^\w_]", "_"))

lib/data_layer.ex

@@ -3729,10 +3729,9 @@ defmodule AshPostgres.DataLayer do
               fields -> fields
             end
           else
-            case Enum.filter(index.fields, &is_atom/1) do
-              [] -> pkey
-              fields -> fields
-            end
+            index.fields
+            |> Enum.map(&AshPostgres.CustomIndex.column_name/1)
+            |> Enum.uniq()
           end
 
         Ecto.Changeset.unique_constraint(changeset, fields, opts)

lib/migration_generator/migration_generator.ex

@@ -2790,7 +2790,7 @@ defmodule AshPostgres.MigrationGenerator do
   defp custom_index_comparison_key(index, snapshot) do
     index
     |> Map.update!(:fields, fn fields ->
-      Enum.map(fields, &to_string/1)
+      Enum.map(fields, &AshPostgres.CustomIndex.field_comparison_key/1)
     end)
     |> add_custom_index_name(snapshot.table)
     |> Map.put(:where, {snapshot.base_filter, index.where})
@@ -4597,10 +4597,7 @@ defmodule AshPostgres.MigrationGenerator do
     |> Map.update!(:custom_indexes, fn indexes ->
       Enum.map(indexes, fn index ->
         fields =
-          Enum.map(index.fields, fn
-            field when is_atom(field) -> %{type: "atom", value: field}
-            field when is_binary(field) -> %{type: "string", value: field}
-          end)
+          Enum.map(index.fields, &AshPostgres.CustomIndex.field_to_snapshot/1)
 
         %{index | fields: fields}
         |> Map.delete(:__spark_metadata__)
@@ -4708,9 +4705,17 @@ defmodule AshPostgres.MigrationGenerator do
       custom_index
       |> Map.update(:fields, [], fn fields ->
         Enum.map(fields, fn
-          %{type: "atom", value: field} -> maybe_to_atom(field)
-          %{type: "string", value: field} -> field
-          field -> field
+          %{type: "atom", value: field} ->
+            maybe_to_atom(field)
+
+          %{type: "string", value: field} ->
+            field
+
+          %{type: "directed", order: order, value: value} ->
+            {maybe_to_atom(order), maybe_to_atom(value)}
+
+          field ->
+            field
         end)
       end)
       |> Map.put_new(:include, [])

lib/migration_generator/operation.ex

@@ -1098,7 +1098,10 @@ defmodule AshPostgres.MigrationGenerator.Operation do
           base_filter: base_filter,
           multitenancy: multitenancy
         }) do
-      keys = index_keys(index.fields, index.all_tenants?, multitenancy)
+      keys =
+        index.fields
+        |> index_keys(index.all_tenants?, multitenancy)
+        |> Enum.map(&AshPostgres.CustomIndex.field_for_migration/1)
 
       index =
         case {index.where, base_filter} do
@@ -1120,26 +1123,33 @@ defmodule AshPostgres.MigrationGenerator.Operation do
           option(:prefix, schema)
         ])
 
+      columns = Enum.join(keys, ", ")
+
       if opts == "" do
-        "create index(:#{as_atom(table)}, [#{Enum.map_join(keys, ", ", &inspect/1)}])"
+        "create index(:#{as_atom(table)}, [#{columns}])"
       else
-        "create index(:#{as_atom(table)}, [#{Enum.map_join(keys, ", ", &inspect/1)}], #{opts})"
+        "create index(:#{as_atom(table)}, [#{columns}], #{opts})"
       end
     end
 
     def down(%{schema: schema, index: index, table: table, multitenancy: multitenancy}) do
-      keys = index_keys(index.fields, index.all_tenants?, multitenancy)
+      keys =
+        index.fields
+        |> index_keys(index.all_tenants?, multitenancy)
+        |> Enum.map(&AshPostgres.CustomIndex.field_for_migration/1)
 
       opts =
         join([
           option(:name, index.name),
           option(:prefix, schema)
         ])
 
+      columns = Enum.join(keys, ", ")
+
       if opts == "" do
-        "drop_if_exists index(:#{as_atom(table)}, [#{Enum.map_join(keys, ", ", &inspect/1)}])"
+        "drop_if_exists index(:#{as_atom(table)}, [#{columns}])"
       else
-        "drop_if_exists index(:#{as_atom(table)}, [#{Enum.map_join(keys, ", ", &inspect/1)}], #{opts})"
+        "drop_if_exists index(:#{as_atom(table)}, [#{columns}], #{opts})"
       end
     end
   end

test/custom_index_test.exs

@@ -29,4 +29,15 @@ defmodule AshPostgres.Test.CustomIndexTest do
                    |> Ash.create!()
                  end
   end
+
+  test "directed custom index fields populate error_fields" do
+    {:ok, index} =
+      AshPostgres.CustomIndex.transform(%AshPostgres.CustomIndex{
+        fields: [:tenant_id, {:desc, :occurred_at}],
+        unique: true,
+        name: "events_tenant_id_occurred_at_index"
+      })
+
+    assert index.error_fields == [:tenant_id, :occurred_at]
+  end
 end

test/migration_generator_test.exs

@@ -819,6 +819,49 @@ defmodule AshPostgres.MigrationGeneratorTest do
     end
   end
 
+  describe "custom_indexes with sort direction" do
+    setup %{snapshot_path: snapshot_path, migration_path: migration_path} do
+      :ok
+
+      defposts do
+        postgres do
+          custom_indexes do
+            index([desc: :title], name: "posts_title_desc_index")
+            index([:id, desc: :title], name: "posts_id_title_desc_index")
+          end
+        end
+
+        attributes do
+          uuid_primary_key(:id)
+          attribute(:title, :string, public?: true)
+        end
+      end
+
+      defdomain([Post])
+
+      AshPostgres.MigrationGenerator.generate(Domain,
+        snapshot_path: snapshot_path,
+        migration_path: migration_path,
+        quiet: true,
+        format: false,
+        auto_name: true
+      )
+    end
+
+    test "it generates create index with sort direction", %{migration_path: migration_path} do
+      assert [custom_index_migration] =
+               Enum.sort(Path.wildcard("#{migration_path}/**/*_migrate_resources*.exs"))
+               |> Enum.reject(&String.contains?(&1, "extensions"))
+
+      file = File.read!(custom_index_migration)
+
+      assert file =~ ~S/create index(:posts, [desc: :title], name: "posts_title_desc_index")/
+
+      assert file =~
+               ~S/create index(:posts, [:id, desc: :title], name: "posts_id_title_desc_index")/
+    end
+  end
+
   describe "custom_indexes with follow up migrations" do
     setup %{snapshot_path: snapshot_path, migration_path: migration_path} do
       :ok