Merge branch '17202-no-redir-crossorigin'
authorTom Clegg <tom@curii.com>
Wed, 9 Dec 2020 23:47:15 +0000 (18:47 -0500)
committerTom Clegg <tom@curii.com>
Wed, 9 Dec 2020 23:47:15 +0000 (18:47 -0500)
refs #17202

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

lib/controller/integration_test.go
lib/costanalyzer/costanalyzer.go
lib/costanalyzer/costanalyzer_test.go
sdk/python/tests/run_test_server.py
services/api/app/models/collection.rb
services/api/db/migrate/20201202174753_fix_collection_versions_timestamps.rb [new file with mode: 0644]
services/api/db/structure.sql
services/api/lib/fix_collection_versions_timestamps.rb [new file with mode: 0644]
services/api/test/fixtures/collections.yml
services/api/test/unit/collection_test.rb
tools/arvbox/bin/arvbox

index 6ac8c2e338205ad0fbcf0ae30e019b49b2705116..3418c1f81a1f9d7dfc4881fe300fd94fdaefc984 100644 (file)
@@ -350,9 +350,8 @@ func (s *IntegrationSuite) TestS3WithFederatedToken(c *check.C) {
                c.Check(err, check.IsNil)
                c.Check(string(buf), check.Matches, `.* `+fmt.Sprintf("%d", len(testText))+` +s3://`+coll.UUID+`/test.txt\n`)
 
-               buf, err = exec.Command("s3cmd", append(s3args, "get", "s3://"+coll.UUID+"/test.txt", c.MkDir()+"/tmpfile")...).CombinedOutput()
+               buf, _ = exec.Command("s3cmd", append(s3args, "get", "s3://"+coll.UUID+"/test.txt", c.MkDir()+"/tmpfile")...).CombinedOutput()
                // Command fails because we don't return Etag header.
-               // c.Check(err, check.IsNil)
                flen := strconv.Itoa(len(testText))
                c.Check(string(buf), check.Matches, `(?ms).*`+flen+` of `+flen+`.*`)
        }
index 4284542b869a07d713d698fbb7eaa1a243e77ec5..d81ade607c6ebc1b1426220077b60c9fa3cad77d 100644 (file)
@@ -44,7 +44,9 @@ func (i *arrayFlags) String() string {
 }
 
 func (i *arrayFlags) Set(value string) error {
-       *i = append(*i, value)
+       for _, s := range strings.Split(value, ",") {
+               *i = append(*i, s)
+       }
        return nil
 }
 
