From 40f3eeff7c66d23fc7798619c0c9ef8d9bd95527 Mon Sep 17 00:00:00 2001
From: Pim Kunis <pim@kunis.nl>
Date: Sat, 4 Sep 2021 16:40:17 +0200
Subject: [PATCH] Improve primary keys usage

---
 lib/architex/room_server.ex                    | 18 +++++++++---------
 lib/architex/schema/account.ex                 |  2 +-
 lib/architex/schema/device.ex                  | 18 ++++++++++--------
 lib/architex/schema/device_transaction.ex      |  4 ++--
 lib/architex/schema/event.ex                   | 11 ++++++-----
 lib/architex/schema/room.ex                    |  4 ++--
 lib/architex/state_resolution.ex               | 14 +++++++-------
 lib/architex/state_resolution/authorization.ex |  4 ++--
 .../client/controllers/login_controller.ex     |  2 +-
 .../client/controllers/register_controller.ex  |  2 +-
 .../client/controllers/room_controller.ex      |  9 +++++++++
 .../federation/controllers/event_controller.ex |  4 ++--
 lib/architex_web/router.ex                     |  1 +
 .../20210830160818_create_initial_tables.exs   | 17 +++++++++++------
 priv/repo/seeds.exs                            |  2 +-
 test/support/factory.ex                        |  2 +-
 16 files changed, 66 insertions(+), 48 deletions(-)

diff --git a/lib/architex/room_server.ex b/lib/architex/room_server.ex
index aaccbc8..da567b8 100644
--- a/lib/architex/room_server.ex
+++ b/lib/architex/room_server.ex
@@ -183,7 +183,7 @@ defmodule Architex.RoomServer do
 
     state_set =
       Event
