aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record/validations
Commit message (Collapse)AuthorAgeFilesLines
* Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-131-1/+0
| | | | | | | | | | | We sometimes say "✂️ newline after `private`" in a code review (e.g. https://github.com/rails/rails/pull/18546#discussion_r23188776, https://github.com/rails/rails/pull/34832#discussion_r244847195). Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style `EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059). That cop and enforced style will reduce the our code review cost.
* Merge pull request #35503 from samjohn/cannot-grammar-correctionXavier Noria2019-03-071-1/+1
|\ | | | | Replace “can not” with “cannot”.
| * Replace “can not” with “cannot”.Samantha John2019-03-061-1/+1
| |
* | Move all Arel constructions from uniqueness validator into connection adapterRyuta Kamizono2019-03-071-3/+1
|/
* Deprecate mismatched collation comparison for uniquness validatorRyuta Kamizono2019-03-041-1/+1
| | | | | | | | | | | | | | | | | | | | | In MySQL, the default collation is case insensitive. Since the uniqueness validator enforces case sensitive comparison by default, it frequently causes mismatched collation issues (performance, weird behavior, etc) to MySQL users. https://grosser.it/2009/12/11/validates_uniqness_of-mysql-slow/ https://github.com/rails/rails/issues/1399 https://github.com/rails/rails/pull/13465 https://github.com/gitlabhq/gitlabhq/commit/c1dddf8c7d947691729f6d64a8ea768b5c915855 https://github.com/huginn/huginn/pull/1330#discussion_r55152573 I'd like to deprecate the implicit default enforcing since I frequently experienced the problems in code reviews. Note that this change has no effect to sqlite3, postgresql, and oracle-enhanced adapters which are implemented as case sensitive by default, only affect to mysql2 adapter (I can take a work if sqlserver adapter will support Rails 6.0).
* Extract `default_uniqueness_comparison` to ease to handle mismatched ↵Ryuta Kamizono2019-02-211-1/+3
| | | | | | | | | | | | | | | | | | collation issues In MySQL, the default collation is case insensitive. Since the uniqueness validator enforces case sensitive comparison by default, it frequently causes mismatched collation issues (performance, weird behavior, etc) to MySQL users. https://grosser.it/2009/12/11/validates_uniqness_of-mysql-slow/ https://github.com/rails/rails/issues/1399 https://github.com/rails/rails/pull/13465 https://github.com/gitlabhq/gitlabhq/commit/c1dddf8c7d947691729f6d64a8ea768b5c915855 https://github.com/huginn/huginn/pull/1330#discussion_r55152573 This extracts `default_uniqueness_comparison` to ease to handle the mismatched collation issues on the connection.
* Use `unboundable?` rather than `boundable?`Ryuta Kamizono2019-01-181-1/+1
| | | | | | | | | | | | | | | | | | The `unboundable?` behaves like the `infinite?`. ```ruby inf = Topic.predicate_builder.build_bind_attribute(:id, Float::INFINITY) inf.infinite? # => 1 oob = Topic.predicate_builder.build_bind_attribute(:id, 9999999999999999999999999999999) oob.unboundable? # => 1 inf = Topic.predicate_builder.build_bind_attribute(:id, -Float::INFINITY) inf.infinite? # => -1 oob = Topic.predicate_builder.build_bind_attribute(:id, -9999999999999999999999999999999) oob.unboundable? # => -1 ```
* Refactor `bind_attribute` to expand an association to actual attributeRyuta Kamizono2019-01-111-5/+0
|
* Refactor `build_relation` in the uniqueness validator to avoid low level ↵Ryuta Kamizono2019-01-111-20/+13
| | | | predicate construction
* `id_in_database` do not return nil value for persisted recordRyuta Kamizono2018-03-041-1/+1
| | | | | This removes `|| id` which were added in #9963 and #23887 since it is no longer necessary.
* remove incorrect statement about serializable transactionsJoe Van Dyk2017-10-251-3/+1
| | | using serializable isolation would prevent the duplicate insert as done in the example from happening
* Update links to use https instead of http [ci skip]Yoshiyuki Hirano2017-08-221-1/+1
|
* Check :scope input in Uniqueness validatorKir Shatrov2017-08-131-0/+4
|
* Merge pull request #29946 from kamipo/passing_arel_to_where_is_boundableSean Griffin2017-07-281-4/+1
|\ | | | | Building `where_clause` in `UniquenessValidator` is no longer needed
| * Building `where_clause` in `UniquenessValidator` is no longer neededRyuta Kamizono2017-07-261-4/+1
| | | | | | | | | | | | 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.
* | Use `predicate_builder.build_bind_attribute` wherever possibleRyuta Kamizono2017-07-281-2/+1
|/ | | | For less duplicated code.
* Refactor Active Record to let Arel manage bind paramsSean Griffin2017-07-241-1/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* Use frozen-string-literal in ActiveRecordKir Shatrov2017-07-195-0/+10
|
* Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"Matthew Draper2017-07-025-5/+0
| | | | | This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
* Enforce frozen string in RubocopKir Shatrov2017-07-015-0/+5
|
* change ActiveModel::Validation to ActiveModel::Validations in commentsSen Zhang2017-02-151-1/+1
|
* Merge pull request #26378 from ↵Jeremy Daer2017-02-061-31/+1
|\ | | | | | | | | kamipo/decouple_building_arel_ast_for_uniqueness_validator Decouple the building Arel ASTs for uniqueness validator
| * Decouple the building Arel ASTs for uniqueness validatorRyuta Kamizono2016-12-251-31/+1
| | | | | | | | | | | | Currently uniqueness validator is coupled with building Arel ASTs. This commit extracts `WhereClauseFactory#build_for_case_sensitive` for decouple the building Arel ASTs.
* | No need `:doc:` for `:nodoc:` classes [ci skip]Ryuta Kamizono2016-12-251-2/+2
|/ | | | | | Follow up to 5b14129d8d4ad302b4e11df6bd5c7891b75f393c. http://edgeapi.rubyonrails.org/classes/ActiveRecord/Attribute.html
* Privatize unneededly protected methods in Active RecordAkira Matsuda2016-12-241-5/+5
|
* fix the uniqueness validation scope with a polymorphic associationSergey Alekseev2016-11-261-4/+3
| | | | https://gist.github.com/sergey-alekseev/946657ebdb5e58d1bee115714056ec96
* Merge pull request #26905 from bogdanvlviv/docsAndrew White2016-11-132-2/+2
|\ | | | | Add missing `+` around a some literals.
| * Add missing `+` around a some literals.bogdanvlviv2016-10-272-2/+2
| | | | | | | | | | | | Mainly around `nil` [ci skip]
* | Deprecate the behavior of AR::Dirty inside of after_(create|update|save) ↵Sean Griffin2016-11-011-1/+1
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* Revert "Extract `PredicateBuilder::CaseSensitiveHandler`"Sean Griffin2016-08-311-1/+31
| | | | | | | | | 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.
* Extract `PredicateBuilder::CaseSensitiveHandler`Ryuta Kamizono2016-08-161-31/+1
| | | | | | Currently uniqueness validator is coupled with building Arel ASTs. This commit extracts `PredicateBuilder::CaseSensitiveHandler` for decouple the building Arel ASTs.
* Revert passing arel node with splat binds for `where`Ryuta Kamizono2016-08-061-2/+5
| | | | | | | | | | | | 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.
* Don't passing a nil value to `case_sensitive_comparison`Ryuta Kamizono2016-08-061-7/+7
| | | | | A `value` is only used for checking `value.nil?`. It is unnecessary if immediately return when `value.nil?`.
* Remove unused `table` arg for `UniquenessValidator#scope_relation`Ryuta Kamizono2016-07-181-5/+5
|
* Prevent `RangeError` for `FinderMethods#exists?`Ryuta Kamizono2016-06-161-1/+0
| | | | | | | | | | | | | `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`.
* Avoid type casting in uniqueness validatorRyuta Kamizono2016-06-041-5/+2
| | | | | Type casting in uniqueness validator is for a string value truncation. It was removed at #23523.
* Merge pull request #23523 from kamipo/avoid_truncation_in_uniqueness_validationJeremy Daer2016-04-181-3/+0
|\ | | | | | | Avoid a string value truncation in uniqueness validation
| * Avoid a string value truncation in uniqueness validationRyuta Kamizono2016-02-121-3/+0
| | | | | | | | | | | | | | | | | | 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.
* | Fix uniqueness validation with an after_create hook.Joe Rafaniello2016-02-251-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | Revert changes to validations from PR #18612eileencodes2016-02-234-15/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | Merge pull request #23628 from maclover7/fix-23625Sean Griffin2016-02-231-1/+1
|\ \ | | | | | | Fix issue #23625
| * | Fix issue #23625Jon Moss2016-02-181-1/+1
| |/ | | | | | | | | | | 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.
* | Reduce `attribute.to_s`Ryuta Kamizono2016-02-221-6/+5
| |
* | Always validate record if validating a virtual attributeeileencodes2016-02-203-3/+3
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes #23645 When you're using an `attr_accessor` for a record instead of an attribute in the database there's no way for the record to know if it has `changed?` unless you tell it `attribute_will_change!("attribute")`. The change made in 27aa4dd updated validations to check if a record was `changed?` or `marked_for_destruction?` or not `persisted?`. It did not take into account virtual attributes that do not affect the model's dirty status. The only way to fix this is to always validate the record if the attribute does not belong to the set of attributes the record expects (in `record.attributes`) because virtual attributes will not be in that hash. I think we should consider deprecating this particular behavior in the future and requiring that the user mark the record dirty by noting that the virtual attribute will change. Unfortunately this isn't easy because we have no way of knowing that you did the "right thing" in your application by marking it dirty and will get the deprecation warning even if you are doing the correct thing. For now this restores expected behavior when using a virtual attribute by always validating the record, as well as adds tests for this case. I was going to add the `!record.attributes.include?(attribute)` to the `should_validate?` method but `uniqueness` cannot validate a virtual attribute with nothing to hold on to the attribute. Because of this `should_validate?` was about to become a very messy method so I decided to split them up so we can handle it specifically for each case.
* UniquenessValidator exclude itself when PK changedDiego Silva2016-02-091-1/+1
| | | | | | | 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).
* Refactor `case_{sensitive|insensitive}_comparison`Ryuta Kamizono2016-01-011-3/+6
| | | | | | | | | | | | | | 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]] ```
* Remove legacy mysql adapterAbdelkader Boudih2015-12-171-1/+0
|
* Improve support for non Active Record objects on `validates_associated`Kassio Borges2015-11-081-2/+8
| | | | | | Skipping `marked_for_destruction?` when the associated object does not responds to it make easier to validate virtual associations built on top of Active Model objects and/or serialized objects that implement a `valid?` instance method.
* applies new doc guidelines to Active Record.Yves Senn2015-10-143-8/+11
| | | | | | | | | | | | | | | | | | | 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
* uniqueness validation raises error for persisted record without pk.Yves Senn2015-08-201-1/+5
| | | | | | | | 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).