@@ -54,20 +56,26 @@ func parseFlags(prog string, args []string, loader *config.Loader, logger *logru
        flags.Usage = func() {
                fmt.Fprintf(flags.Output(), `
 Usage:
-  %s [options ...]
+  %s [options ...] <uuid> ...
 
        This program analyzes the cost of Arvados container requests. For each uuid
        supplied, it creates a CSV report that lists all the containers used to
        fulfill the container request, together with the machine type and cost of
-       each container.
+       each container. At least one uuid must be specified.
 
        When supplied with the uuid of a container request, it will calculate the
-       cost of that container request and all its children. When suplied with a
-       project uuid or when supplied with multiple container request uuids, it will
-       create a CSV report for each supplied uuid, as well as a CSV file with
-       aggregate cost accounting for all supplied uuids. The aggregate cost report
-       takes container reuse into account: if a container was reused between several
-       container requests, its cost will only be counted once.
+       cost of that container request and all its children.
+
+       When supplied with the uuid of a collection, it will see if there is a
+       container_request uuid in the properties of the collection, and if so, it
+       will calculate the cost of that container request and all its children.
+
+       When supplied with a project uuid or when supplied with multiple container
+       request or collection uuids, it will create a CSV report for each supplied
+       uuid, as well as a CSV file with aggregate cost accounting for all supplied
+       uuids. The aggregate cost report takes container reuse into account: if a
+       container was reused between several container requests, its cost will only
+       be counted once.
 
        To get the node costs, the progam queries the Arvados API for current cost
        data for each node type used. This means that the reported cost always
@@ -86,13 +94,18 @@ Usage:
        In order to get the data for the uuids supplied, the ARVADOS_API_HOST and
        ARVADOS_API_TOKEN environment variables must be set.
 
+       This program prints the total dollar amount from the aggregate cost
+       accounting across all provided uuids on stdout.
+
+       When the '-output' option is specified, a set of CSV files with cost details
+       will be written to the provided directory.
+
 Options:
 `, prog)
                flags.PrintDefaults()
        }
        loglevel := flags.String("log-level", "info", "logging `level` (debug, info, ...)")
-       flags.StringVar(&resultsDir, "output", "", "output `directory` for the CSV reports (required)")
-       flags.Var(&uuids, "uuid", "Toplevel `project or container request` uuid. May be specified more than once. (required)")
+       flags.StringVar(&resultsDir, "output", "", "output `directory` for the CSV reports")
        flags.BoolVar(&cache, "cache", true, "create and use a local disk cache of Arvados objects")
        err = flags.Parse(args)
        if err == flag.ErrHelp {
@@ -103,6 +116,7 @@ Options:
                exitCode = 2
                return
        }
+       uuids = flags.Args()
 
        if len(uuids) < 1 {
                flags.Usage()
@@ -111,13 +125,6 @@ Options:
                return
        }
 
-       if resultsDir == "" {
-               flags.Usage()
-               err = fmt.Errorf("Error: output directory must be specified")
-               exitCode = 2
-               return
-       }
-
        lvl, err := logrus.ParseLevel(*loglevel)
        if err != nil {
                exitCode = 2
@@ -180,8 +187,8 @@ func addContainerLine(logger *logrus.Logger, node nodeInfo, cr arvados.Container
 
 func loadCachedObject(logger *logrus.Logger, file string, uuid string, object interface{}) (reload bool) {
        reload = true
-       if strings.Contains(uuid, "-j7d0g-") {
-               // We do not cache projects, they have no final state
+       if strings.Contains(uuid, "-j7d0g-") || strings.Contains(uuid, "-4zz18-") {
+               // We do not cache projects or collections, they have no final state
                return
        }
        // See if we have a cached copy of this object
@@ -251,6 +258,8 @@ func loadObject(logger *logrus.Logger, ac *arvados.Client, path string, uuid str
                err = ac.RequestAndDecode(&object, "GET", "arvados/v1/container_requests/"+uuid, nil, nil)
        } else if strings.Contains(uuid, "-dz642-") {
                err = ac.RequestAndDecode(&object, "GET", "arvados/v1/containers/"+uuid, nil, nil)
+       } else if strings.Contains(uuid, "-4zz18-") {
+               err = ac.RequestAndDecode(&object, "GET", "arvados/v1/collections/"+uuid, nil, nil)
        } else {
                err = fmt.Errorf("unsupported object type with UUID %q:\n  %s", uuid, err)
                return
@@ -309,7 +318,6 @@ func getNode(arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclien
 }
 
 func handleProject(logger *logrus.Logger, uuid string, arv *arvadosclient.ArvadosClient, ac *arvados.Client, kc *keepclient.KeepClient, resultsDir string, cache bool) (cost map[string]float64, err error) {
-
        cost = make(map[string]float64)
 
        var project arvados.Group
@@ -366,9 +374,27 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
        var tmpTotalCost float64
        var totalCost float64
 
+       var crUUID = uuid
+       if strings.Contains(uuid, "-4zz18-") {
+               // This is a collection, find the associated container request (if any)
+               var c arvados.Collection
+               err = loadObject(logger, ac, uuid, uuid, cache, &c)
+               if err != nil {
+                       return nil, fmt.Errorf("error loading collection object %s: %s", uuid, err)
+               }
+               value, ok := c.Properties["container_request"]
+               if !ok {
+                       return nil, fmt.Errorf("error: collection %s does not have a 'container_request' property", uuid)
+               }
+               crUUID, ok = value.(string)
+               if !ok {
+                       return nil, fmt.Errorf("error: collection %s does not have a 'container_request' property of the string type", uuid)
+               }
+       }
+
        // This is a container request, find the container
        var cr arvados.ContainerRequest
-       err = loadObject(logger, ac, uuid, uuid, cache, &cr)
+       err = loadObject(logger, ac, crUUID, crUUID, cache, &cr)
        if err != nil {
                return nil, fmt.Errorf("error loading cr object %s: %s", uuid, err)
        }
@@ -424,13 +450,15 @@ func generateCrCsv(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvado
 
        csv += "TOTAL,,,,,,,,," + strconv.FormatFloat(totalCost, 'f', 8, 64) + "\n"
 
-       // Write the resulting CSV file
-       fName := resultsDir + "/" + uuid + ".csv"
-       err = ioutil.WriteFile(fName, []byte(csv), 0644)
-       if err != nil {
-               return nil, fmt.Errorf("error writing file with path %s: %s", fName, err.Error())
+       if resultsDir != "" {
+               // Write the resulting CSV file
+               fName := resultsDir + "/" + uuid + ".csv"
+               err = ioutil.WriteFile(fName, []byte(csv), 0644)
+               if err != nil {
+                       return nil, fmt.Errorf("error writing file with path %s: %s", fName, err.Error())
+               }
+               logger.Infof("\nUUID report in %s\n\n", fName)
        }
-       logger.Infof("\nUUID report in %s\n\n", fName)
 
        return
 }
@@ -440,10 +468,12 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
        if exitcode != 0 {
                return
        }
-       err = ensureDirectory(logger, resultsDir)
-       if err != nil {
-               exitcode = 3
-               return
+       if resultsDir != "" {
+               err = ensureDirectory(logger, resultsDir)
+               if err != nil {
+                       exitcode = 3
+                       return
+               }
        }
 
        // Arvados Client setup
@@ -474,12 +504,12 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
                        for k, v := range cost {
                                cost[k] = v
                        }
-               } else if strings.Contains(uuid, "-xvhdp-") {
+               } else if strings.Contains(uuid, "-xvhdp-") || strings.Contains(uuid, "-4zz18-") {
                        // This is a container request
                        var crCsv map[string]float64
                        crCsv, err = generateCrCsv(logger, uuid, arv, ac, kc, resultsDir, cache)
                        if err != nil {
-                               err = fmt.Errorf("Error generating container_request CSV for uuid %s: %s", uuid, err.Error())
+                               err = fmt.Errorf("Error generating CSV for uuid %s: %s", uuid, err.Error())
                                exitcode = 2
                                return
                        }
@@ -492,6 +522,10 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
                        // "Home" project is not supported by this program. Skip this uuid, but
                        // keep going.
                        logger.Errorf("Cost analysis is not supported for the 'Home' project: %s", uuid)
+               } else {
+                       logger.Errorf("This argument does not look like a uuid: %s\n", uuid)
+                       exitcode = 3
+                       return
                }
        }
 
@@ -515,14 +549,20 @@ func costanalyzer(prog string, args []string, loader *config.Loader, logger *log
 
        csv += "TOTAL," + strconv.FormatFloat(total, 'f', 8, 64) + "\n"
 
-       // Write the resulting CSV file
-       aFile := resultsDir + "/" + time.Now().Format("2006-01-02-15-04-05") + "-aggregate-costaccounting.csv"
-       err = ioutil.WriteFile(aFile, []byte(csv), 0644)
-       if err != nil {
-               err = fmt.Errorf("Error writing file with path %s: %s", aFile, err.Error())
-               exitcode = 1
-               return
+       if resultsDir != "" {
+               // Write the resulting CSV file
+               aFile := resultsDir + "/" + time.Now().Format("2006-01-02-15-04-05") + "-aggregate-costaccounting.csv"
+               err = ioutil.WriteFile(aFile, []byte(csv), 0644)
+               if err != nil {
+                       err = fmt.Errorf("Error writing file with path %s: %s", aFile, err.Error())
+                       exitcode = 1
+                       return
+               }
+               logger.Infof("Aggregate cost accounting for all supplied uuids in %s\n", aFile)
        }
-       logger.Infof("Aggregate cost accounting for all supplied uuids in %s\n", aFile)
+
+       // Output the total dollar amount on stdout
+       fmt.Fprintf(stdout, "%s\n", strconv.FormatFloat(total, 'f', 8, 64))
+
        return
 }
index 4fab93bf4fadb13919b0346109bc175c96084c93..3488f0d8dfd62522a7bfd4774f0ac8d7b903a88b 100644 (file)
@@ -161,7 +161,49 @@ func (*Suite) TestUsage(c *check.C) {
 func (*Suite) TestContainerRequestUUID(c *check.C) {
        var stdout, stderr bytes.Buffer
        // Run costanalyzer with 1 container request uuid
-       exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedContainerRequestUUID, "-output", "results"}, &bytes.Buffer{}, &stdout, &stderr)
+       exitcode := Command.RunCommand("costanalyzer.test", []string{"-output", "results", arvadostest.CompletedContainerRequestUUID}, &bytes.Buffer{}, &stdout, &stderr)
+       c.Check(exitcode, check.Equals, 0)
+       c.Assert(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
+
+       uuidReport, err := ioutil.ReadFile("results/" + arvadostest.CompletedContainerRequestUUID + ".csv")
+       c.Assert(err, check.IsNil)
+       c.Check(string(uuidReport), check.Matches, "(?ms).*TOTAL,,,,,,,,,7.01302889")
+       re := regexp.MustCompile(`(?ms).*supplied uuids in (.*?)\n`)
+       matches := re.FindStringSubmatch(stderr.String()) // matches[1] contains a string like 'results/2020-11-02-18-57-45-aggregate-costaccounting.csv'
+
+       aggregateCostReport, err := ioutil.ReadFile(matches[1])
+       c.Assert(err, check.IsNil)
+
+       c.Check(string(aggregateCostReport), check.Matches, "(?ms).*TOTAL,7.01302889")
+}
+
+func (*Suite) TestCollectionUUID(c *check.C) {
+       var stdout, stderr bytes.Buffer
+
+       // Run costanalyzer with 1 collection uuid, without 'container_request' property
+       exitcode := Command.RunCommand("costanalyzer.test", []string{"-output", "results", arvadostest.FooCollection}, &bytes.Buffer{}, &stdout, &stderr)
+       c.Check(exitcode, check.Equals, 2)
+       c.Assert(stderr.String(), check.Matches, "(?ms).*does not have a 'container_request' property.*")
+
+       // Update the collection, attach a 'container_request' property
+       ac := arvados.NewClientFromEnv()
+       var coll arvados.Collection
+
+       // Update collection record
+       err := ac.RequestAndDecode(&coll, "PUT", "arvados/v1/collections/"+arvadostest.FooCollection, nil, map[string]interface{}{
+               "collection": map[string]interface{}{
+                       "properties": map[string]interface{}{
+                               "container_request": arvadostest.CompletedContainerRequestUUID,
+                       },
+               },
+       })
+       c.Assert(err, check.IsNil)
+
+       stdout.Truncate(0)
+       stderr.Truncate(0)
+
+       // Run costanalyzer with 1 collection uuid
+       exitcode = Command.RunCommand("costanalyzer.test", []string{"-output", "results", arvadostest.FooCollection}, &bytes.Buffer{}, &stdout, &stderr)
        c.Check(exitcode, check.Equals, 0)
        c.Assert(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
@@ -180,7 +222,7 @@ func (*Suite) TestContainerRequestUUID(c *check.C) {
 func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
        var stdout, stderr bytes.Buffer
        // Run costanalyzer with 2 container request uuids
-       exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedContainerRequestUUID, "-uuid", arvadostest.CompletedContainerRequestUUID2, "-output", "results"}, &bytes.Buffer{}, &stdout, &stderr)
+       exitcode := Command.RunCommand("costanalyzer.test", []string{"-output", "results", arvadostest.CompletedContainerRequestUUID, arvadostest.CompletedContainerRequestUUID2}, &bytes.Buffer{}, &stdout, &stderr)
        c.Check(exitcode, check.Equals, 0)
        c.Assert(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
@@ -220,7 +262,7 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
        c.Assert(err, check.IsNil)
 
        // Run costanalyzer with the project uuid
-       exitcode = Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.AProjectUUID, "-cache=false", "-log-level", "debug", "-output", "results"}, &bytes.Buffer{}, &stdout, &stderr)
+       exitcode = Command.RunCommand("costanalyzer.test", []string{"-cache=false", "-log-level", "debug", "-output", "results", arvadostest.AProjectUUID}, &bytes.Buffer{}, &stdout, &stderr)
        c.Check(exitcode, check.Equals, 0)
        c.Assert(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
@@ -243,8 +285,19 @@ func (*Suite) TestDoubleContainerRequestUUID(c *check.C) {
 
 func (*Suite) TestMultipleContainerRequestUUIDWithReuse(c *check.C) {
        var stdout, stderr bytes.Buffer
+       // Run costanalyzer with 2 container request uuids, without output directory specified
+       exitcode := Command.RunCommand("costanalyzer.test", []string{arvadostest.CompletedDiagnosticsContainerRequest1UUID, arvadostest.CompletedDiagnosticsContainerRequest2UUID}, &bytes.Buffer{}, &stdout, &stderr)
+       c.Check(exitcode, check.Equals, 0)
+       c.Assert(stderr.String(), check.Not(check.Matches), "(?ms).*supplied uuids in .*")
+
+       // Check that the total amount was printed to stdout
+       c.Check(stdout.String(), check.Matches, "0.01492030\n")
+
+       stdout.Truncate(0)
+       stderr.Truncate(0)
+
        // Run costanalyzer with 2 container request uuids
-       exitcode := Command.RunCommand("costanalyzer.test", []string{"-uuid", arvadostest.CompletedDiagnosticsContainerRequest1UUID, "-uuid", arvadostest.CompletedDiagnosticsContainerRequest2UUID, "-output", "results"}, &bytes.Buffer{}, &stdout, &stderr)
+       exitcode = Command.RunCommand("costanalyzer.test", []string{"-output", "results", arvadostest.CompletedDiagnosticsContainerRequest1UUID, arvadostest.CompletedDiagnosticsContainerRequest2UUID}, &bytes.Buffer{}, &stdout, &stderr)
        c.Check(exitcode, check.Equals, 0)
        c.Assert(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*")
 
index 0cb4151ac3b040292c0af4b5c8b5eef888b48096..c79aa4e945924f782b93fc7c76389b657fe6ecc7 100644 (file)
@@ -73,6 +73,7 @@ if not os.path.exists(TEST_TMPDIR):
 my_api_host = None
 _cached_config = {}
 _cached_db_config = {}
+_already_used_port = {}
 
 def find_server_pid(PID_PATH, wait=10):
     now = time.time()
@@ -181,11 +182,15 @@ def find_available_port():
     would take care of the races, and this wouldn't be needed at all.
     """
 
-    sock = socket.socket()
-    sock.bind(('0.0.0.0', 0))
-    port = sock.getsockname()[1]
-    sock.close()
-    return port
+    global _already_used_port
+    while True:
+        sock = socket.socket()
+        sock.bind(('0.0.0.0', 0))
+        port = sock.getsockname()[1]
+        sock.close()
+        if port not in _already_used_port:
+            _already_used_port[port] = True
+            return port
 
 def _wait_until_port_listens(port, timeout=10, warn=True):
     """Wait for a process to start listening on the given port.
index 8b549a71ab4fba348ab9279f456595912fb693db..3637f34e105b1bd66013a7cc0882860dc434034e 100644 (file)
@@ -269,6 +269,7 @@ class Collection < ArvadosModel
         snapshot = self.dup
         snapshot.uuid = nil # Reset UUID so it's created as a new record
         snapshot.created_at = self.created_at
+        snapshot.modified_at = self.modified_at_was
       end
 
       # Restore requested changes on the current version
@@ -294,8 +295,10 @@ class Collection < ArvadosModel
       if snapshot
         snapshot.attributes = self.syncable_updates
         leave_modified_by_user_alone do
-          act_as_system_user do
-            snapshot.save
+          leave_modified_at_alone do
+            act_as_system_user do
+              snapshot.save
+            end
           end
         end
       end
diff --git a/services/api/db/migrate/20201202174753_fix_collection_versions_timestamps.rb b/services/api/db/migrate/20201202174753_fix_collection_versions_timestamps.rb
new file mode 100644 (file)
index 0000000..4c56d3d
--- /dev/null
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'fix_collection_versions_timestamps'
+
+class FixCollectionVersionsTimestamps < ActiveRecord::Migration[5.2]
+  def up
+    # Defined in a function for easy testing.
+    fix_collection_versions_timestamps
+  end
+
+  def down
+    # This migration is not reversible.  However, the results are
+    # backwards compatible.
+  end
+end
index 58c064ac3341ce953d41f83cab0425a0370e5f67..12a28c6c723609bd14b2c20129b8c749695bdc28 100644 (file)
@@ -3186,6 +3186,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20200602141328'),
 ('20200914203202'),
 ('20201103170213'),
-('20201105190435');
+('20201105190435'),
+('20201202174753');
 
 
diff --git a/services/api/lib/fix_collection_versions_timestamps.rb b/services/api/lib/fix_collection_versions_timestamps.rb
new file mode 100644 (file)
index 0000000..61da988
--- /dev/null
@@ -0,0 +1,43 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'set'
+
+include CurrentApiClient
+include ArvadosModelUpdates
+
+def fix_collection_versions_timestamps
+  act_as_system_user do
+    uuids = [].to_set
+    # Get UUIDs from collections with more than 1 version
+    Collection.where(version: 2).find_each(batch_size: 100) do |c|
+      uuids.add(c.current_version_uuid)
+    end
+    uuids.each do |uuid|
+      first_pair = true
+      # All versions of a particular collection get fixed together.
+      ActiveRecord::Base.transaction do
+        Collection.where(current_version_uuid: uuid).order(version: :desc).each_cons(2) do |c1, c2|
+          # Skip if the 2 newest versions' modified_at values are separate enough;
+          # this means that this collection doesn't require fixing, allowing for
+          # migration re-runs in case of transient problems.
+          break if first_pair && (c2.modified_at.to_f - c1.modified_at.to_f) > 1
+          first_pair = false
+          # Fix modified_at timestamps by assigning to N-1's value to N.
+          # Special case: the first version's modified_at will be == to created_at
+          leave_modified_by_user_alone do
+            leave_modified_at_alone do
+              c1.modified_at = c2.modified_at
+              c1.save!(validate: false)
+              if c2.version == 1
+                c2.modified_at = c2.created_at
+                c2.save!(validate: false)
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
\ No newline at end of file
index 767f035b88cb824c47de95ef9571c5531c228c23..61bb3f79f8c882565b1ee9bd4ec5806963c1cf53 100644 (file)
@@ -23,8 +23,8 @@ collection_owned_by_active:
   created_at: 2014-02-03T17:22:54Z
   modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
   modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
-  modified_at: 2014-02-03T17:22:54Z
-  updated_at: 2014-02-03T17:22:54Z
+  modified_at: 2014-02-03T18:22:54Z
+  updated_at: 2014-02-03T18:22:54Z
   manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
   name: owned_by_active
   version: 2
@@ -43,7 +43,7 @@ collection_owned_by_active_with_file_stats:
   file_count: 1
   file_size_total: 3
   name: owned_by_active_with_file_stats
-  version: 2
+  version: 1
 
 collection_owned_by_active_past_version_1:
   uuid: zzzzz-4zz18-znfnqtbbv4spast
@@ -53,8 +53,8 @@ collection_owned_by_active_past_version_1:
   created_at: 2014-02-03T17:22:54Z
   modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
   modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
-  modified_at: 2014-02-03T15:22:54Z
-  updated_at: 2014-02-03T15:22:54Z
+  modified_at: 2014-02-03T18:22:54Z
+  updated_at: 2014-02-03T18:22:54Z
   manifest_text: ". 37b51d194a7513e45b56f6524f2d51f2+3 0:3:bar\n"
   name: owned_by_active_version_1
   version: 1
@@ -106,8 +106,8 @@ w_a_z_file:
   created_at: 2015-02-09T10:53:38Z
   modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
   modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
-  modified_at: 2015-02-09T10:53:38Z
-  updated_at: 2015-02-09T10:53:38Z
+  modified_at: 2015-02-09T10:55:38Z
+  updated_at: 2015-02-09T10:55:38Z
   manifest_text: ". 4c6c2c0ac8aa0696edd7316a3be5ca3c+5 0:5:w\\040\\141\\040z\n"
   name: "\"w a z\" file"
   version: 2
@@ -120,8 +120,8 @@ w_a_z_file_version_1:
   created_at: 2015-02-09T10:53:38Z
   modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
   modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
-  modified_at: 2015-02-09T10:53:38Z
-  updated_at: 2015-02-09T10:53:38Z
+  modified_at: 2015-02-09T10:55:38Z
+  updated_at: 2015-02-09T10:55:38Z
   manifest_text: ". 4d20280d5e516a0109768d49ab0f3318+3 0:3:waz\n"
   name: "waz file"
   version: 1
index 48cae5afee92622fcda41c2b48e89761f54ac80d..a28893e0119162ea388b698991599e71d74cc3c8 100644 (file)
@@ -4,6 +4,7 @@
 
 require 'test_helper'
 require 'sweep_trashed_objects'
+require 'fix_collection_versions_timestamps'
 
 class CollectionTest < ActiveSupport::TestCase
   include DbCurrentTime
@@ -334,6 +335,7 @@ class CollectionTest < ActiveSupport::TestCase
       # Set up initial collection
       c = create_collection 'foo', Encoding::US_ASCII
       assert c.valid?
+      original_version_modified_at = c.modified_at.to_f
       # Make changes so that a new version is created
       c.update_attributes!({'name' => 'bar'})
       c.reload
@@ -344,9 +346,7 @@ class CollectionTest < ActiveSupport::TestCase
 
       version_creation_datetime = c_old.modified_at.to_f
       assert_equal c.created_at.to_f, c_old.created_at.to_f
-      # Current version is updated just a few milliseconds before the version is
-      # saved on the database.
-      assert_operator c.modified_at.to_f, :<, version_creation_datetime
+      assert_equal original_version_modified_at, version_creation_datetime
 
       # Make update on current version so old version get the attribute synced;
       # its modified_at should not change.
@@ -361,6 +361,29 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  # Bug #17152 - This test relies on fixtures simulating the problem.
+  test "migration fixing collection versions' modified_at timestamps" do
+    versioned_collection_fixtures = [
+      collections(:w_a_z_file).uuid,
+      collections(:collection_owned_by_active).uuid
+    ]
+    versioned_collection_fixtures.each do |uuid|
+      cols = Collection.where(current_version_uuid: uuid).order(version: :desc)
+      assert_equal cols.size, 2
+      # cols[0] -> head version // cols[1] -> old version
+      assert_operator (cols[0].modified_at.to_f - cols[1].modified_at.to_f), :==, 0
+      assert cols[1].modified_at != cols[1].created_at
+    end
+    fix_collection_versions_timestamps
+    versioned_collection_fixtures.each do |uuid|
+      cols = Collection.where(current_version_uuid: uuid).order(version: :desc)
+      assert_equal cols.size, 2
+      # cols[0] -> head version // cols[1] -> old version
+      assert_operator (cols[0].modified_at.to_f - cols[1].modified_at.to_f), :>, 1
+      assert_operator cols[1].modified_at, :==, cols[1].created_at
+    end
+  end
+
   test "past versions should not be directly updatable" do
     Rails.configuration.Collections.CollectionVersioning = true
     Rails.configuration.Collections.PreserveVersionIfIdle = 0
index a180b43630471f0c92ded944e6e1b8290ece4245..96f3666cda9ee498970ebf0c5ce29badd0fc18d8 100755 (executable)
@@ -205,6 +205,7 @@ run() {
               --publish=8900:8900
               --publish=9000:9000
               --publish=9002:9002
+              --publish=9004:9004
               --publish=25101:25101
               --publish=8001:8001
               --publish=8002:8002