Refactor invalid scope
What does this MR do and why?
This change converts LfsObject::for_oid_and_size(oid, size)
from a scope
to a class method to prevent us from raising NoMethodError
further down the stack
Rails scopes should return an ActiveRecord::Relation or nil to allow chaining scopes together. When scopes return a single record we can no longer chain them. Returning a single record from a scope usually won't have any undesirable side-effects, however, if the scope returns nil Rails will treat the scope as if you called Klass.all
which causes the app to load all the records in the table and will also return NoMethodError when you try to use a method on the object you expected.
For example:
class LfsObject
scope :subset, ->(ids) { where(id: ids) }
scope :invalid_with_correct_result, -> { find_by(id: first.id) }
scope :invalid_with_unexptected_result, -> { find_by(id: nil) }
Calling LfsObject.subset(first_three_ids)
will return the first 3 records in and ActiveRecord::Relation
(similar to an Array)
However, if you are expecting to return a single record and then calling lfs_object.do_something
, using invalid_with_correct_result
and invalid_with_unexptected_result
behave very differently
#> lfs_object = LfsObject.invalid_with_correct_result
#> lfs_object === LfsObject.first
=> true
#> lfs_object.do_something
=> did something
Whereas when no record is found and nil is returned inside the scope, the scope returns all records so we can continue chaining scopes
invalid_with_unexptected_result
#> lfs_object = LfsObject.invalid_with_unexptected_result
#> lfs_object === nil
=> false
#> lfs_object === LfsObject.all
=> true
#> lfs_object.do_something
=> NoMethodError: undefined method `do_something' for #<LfsObject::ActiveRecord_Relation:0x000000012a3c5520>
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.