[feature] Raise when an update to a read-only attribute is attempted

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[feature] Raise when an update to a read-only attribute is attempted

Chris Stadler
Currently, updates to read-only attributes fail silently. For example:
class ReadOnly < ActiveRecord::Base
  attr_readonly
:name
end

record
= ReadOnly.create(name: "original")
record
.update(name: "changed") # true
record
.reload.name # "original"

This behavior is highly unexpected to me because it provides no feedback that the updates were not persisted, a significant exception to the normal behavior of #update and #save.

Raising an error whenever an update is attempted, or treating attr_readonly as a validation both seem like good options to me. I've attempted to implement the former (mostly because it was simpler) here: https://github.com/CJStadler/rails/commit/3fe8a29429a3dade8877513db653851b7a43333d.

I realize this would be a breaking change and can't be undertaken lightly, but the current behavior feels like a big "gotcha", and one that's difficult to debug.

Thanks!
Chris Stadler

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [feature] Raise when an update to a read-only attribute is attempted

Ben Woosley
I would also like to see a noisy failure on attempted update, or the option of one.

A good approach would be to add an option to the attr_readonly call to support raise on attempted update. The default behavior could be switched in a later release if desired.

On Sat, Apr 22, 2017 at 14:23 Chris Stadler <[hidden email]> wrote:
Currently, updates to read-only attributes fail silently. For example:
class ReadOnly < ActiveRecord::Base
  attr_readonly
:name
end

record
= ReadOnly.create(name: "original")
record
.update(name: "changed") # true
record
.reload.name # "original"

This behavior is highly unexpected to me because it provides no feedback that the updates were not persisted, a significant exception to the normal behavior of #update and #save.

Raising an error whenever an update is attempted, or treating attr_readonly as a validation both seem like good options to me. I've attempted to implement the former (mostly because it was simpler) here: https://github.com/CJStadler/rails/commit/3fe8a29429a3dade8877513db653851b7a43333d.

I realize this would be a breaking change and can't be undertaken lightly, but the current behavior feels like a big "gotcha", and one that's difficult to debug.

Thanks!
Chris Stadler

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [feature] Raise when an update to a read-only attribute is attempted

Joshua Flanagan
In reply to this post by Chris Stadler
It works exactly as documented:

"will be used to create a new record but update operations will ignore these fields."

If that isn't the behavior you want, you're probably better off using a different solution, rather than changing the intended behavior for everyone. Maybe override the attribute setter:

def name=(val)
  raise ActiveRecord::ReadOnlyRecord.new("Name is readonly) if persisted?
  super
end


On Saturday, April 22, 2017 at 4:23:42 PM UTC-5, Chris Stadler wrote:
Currently, updates to read-only attributes fail silently. For example:
class ReadOnly < ActiveRecord::Base
  attr_readonly
:name
end

record
= ReadOnly.create(name: "original")
record
.update(name: "changed") # true
record
.reload.name # "original"

This behavior is highly unexpected to me because it provides no feedback that the updates were not persisted, a significant exception to the normal behavior of #update and #save.

Raising an error whenever an update is attempted, or treating attr_readonly as a validation both seem like good options to me. I've attempted to implement the former (mostly because it was simpler) here: <a href="https://github.com/CJStadler/rails/commit/3fe8a29429a3dade8877513db653851b7a43333d" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2FCJStadler%2Frails%2Fcommit%2F3fe8a29429a3dade8877513db653851b7a43333d\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG_VDqSP1Dg0NN1_Gt_I2iMFOvjrQ&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2FCJStadler%2Frails%2Fcommit%2F3fe8a29429a3dade8877513db653851b7a43333d\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG_VDqSP1Dg0NN1_Gt_I2iMFOvjrQ&#39;;return true;">https://github.com/CJStadler/rails/commit/3fe8a29429a3dade8877513db653851b7a43333d.

I realize this would be a breaking change and can't be undertaken lightly, but the current behavior feels like a big "gotcha", and one that's difficult to debug.

Thanks!
Chris Stadler

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [feature] Raise when an update to a read-only attribute is attempted

Chris Stadler
I agree that it works as documented. I'm not saying that there is bug to fix, I'm proposing that the API could be improved.

I have not been able to find out what the motivation was for the current behavior (I think this is the initial commit), so I'm interested to know. It seems to me that any attempt to update a readonly attribute is probably the result of a bug. For example, a form may be exposing a readonly attribute (and then incorrectly reporting that the attribute was updated). I'm sure there are cases I haven't thought of though. When is it necessary to allow attempted updates to readonly attributes?

I realize that we cannot just change the current behavior. Does Ben's suggestion of adding an option on attr_readonly seem reasonable?

Thanks!

On Monday, 24 April 2017 12:48:17 UTC-4, Joshua Flanagan wrote:
It works exactly as documented:

"will be used to create a new record but update operations will ignore these fields."

If that isn't the behavior you want, you're probably better off using a different solution, rather than changing the intended behavior for everyone. Maybe override the attribute setter:

def name=(val)
  raise ActiveRecord::ReadOnlyRecord.new("Name is readonly) if persisted?
  super
end


On Saturday, April 22, 2017 at 4:23:42 PM UTC-5, Chris Stadler wrote:
Currently, updates to read-only attributes fail silently. For example:
class ReadOnly < ActiveRecord::Base
  attr_readonly
:name
end

record
= ReadOnly.create(name: "original")
record
.update(name: "changed") # true
record
.reload.name # "original"

This behavior is highly unexpected to me because it provides no feedback that the updates were not persisted, a significant exception to the normal behavior of #update and #save.

Raising an error whenever an update is attempted, or treating attr_readonly as a validation both seem like good options to me. I've attempted to implement the former (mostly because it was simpler) here: <a href="https://github.com/CJStadler/rails/commit/3fe8a29429a3dade8877513db653851b7a43333d" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2FCJStadler%2Frails%2Fcommit%2F3fe8a29429a3dade8877513db653851b7a43333d\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG_VDqSP1Dg0NN1_Gt_I2iMFOvjrQ&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2FCJStadler%2Frails%2Fcommit%2F3fe8a29429a3dade8877513db653851b7a43333d\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNG_VDqSP1Dg0NN1_Gt_I2iMFOvjrQ&#39;;return true;">https://github.com/CJStadler/rails/commit/3fe8a29429a3dade8877513db653851b7a43333d.

I realize this would be a breaking change and can't be undertaken lightly, but the current behavior feels like a big "gotcha", and one that's difficult to debug.

Thanks!
Chris Stadler

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Loading...