| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
| |
This removes `|| id` which were added in #9963 and #23887 since it is no
longer necessary.
|
|
|
| |
using serializable isolation would prevent the duplicate insert as done in the example from happening
|
| |
|
| |
|
|\
| |
| | |
Building `where_clause` in `UniquenessValidator` is no longer needed
|
| |
| |
| |
| |
| |
| | |
Building `where_clause` manually was introduced at #26073 to include
both `comparison` and `binds` in `where_clause`. Since 213796f,
`comparison` includes `binds`, so it is enough to use `where` simply.
|
|/
|
|
| |
For less duplicated code.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A common source of bugs and code bloat within Active Record has been the
need for us to maintain the list of bind values separately from the AST
they're associated with. This makes any sort of AST manipulation
incredibly difficult, as any time we want to potentially insert or
remove an AST node, we need to traverse the entire tree to find where
the associated bind parameters are.
With this change, the bind parameters now live on the AST directly.
Active Record does not need to know or care about them until the final
AST traversal for SQL construction. Rather than returning just the SQL,
the Arel collector will now return both the SQL and the bind parameters.
At this point the connection adapter will have all the values that it
had before.
A bit of this code is janky and something I'd like to refactor later. In
particular, I don't like how we're handling associations in the
predicate builder, the special casing of `StatementCache::Substitute` in
`QueryAttribute`, or generally how we're handling bind value replacement
in the statement cache when prepared statements are disabled.
This also mostly reverts #26378, as it moved all the code into a
location that I wanted to delete.
/cc @metaskills @yahonda, this change will affect the adapters
Fixes #29766.
Fixes #29804.
Fixes #26541.
Close #28539.
Close #24769.
Close #26468.
Close #26202.
There are probably other issues/PRs that can be closed because of this
commit, but that's all I could find on the first few pages.
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|\
| |
| |
| |
| | |
kamipo/decouple_building_arel_ast_for_uniqueness_validator
Decouple the building Arel ASTs for uniqueness validator
|
| |
| |
| |
| |
| |
| | |
Currently uniqueness validator is coupled with building Arel ASTs.
This commit extracts `WhereClauseFactory#build_for_case_sensitive` for
decouple the building Arel ASTs.
|
|/
|
|
|
|
| |
Follow up to 5b14129d8d4ad302b4e11df6bd5c7891b75f393c.
http://edgeapi.rubyonrails.org/classes/ActiveRecord/Attribute.html
|
| |
|
|
|
|
| |
https://gist.github.com/sergey-alekseev/946657ebdb5e58d1bee115714056ec96
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
callbacks
We pretty frequently get bug reports that "dirty is broken inside of
after callbacks". Intuitively they are correct. You'd expect
`Model.after_save { puts changed? }; model.save` to do the same thing as
`model.save; puts model.changed?`, but it does not.
However, changing this goes much farther than just making the behavior
more intuitive. There are a _ton_ of places inside of AR that can be
drastically simplified with this change. Specifically, autosave
associations, timestamps, touch, counter cache, and just about anything
else in AR that works with callbacks have code to try to avoid "double
save" bugs which we will be able to flat out remove with this change.
We introduce two new sets of methods, both with names that are meant to
be more explicit than dirty. The first set maintains the old behavior,
and their names are meant to center that they are about changes that
occurred during the save that just happened. They are equivalent to
`previous_changes` when called outside of after callbacks, or once the
deprecation cycle moves.
The second set is the new behavior. Their names imply that they are
talking about changes from the database representation. The fact that
this is what we really care about became clear when looking at
`BelongsTo.touch_record` when tests were failing. I'm unsure that this
set of methods should be in the public API. Outside of after callbacks,
they are equivalent to the existing methods on dirty.
Dirty itself is not deprecated, nor are the methods inside of it. They
will only emit the warning when called inside of after callbacks. The
scope of this breakage is pretty large, but the migration path is
simple. Given how much this can improve our codebase, and considering
that it makes our API more intuitive, I think it's worth doing.
|
|
|
|
|
|
|
|
|
| |
This reverts commit 3a1f6fe7b4a70bf0698b0684dd48ac712c6883b6.
This commit takes the code in a direction that I am looking to avoid.
The predicate builder should be purely concerned with AST construction
as it matters to methods like `where`. Things like case sensitivity
should continue to be handled elsewhere.
|
|
|
|
|
|
| |
Currently uniqueness validator is coupled with building Arel ASTs.
This commit extracts `PredicateBuilder::CaseSensitiveHandler` for
decouple the building Arel ASTs.
|
|
|
|
|
|
|
|
|
|
|
|
| |
Passing arel node with splat binds for `where` was introduced at #22877
for uniqueness validator supports prepared statement. But I'd not like
to introduce the following usage:
```ruby
Foo.where(arel, *binds)
```
I'd like to revert this internal usage.
|
|
|
|
|
| |
A `value` is only used for checking `value.nil?`. It is unnecessary if
immediately return when `value.nil?`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`FinderMethods#exists?` should return a boolean rather than raising an
exception.
`UniquenessValidator#build_relation` catches a `RangeError` because it
includes type casting due to a string value truncation. But a string
value truncation was removed at #23523 then type casting in
`build_relation` is no longer necessary. aa06231 removes type casting in
`build_relation` then a `RangeError` moves to `relation.exists?`.
This change will remove the catching a `RangeError`.
|
|
|
|
|
| |
Type casting in uniqueness validator is for a string value truncation.
It was removed at #23523.
|
|\
| |
| |
| | |
Avoid a string value truncation in uniqueness validation
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In MySQL, PostgreSQL, Oracle and SQLServer, a value over the limit
cannot be inserted or updated (See #23522).
In SQLite3, a value is inserted or updated regardless of the limit.
We should avoid a string value truncation in uniqueness validation.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
record.id_was is nil in after_create/after_save, so we should use
id in these cases.
While this logic feels incomplete, the existing update_record uses the same
logic:
https://github.com/rails/rails/blob/2fda4e0874a97a76107ab9e88305169f2c625933/activerecord/lib/active_record/relation.rb#L83
This logic was originally added for a similar problem:
updates not working with after_create hook.
See: 482f8c15b1d699c95bfbc3d836f674a09c0d9031
Followup to #23581
Fixes #23844
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In order to fix issue #17621 we added a check to validations that
determined if a record should be validated. Based on the existing tests
and behavior we wanted we determined the best way to do that was by
checking if `!record.peristed? || record.changed? || record.marked_for_destruction?`
This change didn't make it into a release until now. When #23790 was
opened we realized that `valid?` and `invalid?` were broken and did not
work on persisted records because of the `!record.persisted?`.
While there is still a bug that #17621 brought up, this change was too
drastic and should not be a RC blocker. I will work on fixing this so
that we don't break `valid?` but also aren't validating parent records
through child records if that parent record is validate false. This
change removes the code changes to validate and the corresponding tests.
It adds tests for two of the bugs found since Rails 5 beta2 release.
Fixes #17621
|
|\ \
| | |
| | | |
Fix issue #23625
|
| |/
| |
| |
| |
| |
| | |
This resolves a bug where if the primary key used is not `id` (ex:
`uuid`), and has a `validates_uniqueness_of` in the model, a uniqueness error
would be raised. This is a partial revert of commit `119b9181ece399c67213543fb5227b82688b536f`, which introduced this behavior.
|
|/ |
|
|
|
|
|
|
|
| |
When changing the PK for a record which has a uniqueness validation on
some other attribute, Active Record should exclude itself from the
validation based on the PK value stored on the DB (id_was) instead of
its new value (id).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
```
SELECT 1 AS one FROM "topics" WHERE "topics"."title" = 'abc' LIMIT $1 [["LIMIT", 1]]
```
After:
```
SELECT 1 AS one FROM "topics" WHERE "topics"."title" = $1 LIMIT $2 [["title", "abc"], ["LIMIT", 1]]
```
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The focus of this change is to make the API more accessible.
References to method and classes should be linked to make it easy to
navigate around.
This patch makes exzessiv use of `rdoc-ref:` to provide more readable
docs. This makes it possible to document `ActiveRecord::Base#save` even
though the method is within a separate module
`ActiveRecord::Persistence`. The goal here is to bring the API closer to
the actual code that you would write.
This commit only deals with Active Record. The other gems will be
updated accordingly but in different commits. The pass through Active
Record is not completely finished yet. A follow up commit will change
the spots I haven't yet had the time to update.
/cc @fxn
|
|
|
|
|
|
|
|
| |
Closes #21304.
While we can validate uniqueness for record without primary key on
creation, there is no way to exclude the current record when
updating. (The update itself will need a primary key to work correctly).
|
|
|
|
|
|
| |
This is an alternate implementation of #20966.
[Sean Griffin & presskey]
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This predicate was only to figure out if it's safe to do case
insensitive comparison, which is only a problem on PG. Turns out, PG can
just tell us whether we are able to do it or not. If the query turns out
to be a problem, let's just replace that method with checking the SQL
type for `text` or `character`. I'd rather not burden the type objects
with adapter specific knowledge.
The *real* solution, is to deprecate this behavior entirely. The only
reason we need it is because the `:case_sensitive` option for
`validates_uniqueness_of` is documented as "this option is ignored for
non-strings". It makes no sense for us to do that. If the type can't be
compared in a case insensitive way, the user shouldn't tell us to do
case insensitive comparison.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes #17621. This 5 year old (or older) issue causes validations to fire
when a parent record has `validate: false` option and a child record is
saved. It's not the responsibility of the model to validate an
associated object unless the object was created or modified by the
parent.
Clean up tests related to validations
`assert_nothing_raised` is not benefiting us in these tests
Corrected spelling of "respects"
It's better to use `assert_not_operator` over `assert !r.valid`
|
|
|
|
|
|
| |
Remaining are `limit`, `precision`, `scale`, and `type` (the symbol
version). These will remain on the column, since they mirror the options
to the `column` method in the schema definition DSL
|
|
|
|
| |
The validator has access to richer type information
|
|
|
|
|
|
|
|
|
|
| |
This is no longer required now that we are injecting a type caster
object into the Arel table, with the exception of uniqueness
validations. Since it calls `ConnectionAdapter#type_cast`, the value has
already been cast for the database. We don't want Arel to attempt to
cast it further, so we need to continue wrapping it in a quoted node.
This can potentially go away when this validator is refactored to make
better use of `where` or the predicate builder.
|
|
|
|
|
|
|
| |
This code could use some much heavier refactoring. It looks like
`build_relation` duplicates most of the logic of `Relation#where` and
`PredicateBuilder` with regards to handling associations and attribute
aliases
|
|
|
|
|
|
|
| |
Part of the larger refactoring to remove type casting from Arel. Since
we've already cast the value a few lines above, we don't need to re-cast
it later. We can inform Arel of this by wrapping it in an
`Arel::Nodes::Quoted`, which will no longer be required in Rails 5.1
|
|
|
|
|
|
|
|
| |
I think we should deprecate this behavior and just error if you tell us
to do a case insensitive comparison for types which are not case
sensitive. Partially reverts 35592307
Fixes #18195
|
|
|
|
|
|
|
| |
We added a comparison to "id", and call to `self.class.primary_key` a
*lot*. We also have performance hits from `&block` all over the place.
We skip the check in a new method, in order to avoid breaking the
behavior of `read_attribute`
|
| |
|
|
|
|
|
|
|
| |
This was only used for uniqueness validations. The first usage was in
conjunction with `limit`. Types which cast to string, but are not
considered text cannot have a limit. The second case was only with an
explicit `:case_sensitive => true` option given by the user.
|