Schema Validations

[hamiltop]

There are currently few or no schema validations. What sort of validations do we want to have?

Should we limit fields to those listed in the schema? Should we validate that each field has the correct type?

[thiagoc7]

you provide the raw driver, so I think that this repo should follow Ecto conventions.

• limit fields to schema
• validate required fields
• validate field types
• friendly database errors.

And there is the uniqueness problem. As far as I know, you can't implement unique index in rethinkdb. We could abstract unique_constraint inside this repo, and make a query to validate it (like active record).

[hamiltop]

Limit fields to schema is already done by the Struct. Enforce required and optional is already done by the Changeset. Enforce types is missing. Uniqueness I don't want to support. We don't have transactions so at best we have a race condition.

So enforce types it is. I have a rough idea of how to do it.

[thiagoc7]

if enforce required is done, there is something worng with TestRepo setup. Title is a required field. date tests don´t provide it, and still pass.

Do you need help for enforce types?

[hamiltop]

The date tests aren't using a changeset. A Changeset should enforce required already.

[hamiltop]

To be more precise, required fields are enforced explicitly by calling cast with required fields.

[hamiltop]

I failed to address the "friendly database errors" in your first comment. Those are definitely needed. One part of that is the bang methods for CRUD. If you wanted to take on another task, that would be a great one (though opening another issue for those is probably a good idea).

[hamiltop]

So... It looks like the schema types are also enforced by Ecto already:

c = Ecto.Changeset.cast(%PhxControl.Post{}, %{title: "peter"}, [:title], [])
%Ecto.Changeset{action: nil, changes: %{title: "peter"}, constraints: [],
 errors: [], filters: %{},
 model: %PhxControl.Post{__meta__: #Ecto.Schema.Metadata<:built>, body: nil,
  id: nil, inserted_at: nil, score: nil, title: nil, updated_at: nil},
 optional: [], opts: [], params: %{"title" => "peter"}, prepare: [], repo: nil,
 required: [:title],
 types: %{body: :string, id: :id, inserted_at: Ecto.DateTime, score: :integer,
   title: :string, updated_at: Ecto.DateTime}, valid?: true, validations: []}
iex(32)> c.errors
[]
iex(33)> c = Ecto.Changeset.cast(%PhxControl.Post{}, %{title: 0}, [:title], [])
%Ecto.Changeset{action: nil, changes: %{}, constraints: [],
 errors: [title: "is invalid"], filters: %{},
 model: %PhxControl.Post{__meta__: #Ecto.Schema.Metadata<:built>, body: nil,
  id: nil, inserted_at: nil, score: nil, title: nil, updated_at: nil},
 optional: [], opts: [], params: %{"title" => 0}, prepare: [], repo: nil,
 required: [:title],
 types: %{body: :string, id: :id, inserted_at: Ecto.DateTime, score: :integer,
   title: :string, updated_at: Ecto.DateTime}, valid?: false, validations: []}
iex(34)> c.errors
[title: "is invalid"]

[thiagoc7]

Hi @hamiltop, I created some tests for this. The only thing that does not work is enforcing schema declaration, like strong params. It seems that you can insert any attr you want on db.

For the bang methods, I just need to return the model / changeset, instead of a tuple?

[hamiltop]

@thiagoc7 strong params is encapsulated in the required + optional params of cast. If it's not in either of those, it won't be in the final model written to the db.

The bang methods return return the model, but raise an error if it fails. We should match the errors that ecto normally raises.