Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Refactor Model not to rely on return value of _refresh

Instead, have lock! and refresh return self manually.  This makes
plugins easier, as they don't have to worry about returning the
value of super.  This fixes an issue with the dirty plugin not
having refresh return self.  Thanks to viking for the heads up.
  • Loading branch information...
commit 1808d49f880bf9b823f9e5b800e24bed277820cb 1 parent 8cbb3c4
Jeremy Evans authored
5 lib/sequel/model/base.rb
@@ -1106,7 +1106,8 @@ def keys
1106 1106 # a.update(...)
1107 1107 # end
1108 1108 def lock!
1109   - new? ? self : _refresh(this.for_update)
  1109 + _refresh(this.for_update) unless new?
  1110 + self
1110 1111 end
1111 1112
1112 1113 # Remove elements of the model object that make marshalling fail. Returns self.
@@ -1185,6 +1186,7 @@ def pk_hash
1185 1186 def refresh
1186 1187 raise Sequel::Error, "can't refresh frozen object" if frozen?
1187 1188 _refresh(this)
  1189 + self
1188 1190 end
1189 1191
1190 1192 # Alias of refresh, but not aliased directly to make overriding in a plugin easier.
@@ -1532,7 +1534,6 @@ def _insert_select_raw(ds)
1532 1534 def _refresh(dataset)
1533 1535 set_values(_refresh_get(dataset) || raise(Error, "Record not found"))
1534 1536 changed_columns.clear
1535   - self
1536 1537 end
1537 1538
1538 1539 # Get the row of column data from the database.
3  lib/sequel/plugins/composition.rb
@@ -150,9 +150,8 @@ def define_composition_accessor(name, opts={})
150 150 module InstanceMethods
151 151 # Clear the cached compositions when refreshing.
152 152 def _refresh(ds)
153   - v = super
  153 + super
154 154 compositions.clear
155   - v
156 155 end
157 156
158 157 # For each composition, set the columns in the model class based

5 comments on commit 1808d49

Rodrigo Rosenfeld Rosas

Could you please release a new version including this commit? This change since the previous version has changed my specs...

Jeremy Evans
Owner

This isn't a regression (the dirty plugin was just introduced in the latest version), and I generally do not do point releases except to fix regressions or very serious issues in new features. While it's unfortunate that 3.35.0 shipped with this bug, I don't think it's significant enough to warrant releasing 3.35.1. 3.36.0 will be released June 1, so you won't have to wait that long for a fix.

Rodrigo Rosenfeld Rosas

Fortunately Bundler will allow me to use your master branch until there :)

Rodrigo Rosenfeld Rosas

By the way, I remember a day where Rails has released two minor versions in the same day, so I don't really understand strict rules for release dates :)

Jeremy Evans
Owner

Well, Rails has a lot of developers working on it, whereas Sequel is mostly just me. Also, the two releases in the same day was because Rails released a completely broken release first, and then had to issue another release to fix it. That's not really the case here.

I understand the issue affected you, and I'm sorry for that. Your recent pull request will help ensure that a similar bug does not occur in the future, and I thank you for that.

Please sign in to comment.
Something went wrong with that request. Please try again.