Fix event name validation (#2975)

* Fix event name validation

* Add @moduledoc

* Move tests to Ingestion.RequestTest

* Remove comment

lib/plausible/ingestion/request.ex

@@ -1,3 +1,20 @@
+defmodule Plausible.Ecto.EventName do
+  @moduledoc """
+    Custom type for event name. Accepts Strings and Integers and stores them as String. Returns
+    cast error if any other type is provided. Accepting integers is important for 404 tracking.
+  """
+
+  use Ecto.Type
+  def type, do: :string
+
+  def cast(val) when is_binary(val), do: {:ok, val}
+  def cast(val) when is_integer(val), do: {:ok, Integer.to_string(val)}
+
+  def cast(_), do: :error
+  def load(val), do: {:ok, val}
+  def dump(val), do: {:ok, val}
+end
+
 defmodule Plausible.Ingestion.Request do
   @moduledoc """
   The %Plausible.Ingestion.Request{} struct stores all needed fields
@@ -13,12 +30,12 @@ defmodule Plausible.Ingestion.Request do
   embedded_schema do
     field :remote_ip, :string
     field :user_agent, :string
-    field :event_name, :string
+    field :event_name, Plausible.Ecto.EventName
     field :uri, :map
     field :hostname, :string
     field :referrer, :string
     field :domains, {:array, :string}
-    field :hash_mode, :string
+    field :hash_mode, :integer
     field :pathname, :string
     field :props, :map
     field :query_params, :map
@@ -88,10 +105,13 @@ defmodule Plausible.Ingestion.Request do
   end
 
   defp put_request_params(changeset, %{} = request_body) do
-    Changeset.change(
+    Changeset.cast(
       changeset,
-      event_name: request_body["n"] || request_body["name"],
-      hash_mode: request_body["h"] || request_body["hashMode"]
+      %{
+        event_name: request_body["n"] || request_body["name"],
+        hash_mode: request_body["h"] || request_body["hashMode"]
+      },
+      [:event_name, :hash_mode]
     )
   end
 

test/plausible/ingestion/request_test.exs

@@ -100,6 +100,18 @@ defmodule Plausible.Ingestion.RequestTest do
     assert request.uri.host == "dummy.site"
   end
 
+  test "request can be built with numeric event name" do
+    payload = %{
+      n: 404,
+      d: "dummy.site",
+      u: "http://dummy.site/index"
+    }
+
+    conn = build_conn(:post, "/api/events", payload)
+    assert {:ok, request} = Request.build(conn)
+    assert request.event_name == "404"
+  end
+
   test "hostname is (none) if host-less uri provided" do
     payload = %{
       name: "pageview",
@@ -237,6 +249,30 @@ defmodule Plausible.Ingestion.RequestTest do
     assert {"should be at most %{count} character(s)", _} = changeset.errors[:event_name]
   end
 
+  test "returns validation error when event name is blank" do
+    payload = %{
+      name: nil,
+      domain: "dummy.site",
+      url: "https://dummy.site/"
+    }
+
+    conn = build_conn(:post, "/api/events", payload)
+    assert {:error, changeset} = Request.build(conn)
+    assert {"can't be blank", _} = changeset.errors[:event_name]
+  end
+
+  test "returns validation error when event name cannot be cast to string" do
+    payload = %{
+      name: ["list", "of", "things"],
+      domain: "dummy.site",
+      url: "https://dummy.site/"
+    }
+
+    conn = build_conn(:post, "/api/events", payload)
+    assert {:error, changeset} = Request.build(conn)
+    assert {"is invalid", _} = changeset.errors[:event_name]
+  end
+
   test "truncates referrer when too long" do
     payload = %{
       name: "pageview",