[arvados] created: 2.4.2-28-g991940e99

[git repository hosting]

        at  991940e9924c33a08a6cf57142873fd45732b1d2 (commit)


commit 991940e9924c33a08a6cf57142873fd45732b1d2
Author: Tom Clegg <tom at curii.com>
Date:   Mon Sep 19 16:35:58 2022 -0400

    19502: Fix updating wrong collection with same PDH.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index 2d200814f..25b113671 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -334,7 +334,11 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo
 		// We looked up UUID->PDH very recently, and we still
 		// have the manifest for that PDH.
 		c.logger.Debugf("cache(%s): have pdh %s and refresh not needed", targetID, pdh)
-		return cached, nil
+		return &arvados.Collection{
+			UUID:             targetID,
+			ManifestText:     cached.ManifestText,
+			PortableDataHash: pdh,
+		}, nil
 	} else {
 		// Get current PDH for this UUID (and confirm we still
 		// have read permission).  Most likely, the cached PDH
@@ -350,13 +354,21 @@ func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceRelo
 			// PDH has not changed, cached manifest is
 			// correct.
 			c.logger.Debugf("cache(%s): verified cached pdh %s is still correct", targetID, pdh)
-			return cached, nil
+			return &arvados.Collection{
+				UUID:             targetID,
+				ManifestText:     cached.ManifestText,
+				PortableDataHash: pdh,
+			}, nil
 		}
 		if cached := c.lookupCollection(arv.ApiToken + "\000" + current.PortableDataHash); cached != nil {
 			// PDH changed, and we already have the
 			// manifest for that new PDH.
 			c.logger.Debugf("cache(%s): cached pdh %s was stale, new pdh is %s and manifest is already in cache", targetID, pdh, current.PortableDataHash)
-			return cached, nil
+			return &arvados.Collection{
+				UUID:             targetID,
+				ManifestText:     cached.ManifestText,
+				PortableDataHash: current.PortableDataHash,
+			}, nil
 		}
 	}
 
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 6412789ff..45a21c546 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -987,6 +987,78 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 	}
 }
 
+// see https://dev.arvados.org/issues/19502
+func (s *IntegrationSuite) TestUpdateIdenticalCollection(c *check.C) {
+	s.testServer.Config.cluster.Services.WebDAVDownload.ExternalURL.Host = "example.com"
+	arv := arvados.NewClientFromEnv()
+	var collections [2]arvados.Collection
+	for i := range collections {
+		err := arv.RequestAndDecode(&collections[i], "POST", "arvados/v1/collections", nil, map[string]interface{}{
+			"collection": map[string]string{
+				"owner_uuid":    arvadostest.ActiveUserUUID,
+				"manifest_text": ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n",
+				"name":          "keep-web test collection",
+			},
+			"ensure_unique_name": true,
+		})
+		c.Assert(err, check.IsNil)
+		defer arv.RequestAndDecode(nil, "DELETE", "arvados/v1/collections/"+collections[i].UUID, nil, nil)
+	}
+	// Warm up the cache with collection 1 (uuid1=>pdh and
+	// pdh=>collection1) followed by collection 2 (uuid2=>pdh and
+	// pdh=>collection2)
+	for _, coll := range collections {
+		u, _ := url.Parse("http://example.com/c=" + coll.UUID + "/foo.txt")
+		req := &http.Request{
+			Method:     "GET",
+			Host:       u.Host,
+			URL:        u,
+			RequestURI: u.RequestURI(),
+			Header: http.Header{
+				"Authorization": {"Bearer " + arvadostest.ActiveToken},
+			},
+		}
+		resp := httptest.NewRecorder()
+		s.testServer.Handler.ServeHTTP(resp, req)
+		c.Check(resp.Code, check.Equals, http.StatusOK)
+	}
+	// Delete a file from the 1st collection (this will use the
+	// cached collection record, which was fetched with uuid2)
+	{
+		u, _ := url.Parse("http://example.com/c=" + collections[0].UUID + "/foo.txt")
+		req := &http.Request{
+			Method:     "DELETE",
+			Host:       u.Host,
+			URL:        u,
+			RequestURI: u.RequestURI(),
+			Header: http.Header{
+				"Authorization": {"Bearer " + arvadostest.ActiveToken},
+			},
+		}
+		resp := httptest.NewRecorder()
+		s.testServer.Handler.ServeHTTP(resp, req)
+		c.Check(resp.Code, check.Equals, http.StatusNoContent)
+	}
+	// Ensure the 1st collection was modified, not the 2nd one
+	for i, coll := range collections {
+		c.Logf("fetch collection #%d", i)
+		u, _ := url.Parse("http://example.com/c=" + coll.UUID + "/foo.txt")
+		req := &http.Request{
+			Method:     "GET",
+			Host:       u.Host,
+			URL:        u,
+			RequestURI: u.RequestURI(),
+			Header: http.Header{
+				"Authorization": {"Bearer " + arvadostest.ActiveToken},
+				"Cache-Control": {"no-cache"},
+			},
+		}
+		resp := httptest.NewRecorder()
+		s.testServer.Handler.ServeHTTP(resp, req)
+		c.Check(resp.Code == http.StatusOK, check.Equals, i == 1)
+	}
+}
+
 func (s *IntegrationSuite) TestDeleteLastFile(c *check.C) {
 	arv := arvados.NewClientFromEnv()
 	var newCollection arvados.Collection

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


hooks/post-receive
--