Remove the sha1_hash from the package_metadata table

I'm thinking about adding more fields to this table, and the sha1_hash values
will make this tricker.

Therefore, remove the value, and adjust the existing code to cope. This commit
also adds a new test which coveres some of the changed functionality.

Makefile.am

@@ -66,6 +66,7 @@ TESTS =				\
   tests/model-derivation.scm		\
   tests/model-git-branch.scm		\
   tests/model-git-repository.scm	\
+  tests/model-package-metadata.scm	\
   tests/branch-updated-emails.scm
 
 AM_TESTS_ENVIRONMENT = abs_top_srcdir="$(abs_top_srcdir)"

guix-data-service/model/package-metadata.scm

@@ -8,20 +8,26 @@
   #:use-module (guix base16)
   #:use-module (guix inferior)
   #:use-module (guix-data-service model utils)
-  #:export (select-package-metadata
+  #:export (select-package-metadata-by-revision-name-and-version
-            select-package-metadata-by-revision-name-and-version
-            insert-package-metadata
             inferior-packages->package-metadata-ids))
 
-(define (select-package-metadata hashes)
+(define (select-package-metadata package-metadata-values)
-  (string-append "SELECT id, sha1_hash "
+  (string-append "SELECT id, package_metadata.synopsis, "
+                 "package_metadata.description, package_metadata.home_page "
                  "FROM package_metadata "
-                 "WHERE sha1_hash IN ("
+                 "JOIN (VALUES "
-                 (string-join (map (lambda (hash)
+                 (string-join (map (lambda (field-values)
-                                     (simple-format #f "'~A'" hash))
+                                     (apply
-                                   hashes)
+                                      simple-format
+                                      #f "(~A, ~A, ~A)"
+                                      (map value->quoted-string-or-null
+                                           field-values)))
+                                   package-metadata-values)
                               ",")
-                 ");"))
+                 ") AS vals (synopsis, description, home_page) "
+                 "ON package_metadata.synopsis = vals.synopsis AND "
+                 "package_metadata.description = vals.description AND "
+                 "package_metadata.home_page = vals.home_page"))
 
 (define (select-package-metadata-by-revision-name-and-version
          conn revision-commit-hash name version)
@@ -48,13 +54,13 @@ WHERE packages.id IN (
 
 (define (insert-package-metadata metadata-rows)
   (string-append "INSERT INTO package_metadata "
-                 "(sha1_hash, synopsis, description, home_page) "
+                 "(synopsis, description, home_page) "
                  "VALUES "
                  (string-join
                   (map (match-lambda
-                         ((sha1_hash synopsis description home_page)
+                         ((synopsis description home_page)
                           (string-append
-                           "('" sha1_hash "',"
+                           "("
                            (value->quoted-string-or-null synopsis) ","
                            (value->quoted-string-or-null description) ","
                            (value->quoted-string-or-null home_page) ")")))
@@ -67,54 +73,41 @@ WHERE packages.id IN (
 (define (inferior-packages->package-metadata-ids conn packages)
   (define package-metadata
     (map (lambda (package)
-           (let ((data (list (inferior-package-synopsis package)
+           (list (inferior-package-synopsis package)
-                             (inferior-package-description package)
+                 (inferior-package-description package)
-                             (inferior-package-home-page package))))
+                 (inferior-package-home-page package)))
-             `(,(bytevector->base16-string
-                 (sha1 (string->utf8
-                        (string-join
-                         (map (lambda (d)
-                                (cond
-                                 ((string? d) d)
-                                 ((boolean? d) (simple-format #f "~A" d))
-                                 (else d)))
-                              data)
-                         ":"))))
-               ,@data)))
          packages))
 
-  (define package-metadata-hashes
-    (map first package-metadata))
-
   (let* ((existing-package-metadata-entries
           (exec-query->vhash conn
-                             (select-package-metadata
+                             (select-package-metadata package-metadata)
-                              package-metadata-hashes)
+                             (lambda (results)
-                             second ;; sha1_hash
+                               (cdr (take results 4)))
                              first)) ;; id))
          (missing-package-metadata-entries
           (delete-duplicates
            (filter (lambda (metadata)
-                     (not (vhash-assoc (first metadata)
+                     (not (vhash-assoc metadata
                                        existing-package-metadata-entries)))
                    package-metadata)))
          (new-package-metadata-entries
           (if (null? missing-package-metadata-entries)
               '()
-              (map car (exec-query conn
+              (map first
-                                   (insert-package-metadata
+                   (exec-query conn
-                                    missing-package-metadata-entries)))))
+                               (insert-package-metadata
+                                missing-package-metadata-entries)))))
          (new-entries-id-lookup-vhash
-          (two-lists->vhash (map first missing-package-metadata-entries)
+          (two-lists->vhash missing-package-metadata-entries
                             new-package-metadata-entries)))
 
-    (map (lambda (sha1-hash)
+    (map (lambda (package-metadata-values)
            (cdr
-            (or (vhash-assoc sha1-hash
+            (or (vhash-assoc package-metadata-values
                              existing-package-metadata-entries)
-                (vhash-assoc sha1-hash
+                (vhash-assoc package-metadata-values
                              new-entries-id-lookup-vhash)
                 (begin
-                  sha1-hash
+                  (error "missing package-metadata entry"
-                  (error "missing package-metadata entry")))))
+                         package-metadata-values)))))
-         package-metadata-hashes)))
+         package-metadata)))

sqitch/deploy/remove_package_metadata_sha1_hash.sql

@@ -0,0 +1,7 @@
+-- Deploy guix-data-service:remove_package_metadata_sha1_hash to pg
+
+BEGIN;
+
+ALTER TABLE package_metadata DROP COLUMN sha1_hash;
+
+COMMIT;

sqitch/revert/remove_package_metadata_sha1_hash.sql

@@ -0,0 +1,7 @@
+-- Revert guix-data-service:remove_package_metadata_sha1_hash from pg
+
+BEGIN;
+
+-- XXX Add DDLs here.
+
+COMMIT;

sqitch/sqitch.plan

@@ -7,3 +7,4 @@ buildstatus_enum [appschema] 2019-04-13T11:56:37Z Christopher Baines <mail@cbain
 initial_import 2019-04-13T13:06:28Z Christopher Baines <mail@cbaines.net> # Import the manually managed database schema
 git_repositories 2019-05-04T19:03:38Z Christopher Baines <mail@cbaines.net> # Add a git_repositories table
 git_branches 2019-05-05T14:53:12Z Christopher Baines <mail@cbaines.net> # Add a git_branches table
+remove_package_metadata_sha1_hash 2019-05-12T10:37:40Z Christopher Baines <mail@cbaines.net> # Remove the sha1_hash field from package_metadata

sqitch/verify/remove_package_metadata_sha1_hash.sql

@@ -0,0 +1,7 @@
+-- Verify guix-data-service:remove_package_metadata_sha1_hash on pg
+
+BEGIN;
+
+-- XXX Add verifications here.
+
+ROLLBACK;

tests/model-package-metadata.scm

@@ -0,0 +1,37 @@
+(define-module (test-model-package-metadata)
+  #:use-module (ice-9 match)
+  #:use-module (srfi srfi-64)
+  #:use-module (tests mock-inferior)
+  #:use-module (guix-data-service database))
+
+(test-begin "test-model-package-metadata")
+
+(define mock-inferior-package-foo
+  (mock-inferior-package
+   (name "foo")
+   (version "2")
+   (synopsis "Foo")
+   (description "Foo description")
+   (home-page "https://example.com")))
+
+(with-mock-inferior-packages
+ (lambda ()
+   (use-modules (guix-data-service model package)
+                (guix-data-service model git-repository)
+                (guix-data-service model guix-revision)
+                (guix-data-service model package-metadata))
+
+   (with-postgresql-connection
+    (lambda (conn)
+      (test-assert "inferior-packages->package-metadata-ids"
+        (with-postgresql-transaction
+         conn
+         (lambda (conn)
+           (match
+               (inferior-packages->package-metadata-ids
+                conn
+                (list mock-inferior-package-foo))
+             ((x) (string? x))))
+         #:always-rollback? #t))))))
+
+(test-end)