Skip to content

Make uuid encoding configurable and consistent #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes will be documented in this file.
The format is based on [Keep a Changelog][keepachangelog], and this project
adheres to [Semantic Versioning][semver].

## [Unreleased]
### Changed
- UUID encoding for both `:binary_id` and `:uuid`/`Ecto.UUID` is now configurable
- `:uuid`/`Ecto.UUID` is now encoded as a string by default


## [0.6.0] - 2021-08-25
### Changed
- `:utc_datetime` handling has been updated to completely remove the `Z` supplied and
Expand Down
32 changes: 32 additions & 0 deletions lib/ecto/adapters/sqlite3.ex
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,22 @@ defmodule Ecto.Adapters.SQLite3 do
[&Codec.decimal_decode/1, type]
end

@impl Ecto.Adapter
def loaders(:binary_id, type) do
case Application.get_env(:ecto_sqlite3, :binary_id_type, :string) do
:string -> [type]
:binary -> [Ecto.UUID, type]
end
end

@impl Ecto.Adapter
def loaders(:uuid, type) do
case Application.get_env(:ecto_sqlite3, :uuid_type, :string) do
:string -> []
:binary -> [type]
end
end

# when we have an e.g., max(created_date) function
# Ecto does not truly know the return type, hence :maybe
# see Ecto.Query.Planner.collect_fields
Expand Down Expand Up @@ -316,6 +332,22 @@ defmodule Ecto.Adapters.SQLite3 do
[type, &Codec.decimal_encode/1]
end

@impl Ecto.Adapter
def dumpers(:binary_id, type) do
case Application.get_env(:ecto_sqlite3, :binary_id_type, :string) do
:string -> [type]
:binary -> [type, Ecto.UUID]
end
end

@impl Ecto.Adapter
def dumpers(:uuid, type) do
case Application.get_env(:ecto_sqlite3, :uuid_type, :string) do
:string -> []
:binary -> [type]
end
end

@impl Ecto.Adapter
def dumpers(:time, type) do
[type, &Codec.time_encode/1]
Expand Down
19 changes: 14 additions & 5 deletions lib/ecto/adapters/sqlite3/data_type.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,9 @@ defmodule Ecto.Adapters.SQLite3.DataType do
def column_type(:serial, _opts), do: "INTEGER"
def column_type(:bigserial, _opts), do: "INTEGER"
def column_type(:bigint, _opts), do: "INTEGER"
# TODO: We should make this configurable
def column_type(:binary_id, _opts), do: "TEXT"
def column_type(:string, _opts), do: "TEXT"
def column_type(:float, _opts), do: "NUMERIC"
def column_type(:binary, _opts), do: "BLOB"
# TODO: We should make this configurable
# SQLite3 does not support uuid
def column_type(:uuid, _opts), do: "TEXT"
def column_type(:map, _opts), do: "JSON"
def column_type(:array, _opts), do: "JSON"
def column_type({:map, _}, _opts), do: "JSON"
Expand All @@ -42,6 +37,20 @@ defmodule Ecto.Adapters.SQLite3.DataType do
end
end

def column_type(:binary_id, _opts) do
case Application.get_env(:ecto_sqlite3, :binary_id_type, :string) do
:string -> "TEXT_UUID"
:binary -> "UUID"
end
end

def column_type(:uuid, _opts) do
case Application.get_env(:ecto_sqlite3, :uuid_type, :string) do
:string -> "TEXT_UUID"
:binary -> "UUID"
end
end

def column_type(type, _) do
type
|> Atom.to_string()
Expand Down
4 changes: 1 addition & 3 deletions test/ecto/adapters/sqlite3/connection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -947,15 +947,13 @@ defmodule Ecto.Adapters.SQLite3.ConnectionTest do
assert all(query) == ~s{SELECT 1 FROM "schema" AS s0 WHERE (s0."foo" = (0 + 123.0))}
end

# TODO: We need to determine what format to store the UUID. Is it Text or binary 16?
# Are we going for readability or for compactness?
test "tagged type" do
query =
Schema
|> select([], type(^"601d74e4-a8d3-4b6e-8365-eddb4c893327", Ecto.UUID))
|> plan()

assert all(query) == ~s{SELECT CAST(? AS TEXT) FROM "schema" AS s0}
assert all(query) == ~s{SELECT CAST(? AS TEXT_UUID) FROM "schema" AS s0}
end

test "string type" do
Expand Down
28 changes: 23 additions & 5 deletions test/ecto/adapters/sqlite3/data_type_test.exs
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
defmodule Ecto.Adapters.SQLite3.DataTypeTest do
use ExUnit.Case, async: true
use ExUnit.Case, async: false

alias Ecto.Adapters.SQLite3.DataType

setup do
Application.put_env(:ecto_sqlite3, :binary_id_type, :string)
Application.put_env(:ecto_sqlite3, :uuid_type, :string)

on_exit(fn ->
Application.put_env(:ecto_sqlite3, :binary_id_type, :string)
Application.put_env(:ecto_sqlite3, :uuid_type, :string)
end)
end

describe ".column_type/2" do
test ":id is INTEGER" do
assert DataType.column_type(:id, nil) == "INTEGER"
Expand All @@ -16,16 +26,24 @@ defmodule Ecto.Adapters.SQLite3.DataTypeTest do
assert DataType.column_type(:bigserial, nil) == "INTEGER"
end

test ":binary_id is TEXT" do
assert DataType.column_type(:binary_id, nil) == "TEXT"
test ":binary_id is TEXT_UUID OR UUID" do
assert DataType.column_type(:binary_id, nil) == "TEXT_UUID"

Application.put_env(:ecto_sqlite3, :binary_id_type, :binary)

assert DataType.column_type(:binary_id, nil) == "UUID"
end

test ":string is TEXT" do
assert DataType.column_type(:string, nil) == "TEXT"
end

test ":uuid is TEXT" do
assert DataType.column_type(:uuid, nil) == "TEXT"
test ":uuid is TEXT_UUID or UUID" do
assert DataType.column_type(:uuid, nil) == "TEXT_UUID"

Application.put_env(:ecto_sqlite3, :uuid_type, :binary)

assert DataType.column_type(:uuid, nil) == "UUID"
end

test ":map is JSON" do
Expand Down
23 changes: 21 additions & 2 deletions test/ecto/integration/uuid_test.exs
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
defmodule Ecto.Integration.UUIDTest do
use Ecto.Integration.Case
use Ecto.Integration.Case, async: false

alias Ecto.Integration.TestRepo
alias EctoSQLite3.Integration.Product

test "handles uuid serialization and deserialization" do
setup do
Application.put_env(:ecto_sqlite3, :uuid_type, :string)
on_exit(fn -> Application.put_env(:ecto_sqlite3, :uuid_type, :string) end)
end

test "handles uuid serialization and deserialization with string format " do
external_id = Ecto.UUID.generate()
product = TestRepo.insert!(%Product{name: "Pupper Beer", external_id: external_id})

assert product.id
assert product.external_id == external_id

found = TestRepo.get(Product, product.id)
assert found
assert found.external_id == external_id
end

test "handles uuid serialization and deserialization with binary format " do
Application.put_env(:ecto_sqlite3, :uuid_type, :binary)

external_id = Ecto.UUID.generate()
product = TestRepo.insert!(%Product{name: "Pupper Beer", external_id: external_id})

Expand Down