-      |> where([e], e.event_id in ^state_event_ids)
+      |> where([e], e.id in ^state_event_ids)
       |> Repo.all()
       |> Enum.into(%{}, fn %Event{type: type, state_key: state_key} = event ->
         {{type, state_key}, event}
@@ -228,7 +228,7 @@ defmodule Architex.RoomServer do
     room_events =
       Event
       |> where([e], e.room_id == ^room_id)
-      |> select([e], {e.event_id, e})
+      |> select([e], {e.id, e})
       |> Repo.all()
       |> Enum.into(%{})
 
@@ -248,12 +248,12 @@ defmodule Architex.RoomServer do
     room_events =
       Event
       |> where([e], e.room_id == ^room_id)
-      |> select([e], {e.event_id, e})
+      |> select([e], {e.id, e})
       |> Repo.all()
       |> Enum.into(%{})
 
     state_set = StateResolution.resolve(event, false)
-    state_events = Enum.map(state_set, fn {_, %Event{event_id: event_id}} -> event_id end)
+    state_events = Enum.map(state_set, fn {_, %Event{id: event_id}} -> event_id end)
 
     auth_chain =
       state_set
@@ -372,7 +372,7 @@ defmodule Architex.RoomServer do
   defp insert_custom_message(
          state_set,
          room,
-         %Device{id: device_id} = device,
+         %Device{nid: device_nid} = device,
          message_event,
          txn_id
        ) do
@@ -380,13 +380,13 @@ defmodule Architex.RoomServer do
       # Check if we already executed this transaction.
       case Repo.one(
              from dt in DeviceTransaction,
-               where: dt.txn_id == ^txn_id and dt.device_id == ^device_id
+               where: dt.txn_id == ^txn_id and dt.device_nid == ^device_nid
            ) do
         %DeviceTransaction{event_id: event_id} ->
           {state_set, room, event_id}
 
         nil ->
-          with {state_set, room, %Event{event_id: event_id}} <-
+          with {state_set, room, %Event{id: event_id}} <-
                  insert_single_event(room, state_set, message_event).() do
             # Mark this transaction as done.
             Ecto.build_assoc(device, :device_transactions, txn_id: txn_id, event_id: event_id)
@@ -462,7 +462,7 @@ defmodule Architex.RoomServer do
     #       instead of the state_set state.
     #       Create custom type for this.
     serialized_state_set =
-      Enum.map(state_set, fn {{type, state_key}, %Event{event_id: event_id}} ->
+      Enum.map(state_set, fn {{type, state_key}, %Event{id: event_id}} ->
         [type, state_key, event_id]
       end)
 
@@ -538,7 +538,7 @@ defmodule Architex.RoomServer do
     state_set
     |> Map.take(state_pairs)
     |> Map.values()
-    |> Enum.map(fn %Event{event_id: event_id} -> event_id end)
+    |> Enum.map(fn %Event{id: event_id} -> event_id end)
   end
 
   # Get the auth events specific to m.room.member events.
diff --git a/lib/architex/schema/account.ex b/lib/architex/schema/account.ex
index 56d7c66..92f9790 100644
--- a/lib/architex/schema/account.ex
+++ b/lib/architex/schema/account.ex
@@ -71,7 +71,7 @@ defmodule Architex.Account do
 
       device_params = %{
         display_name: initial_device_display_name,
-        device_id: device_id
+        id: device_id
       }
 
       Ecto.build_assoc(account, :devices, access_token: access_token)
diff --git a/lib/architex/schema/device.ex b/lib/architex/schema/device.ex
index 62b8833..df45611 100644
--- a/lib/architex/schema/device.ex
+++ b/lib/architex/schema/device.ex
@@ -7,14 +7,16 @@ defmodule Architex.Device do
   alias ArchitexWeb.Client.Request.Login
 
   @type t :: %__MODULE__{
-          device_id: String.t(),
+          nid: integer(),
+          id: String.t(),
           access_token: String.t(),
           display_name: String.t(),
           account_id: integer()
         }
 
+  @primary_key {:nid, :id, autogenerate: true}
   schema "devices" do
-    field :device_id, :string
+    field :id, :string
     field :access_token, :string, redact: true
     field :display_name, :string
 
@@ -24,9 +26,9 @@ defmodule Architex.Device do
 
   def changeset(device, params \\ %{}) do
     device
-    |> cast(params, [:display_name, :device_id])
-    |> validate_required([:device_id])
-    |> unique_constraint([:device_id, :account_id], name: :devices_device_id_account_id_index)
+    |> cast(params, [:display_name, :id])
+    |> validate_required([:id])
+    |> unique_constraint([:id, :account_id], name: :devices_id_account_id_index)
   end
 
   def generate_access_token(localpart, device_id) do
@@ -47,7 +49,7 @@ defmodule Architex.Device do
 
     update_query =
       from(d in Device)
-      |> update(set: [access_token: ^access_token, device_id: ^device_id])
+      |> update(set: [access_token: ^access_token, id: ^device_id])
       |> then(fn q ->
         if initial_device_display_name do
           update(q, set: [display_name: ^initial_device_display_name])
@@ -57,13 +59,13 @@ defmodule Architex.Device do
       end)
 
     device_params = %{
-      device_id: device_id,
+      id: device_id,
       display_name: initial_device_display_name
     }
 
     Ecto.build_assoc(account, :devices)
     |> Device.changeset(device_params)
     |> put_change(:access_token, access_token)
-    |> Repo.insert(on_conflict: update_query, conflict_target: [:account_id, :device_id])
+    |> Repo.insert(on_conflict: update_query, conflict_target: [:account_id, :id])
   end
 end
diff --git a/lib/architex/schema/device_transaction.ex b/lib/architex/schema/device_transaction.ex
index 6b243b2..42d7de9 100644
--- a/lib/architex/schema/device_transaction.ex
+++ b/lib/architex/schema/device_transaction.ex
@@ -6,13 +6,13 @@ defmodule Architex.DeviceTransaction do
   @type t :: %__MODULE__{
           txn_id: String.t(),
           event_id: String.t(),
-          device_id: integer()
+          device_nid: integer()
         }
 
   @primary_key {:txn_id, :string, []}
   schema "device_transactions" do
     field :event_id, :string
 
-    belongs_to :device, Device
+    belongs_to :device, Device, references: :nid, foreign_key: :device_nid
   end
 end
diff --git a/lib/architex/schema/event.ex b/lib/architex/schema/event.ex
index 8bd8334..a77c8a6 100644
--- a/lib/architex/schema/event.ex
+++ b/lib/architex/schema/event.ex
@@ -20,7 +20,7 @@ defmodule Architex.Event do
           hashes: map() | nil
         }
 
-  @primary_key {:event_id, :string, []}
+  @primary_key {:nid, :id, autogenerate: true}
   schema "events" do
     field :type, :string
     field :origin_server_ts, :integer
@@ -32,6 +32,7 @@ defmodule Architex.Event do
     field :unsigned, :map
     field :signatures, {:map, {:map, :string}}
     field :hashes, {:map, :string}
+    field :id, :string
 
     belongs_to :room, Room, type: :string
   end
@@ -125,12 +126,12 @@ defmodule Architex.Event do
   def prevalidate(%Event{auth_events: auth_event_ids, prev_events: prev_event_ids} = event) do
     prev_events =
       Event
-      |> where([e], e.event_id in ^prev_event_ids)
+      |> where([e], e.id in ^prev_event_ids)
       |> Repo.all()
 
     auth_events =
       Event
-      |> where([e], e.event_id in ^auth_event_ids)
+      |> where([e], e.id in ^auth_event_ids)
       |> Repo.all()
 
     state_pairs = Enum.map(auth_events, &{&1.type, &1.state_key})
@@ -237,7 +238,7 @@ defmodule Architex.Event do
       event
       |> Architex.to_serializable_map()
       |> Map.take([
-        :event_id,
+        :id,
         :type,
         :room_id,
         :sender,
@@ -305,7 +306,7 @@ defmodule Architex.Event do
   @spec set_event_id(t()) :: {:ok, t()} | {:error, Jason.EncodeError.t()}
   def set_event_id(event) do
     with {:ok, event_id} <- generate_event_id(event) do
-      {:ok, %Event{event | event_id: event_id}}
+      {:ok, %Event{event | id: event_id}}
     end
   end
 
diff --git a/lib/architex/schema/room.ex b/lib/architex/schema/room.ex
index 5606f6c..5df8e4a 100644
--- a/lib/architex/schema/room.ex
+++ b/lib/architex/schema/room.ex
@@ -18,7 +18,7 @@ defmodule Architex.Room do
     field :visibility, Ecto.Enum, values: [:public, :private]
     field :state, {:array, {:array, :string}}
     field :forward_extremities, {:array, :string}
-    has_many :events, Event, foreign_key: :event_id
+    has_many :events, Event, foreign_key: :room_id
     has_many :aliases, Alias, foreign_key: :room_id
   end
 
@@ -39,7 +39,7 @@ defmodule Architex.Room do
 
   def update_forward_extremities(
         %Event{
-          event_id: event_id,
+          id: event_id,
           prev_events: prev_event_ids
         },
         %Room{id: room_id, forward_extremities: forward_extremities}
diff --git a/lib/architex/state_resolution.ex b/lib/architex/state_resolution.ex
index 90ba117..3e9cf95 100644
--- a/lib/architex/state_resolution.ex
+++ b/lib/architex/state_resolution.ex
@@ -31,7 +31,7 @@ defmodule Architex.StateResolution do
     room_events =
       Event
       |> where([e], e.room_id == ^room_id)
-      |> select([e], {e.event_id, e})
+      |> select([e], {e.id, e})
       |> Repo.all()
       |> Enum.into(%{})
 
@@ -61,14 +61,14 @@ defmodule Architex.StateResolution do
     room_events =
       Event
       |> where([e], e.room_id == ^room_id)
-      |> select([e], {e.event_id, e})
+      |> select([e], {e.id, e})
       |> Repo.all()
       |> Enum.into(%{})
 
     Event
     |> where([e], e.room_id == ^room_id)
     |> join(:inner, [e], r in Room, on: e.room_id == r.id)
-    |> where([e, r], e.event_id == fragment("ANY(?)", r.forward_extremities))
+    |> where([e, r], e.id == fragment("ANY(?)", r.forward_extremities))
     |> Repo.all()
     |> Enum.map(&resolve/1)
     |> do_resolve(room_events)
@@ -141,7 +141,7 @@ defmodule Architex.StateResolution do
       |> Enum.into(%{}, fn state_pair ->
         events =
           Enum.map(state_sets, fn
-            state_set when is_map_key(state_set, state_pair) -> state_set[state_pair].event_id
+            state_set when is_map_key(state_set, state_pair) -> state_set[state_pair].id
             _ -> nil
           end)
           |> MapSet.new()
@@ -185,7 +185,7 @@ defmodule Architex.StateResolution do
   defp auth_chain(%Event{auth_events: auth_events}, room_events) do
     auth_events
     |> Enum.map(&room_events[&1])
-    |> Enum.reduce(MapSet.new(), fn %Event{event_id: auth_event_id} = auth_event, acc ->
+    |> Enum.reduce(MapSet.new(), fn %Event{id: auth_event_id} = auth_event, acc ->
       auth_event
       |> auth_chain(room_events)
       |> MapSet.union(acc)
@@ -194,8 +194,8 @@ defmodule Architex.StateResolution do
   end
 
   defp rev_top_pow_order(room_events) do
-    fn %Event{origin_server_ts: timestamp1, event_id: event_id1} = event1,
-       %Event{origin_server_ts: timestamp2, event_id: event_id2} = event2 ->
+    fn %Event{origin_server_ts: timestamp1, id: event_id1} = event1,
+       %Event{origin_server_ts: timestamp2, id: event_id2} = event2 ->
       power1 = get_power_level(event1, room_events)
       power2 = get_power_level(event2, room_events)
 
diff --git a/lib/architex/state_resolution/authorization.ex b/lib/architex/state_resolution/authorization.ex
index 791b5c2..ac872ca 100644
--- a/lib/architex/state_resolution/authorization.ex
+++ b/lib/architex/state_resolution/authorization.ex
@@ -23,7 +23,7 @@ defmodule Architex.StateResolution.Authorization do
   def authorized?(
         %Event{type: "m.room.member", state_key: state_key, prev_events: [create_id]},
         %{
-          {"m.room.create", ""} => %Event{event_id: create_id, content: %{"creator" => creator}}
+          {"m.room.create", ""} => %Event{id: create_id, content: %{"creator" => creator}}
         }
       ),
       do: state_key == creator
@@ -306,7 +306,7 @@ defmodule Architex.StateResolution.Authorization do
     # We assume the auth events are validated beforehand.
     state_set =
       Event
-      |> where([e], e.event_id in ^auth_event_ids)
+      |> where([e], e.id in ^auth_event_ids)
       |> Repo.all()
       |> Enum.reduce(%{}, &update_state_set/2)
 
diff --git a/lib/architex_web/client/controllers/login_controller.ex b/lib/architex_web/client/controllers/login_controller.ex
index 578c324..853bdc4 100644
--- a/lib/architex_web/client/controllers/login_controller.ex
+++ b/lib/architex_web/client/controllers/login_controller.ex
@@ -40,7 +40,7 @@ defmodule ArchitexWeb.Client.LoginController do
         case Account.login(input) |> Repo.transaction() do
           {:ok,
            {%Account{localpart: localpart},
-            %Device{access_token: access_token, device_id: device_id}}} ->
+            %Device{access_token: access_token, id: device_id}}} ->
             data = %{
               user_id: Architex.get_mxid(localpart),
               access_token: access_token,
diff --git a/lib/architex_web/client/controllers/register_controller.ex b/lib/architex_web/client/controllers/register_controller.ex
index e935f17..0b24d7b 100644
--- a/lib/architex_web/client/controllers/register_controller.ex
+++ b/lib/architex_web/client/controllers/register_controller.ex
@@ -24,7 +24,7 @@ defmodule ArchitexWeb.Client.RegisterController do
           {:ok,
            %{
              account: %Account{localpart: localpart},
-             device: %Device{device_id: device_id, access_token: access_token}
+             device: %Device{id: device_id, access_token: access_token}
            }} ->
             data = %{user_id: Architex.get_mxid(localpart)}
 
diff --git a/lib/architex_web/client/controllers/room_controller.ex b/lib/architex_web/client/controllers/room_controller.ex
index 63db0d4..fdf895b 100644
--- a/lib/architex_web/client/controllers/room_controller.ex
+++ b/lib/architex_web/client/controllers/room_controller.ex
@@ -232,4 +232,13 @@ defmodule ArchitexWeb.Client.RoomController do
         put_error(conn, :not_found, "The given room was not found.")
     end
   end
+
+  # GET /_matrix/client/r0/rooms/!atYDsyowueiToUvuqY:localhost:4000/messages
+  # Parameters: %{"dir" => "b", "from" => "", "limit" => "727", "path" => ["_matrix", "client", "r0", "rooms", "!atYDsyowueiToUvuqY:localhost:4000", "messages"]}
+  def message(conn, params) do
+
+    conn
+    |> send_resp(400, [])
+    |> halt()
+  end
 end
diff --git a/lib/architex_web/federation/controllers/event_controller.ex b/lib/architex_web/federation/controllers/event_controller.ex
index e9562b7..6c048ed 100644
--- a/lib/architex_web/federation/controllers/event_controller.ex
+++ b/lib/architex_web/federation/controllers/event_controller.ex
@@ -16,7 +16,7 @@ defmodule ArchitexWeb.Federation.EventController do
   def event(%Plug.Conn{assigns: %{origin: origin}} = conn, %{"event_id" => event_id}) do
     query =
       Event
-      |> where([e], e.event_id == ^event_id)
+      |> where([e], e.id == ^event_id)
       |> preload(:room)
 
     case Repo.one(query) do
@@ -82,7 +82,7 @@ defmodule ArchitexWeb.Federation.EventController do
   defp get_state_or_state_ids(conn, state_or_state_ids, origin, event_id, room_id) do
     query =
       Event
-      |> where([e], e.event_id == ^event_id and e.room_id == ^room_id)
+      |> where([e], e.id == ^event_id and e.room_id == ^room_id)
       |> preload(:room)
 
     case Repo.one(query) do
diff --git a/lib/architex_web/router.ex b/lib/architex_web/router.ex
index 930fd85..b0f3f82 100644
--- a/lib/architex_web/router.ex
+++ b/lib/architex_web/router.ex
@@ -67,6 +67,7 @@ defmodule ArchitexWeb.Router do
         post "/ban", RoomController, :ban
         post "/unban", RoomController, :unban
         put "/send/:event_type/:txn_id", RoomController, :send_message
+        get "/messages", RoomController, :messages
       end
     end
   end
diff --git a/priv/repo/migrations/20210830160818_create_initial_tables.exs b/priv/repo/migrations/20210830160818_create_initial_tables.exs
index 4670b27..f285edc 100644
--- a/priv/repo/migrations/20210830160818_create_initial_tables.exs
+++ b/priv/repo/migrations/20210830160818_create_initial_tables.exs
@@ -11,9 +11,9 @@ defmodule Architex.Repo.Migrations.CreateInitialTables do
     create index(:accounts, [:localpart], unique: true)
 
     create table(:rooms, primary_key: false) do
+      add :id, :string, primary_key: true, null: false
       add :state, {:array, {:array, :string}}, default: [], null: false
       add :forward_extremities, {:array, :string}, default: [], null: false
-      add :id, :string, primary_key: true, null: false
       add :visibility, :string, null: false, default: "public"
     end
 
@@ -26,11 +26,13 @@ defmodule Architex.Repo.Migrations.CreateInitialTables do
     end
 
     create table(:events, primary_key: false) do
+      add :nid, :serial, primary_key: true
+
       add :origin_server_ts, :bigint, null: false
       add :unsigned, :map, default: %{}, null: true
       add :hashes, :map, null: false
       add :signatures, :map, null: false
-      add :event_id, :string, null: false
+      add :id, :string, null: false
       add :content, :map
       add :type, :string, null: false
       add :state_key, :string
@@ -40,6 +42,8 @@ defmodule Architex.Repo.Migrations.CreateInitialTables do
       add :room_id, references(:rooms, type: :string), null: false
     end
 
+    create index(:events, [:id], unique: true)
+
     create table(:server_key_info, primary_key: false) do
       add :valid_until, :bigint, default: 0, null: false
       add :server_name, :string, primary_key: true, null: false
@@ -61,21 +65,22 @@ defmodule Architex.Repo.Migrations.CreateInitialTables do
 
     create index(:aliases, [:room_id])
 
-    create table(:devices) do
-      add :device_id, :string, null: false
+    create table(:devices, primary_key: false) do
+      add :nid, :serial, primary_key: true
+      add :id, :string, null: false
       add :access_token, :string, null: false
       add :display_name, :string
 
       add :account_id, references(:accounts, on_delete: :delete_all), null: false
     end
 
-    create index(:devices, [:device_id, :account_id], unique: true)
+    create index(:devices, [:id, :account_id], unique: true)
     create index(:devices, [:account_id])
     create index(:devices, [:access_token], unique: true)
 
     create table(:device_transactions, primary_key: false) do
       add :txn_id, :string, primary_key: true, null: false
-      add :device_id, references(:devices, on_delete: :delete_all), primary_key: true, null: false
+      add :device_nid, references(:devices, column: :nid, on_delete: :delete_all), primary_key: true
       add :event_id, :string, null: false
     end
   end
diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs
index 11e5706..c6aedd4 100644
--- a/priv/repo/seeds.exs
+++ b/priv/repo/seeds.exs
@@ -8,7 +8,7 @@ account =
 
 account
 |> Ecto.build_assoc(:devices,
-  device_id: "android",
+  id: "android",
   display_name: "My Android",
   access_token: "sneed"
 )
diff --git a/test/support/factory.ex b/test/support/factory.ex
index 23e8cdf..f85b0f8 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -15,7 +15,7 @@ defmodule Architex.Factory do
     device_id = sequence(:device_id, &"device#{&1}")
 
     %Device{
-      device_id: device_id,
+      id: device_id,
       access_token: Device.generate_access_token(localpart, device_id),
       display_name: sequence(:display_name, &"Device #{&1}"),
       account: account