[ARVADOS] updated: 2.1.0-1353-g7563e6276

Git user git at public.arvados.org
Thu Sep 16 17:16:13 UTC 2021


Summary of changes:
 lib/controller/federation/conn.go                  | 27 ++++++++++++++++------
 lib/controller/localdb/collection_test.go          |  5 ++--
 .../arvados/v1/collections_controller.rb           |  8 +++++--
 .../arvados/v1/collections_controller_test.rb      | 22 ++++++++++++++++++
 4 files changed, 51 insertions(+), 11 deletions(-)

       via  7563e6276baade41eb3faa3ff167abab1cb0f890 (commit)
       via  2ec0e6087bb56c8354941bbe68d4e4681aede707 (commit)
       via  1c78ef4ac7e6cc1c83143eda5500234146fd46f6 (commit)
      from  5d7bef4122c59bec9145f2853b76170b4ddca67f (commit)

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 7563e6276baade41eb3faa3ff167abab1cb0f890
Author: Tom Clegg <tom at curii.com>
Date:   Thu Sep 16 13:15:29 2021 -0400

    18122: Fix "get collection by pdh" when manifest_text is unselected.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 1b0850a58..c9b36e19e 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -80,18 +80,22 @@ class Arvados::V1::CollectionsController < ApplicationController
       # it will select the Collection object with the longest
       # available lifetime.
 
+      select_attrs = (@select || ["manifest_text"]) | ["portable_data_hash", "trash_at"]
       if c = Collection.
                readable_by(*@read_users, opts).
                where({ portable_data_hash: loc.to_s }).
                order("trash_at desc").
-               select(((@select || ["manifest_text"]) | ["portable_data_hash", "trash_at"]).join(", ")).
+               select(select_attrs.join(", ")).
                limit(1).
                first
         @object = {
           uuid: c.portable_data_hash,
           portable_data_hash: c.portable_data_hash,
-          manifest_text: c.manifest_text,
+          trash_at: c.trash_at,
         }
+        if select_attrs.index("manifest_text")
+          @object[:manifest_text] = c.manifest_text
+        end
       end
     else
       super
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 3b217f06d..af1171598 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1491,4 +1491,26 @@ EOS
     assert_nil json_response["properties"]
     assert_equal collections(:collection_owned_by_active).name, json_response["name"]
   end
+
+  [nil,
+   [],
+   ["is_trashed", "trash_at"],
+   ["is_trashed", "trash_at", "portable_data_hash"],
+   ["portable_data_hash"],
+   ["portable_data_hash", "manifest_text"],
+  ].each do |select|
+    test "select=#{select.inspect} param is respected in 'get by pdh' response" do
+      authorize_with :active
+      get :show, params: {
+            id: collections(:collection_owned_by_active).portable_data_hash,
+            select: select,
+          }
+      assert_response :success
+      if !select || select.index("manifest_text")
+        assert_not_nil json_response["manifest_text"]
+      else
+        assert_nil json_response["manifest_text"]
+      end
+    end
+  end
 end

commit 2ec0e6087bb56c8354941bbe68d4e4681aede707
Author: Tom Clegg <tom at curii.com>
Date:   Thu Sep 16 13:11:57 2021 -0400

    18122: Don't fail on PDH mismatch when manifest_text is missing.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 586ac2301..aa05cb1e6 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -262,13 +262,26 @@ func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions)
 		if err != nil {
 			return err
 		}
-		// options.UUID is either hash+size or
-		// hash+size+hints; only hash+size need to
-		// match the computed PDH.
-		if pdh := arvados.PortableDataHash(c.ManifestText); pdh != options.UUID && !strings.HasPrefix(options.UUID, pdh+"+") {
-			err = httpErrorf(http.StatusBadGateway, "bad portable data hash %q received from remote %q (expected %q)", pdh, remoteID, options.UUID)
-			ctxlog.FromContext(ctx).Warn(err)
-			return err
+		haveManifest := true
+		if options.Select != nil {
+			haveManifest = false
+			for _, s := range options.Select {
+				if s == "manifest_text" {
+					haveManifest = true
+					break
+				}
+			}
+		}
+		if haveManifest {
+			pdh := arvados.PortableDataHash(c.ManifestText)
+			// options.UUID is either hash+size or
+			// hash+size+hints; only hash+size need to
+			// match the computed PDH.
+			if pdh != options.UUID && !strings.HasPrefix(options.UUID, pdh+"+") {
+				err = httpErrorf(http.StatusBadGateway, "bad portable data hash %q received from remote %q (expected %q)", pdh, remoteID, options.UUID)
+				ctxlog.FromContext(ctx).Warn(err)
+				return err
+			}
 		}
 		if remoteID != "" {
 			c.ManifestText = rewriteManifest(c.ManifestText, remoteID)

commit 1c78ef4ac7e6cc1c83143eda5500234146fd46f6
Author: Tom Clegg <tom at curii.com>
Date:   Thu Sep 16 12:09:30 2021 -0400

    18122: Update test.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/collection_test.go b/lib/controller/localdb/collection_test.go
index e0de7256a..4a4494964 100644
--- a/lib/controller/localdb/collection_test.go
+++ b/lib/controller/localdb/collection_test.go
@@ -115,8 +115,9 @@ func (s *CollectionSuite) TestSignatures(c *check.C) {
 		Filters: []arvados.Filter{{"uuid", "=", arvadostest.FooCollection}},
 		Select:  []string{"uuid", "manifest_text"},
 	})
-	c.Check(err, check.IsNil)
-	if c.Check(gresp.Items, check.HasLen, 1) {
+	if err != nil {
+		c.Check(err, check.ErrorMatches, `.*Invalid attribute.*manifest_text.*`)
+	} else if c.Check(gresp.Items, check.HasLen, 1) {
 		c.Check(gresp.Items[0].(map[string]interface{})["uuid"], check.Equals, arvadostest.FooCollection)
 		c.Check(gresp.Items[0].(map[string]interface{})["manifest_text"], check.Equals, nil)
 	}

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list