[ARVADOS] updated: ab90543d36eb2b9ca261d3df9bcc8f2864a8c276

git at public.curoverse.com git at public.curoverse.com
Wed Jun 3 13:38:04 EDT 2015


Summary of changes:
 apps/workbench/app/models/arvados_base.rb     |  5 ++-
 apps/workbench/test/test_helper.rb            | 35 ++++++++++++-----
 apps/workbench/test/unit/arvados_base_test.rb | 56 +++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 11 deletions(-)
 create mode 100644 apps/workbench/test/unit/arvados_base_test.rb

  discards  2f7a97d6dc0c8d5f8d248c6be878be118d704f17 (commit)
  discards  0fb683f7dbb355ff5615905728cb18cb28d340b2 (commit)
       via  ab90543d36eb2b9ca261d3df9bcc8f2864a8c276 (commit)
       via  14c74eb1e3b4e003e2b42dee4e81d812605fadeb (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (2f7a97d6dc0c8d5f8d248c6be878be118d704f17)
            \
             N -- N -- N (ab90543d36eb2b9ca261d3df9bcc8f2864a8c276)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit ab90543d36eb2b9ca261d3df9bcc8f2864a8c276
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Jun 3 12:05:11 2015 -0400

    6087: Remove unneeded CollectionsController#update special case.
    
    ArvadosBase#save now covers the general case of omitting unchanged attributes.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index d4ea86c..e01151c 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -273,15 +273,6 @@ class CollectionsController < ApplicationController
     sharing_popup
   end
 
-  def update
-    @updates ||= params[@object.resource_param_name.to_sym]
-    if @updates && (@updates.keys - ["name", "description"]).empty?
-      # exclude manifest_text since only name or description is being updated
-      @object.manifest_text = nil
-    end
-    super
-  end
-
   protected
 
   def find_usable_token(token_list)

commit 14c74eb1e3b4e003e2b42dee4e81d812605fadeb
Author: Tom Clegg <tom at curoverse.com>
Date:   Tue Jun 2 15:23:51 2015 -0400

    6087: Reset changed-attrs list after saving. Fix only-send-changed-attrs logic. Add tests.

diff --git a/apps/workbench/app/models/arvados_base.rb b/apps/workbench/app/models/arvados_base.rb
index 2fca11e..3599cac 100644
--- a/apps/workbench/app/models/arvados_base.rb
+++ b/apps/workbench/app/models/arvados_base.rb
@@ -171,8 +171,9 @@ class ArvadosBase < ActiveRecord::Base
   def save
     obdata = {}
     self.class.columns.each do |col|
-      unless self.send(col.name.to_sym).nil? and !self.changed.include?(col.name)
-          obdata[col.name.to_sym] = self.send(col.name.to_sym)
+      if changed.include? col.name or
+          self.class.serialized_attributes.include? col.name
+        obdata[col.name.to_sym] = self.send col.name
       end
     end
     obdata.delete :id
@@ -199,6 +200,7 @@ class ArvadosBase < ActiveRecord::Base
     end
 
     @new_record = false
+    changes_applied
 
     self
   end
@@ -281,6 +283,7 @@ class ArvadosBase < ActiveRecord::Base
     end
     @all_links = nil
     @new_record = false
+    changes_applied
     self
   end
 
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index ffc122b..89d15c6 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -95,23 +95,40 @@ module ApiFixtureLoader
 end
 
 module ApiMockHelpers
-  def stub_api_calls_with_body body, status_code=200, headers={}
+  def fake_api_response body, status_code, headers
     resp = mock
-    stubbed_client = ArvadosApiClient.new
-    stubbed_client.instance_eval do
-      resp.responds_like_instance_of HTTP::Message
-      resp.stubs(:headers).returns headers
-      resp.stubs(:content).returns body
-      resp.stubs(:status_code).returns status_code
+    resp.responds_like_instance_of HTTP::Message
+    resp.stubs(:headers).returns headers
+    resp.stubs(:content).returns body
+    resp.stubs(:status_code).returns status_code
+    resp
+  end
+
+  def stub_api_calls_with_body body, status_code=200, headers={}
+    stub_api_calls
+    resp = fake_api_response body, status_code, headers
+    stub_api_client.stubs(:post).returns resp
+  end
+
+  def stub_api_calls
+    @stubbed_client = ArvadosApiClient.new
+    @stubbed_client.instance_eval do
       @api_client = HTTPClient.new
-      @api_client.stubs(:post).returns resp
     end
-    ArvadosApiClient.stubs(:new_or_current).returns(stubbed_client)
+    ArvadosApiClient.stubs(:new_or_current).returns(@stubbed_client)
   end
 
   def stub_api_calls_with_invalid_json
     stub_api_calls_with_body ']"omg,bogus"['
   end
+
+  # Return the HTTPClient mock used by the ArvadosApiClient mock. You
+  # must have called stub_api_calls first.
+  def stub_api_client
+    @stubbed_client.instance_eval do
+      @api_client
+    end
+  end
 end
 
 class ActiveSupport::TestCase
diff --git a/apps/workbench/test/unit/arvados_base_test.rb b/apps/workbench/test/unit/arvados_base_test.rb
new file mode 100644
index 0000000..f202fe7
--- /dev/null
+++ b/apps/workbench/test/unit/arvados_base_test.rb
@@ -0,0 +1,56 @@
+require 'test_helper'
+
+class ArvadosBaseTest < ActiveSupport::TestCase
+  test '#save does not send unchanged string attributes' do
+    use_token :active do
+      fixture = api_fixture("collections")["foo_collection_in_aproject"]
+      c = Collection.find(fixture['uuid'])
+
+      new_name = 'name changed during test'
+
+      got_query = nil
+      stub_api_calls
+      stub_api_client.expects(:post).with do |url, query, opts={}|
+        got_query = query
+        true
+      end.returns fake_api_response('{}', 200, {})
+      c.name = new_name
+      c.save
+
+      updates = JSON.parse got_query['collection']
+      assert_equal updates['name'], new_name
+      refute_includes updates, 'description'
+      refute_includes updates, 'manifest_text'
+    end
+  end
+
+  [false,
+   {},
+   {'foo' => 'bar'},
+  ].each do |init_props|
+    test "#save sends serialized attributes if changed from #{init_props}" do
+      use_token :active do
+        fixture = api_fixture("collections")["foo_collection_in_aproject"]
+        c = Collection.find(fixture['uuid'])
+
+        if init_props
+          c.properties = init_props if init_props
+          c.save!
+        end
+
+        got_query = nil
+        stub_api_calls
+        stub_api_client.expects(:post).with do |url, query, opts={}|
+          got_query = query
+          true
+        end.returns fake_api_response('{"etag":"fake","uuid":"fake"}', 200, {})
+
+        c.properties['baz'] = 'qux'
+        c.save!
+
+        updates = JSON.parse got_query['collection']
+        assert_includes updates, 'properties'
+      end
+    end
+  end
+end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list