Get all rhizome add file test cases passing

- new test cases to cover new payload with different versions - exit status of 2 when duplicate is detected - tighten up other test cases - fix bug in sqlite_exec_int64()
2025-02-20 17:33:08 +00:00 · 2012-04-16 11:46:58 +09:30 · 2012-04-16 11:46:58 +09:30 · 866ddea189
commit 866ddea189
parent 02b253bb29
4 changed files with 108 additions and 51 deletions
--- a/commandline.c
+++ b/commandline.c
@ -670,15 +670,12 @@ int app_rhizome_add_file(int argc, char **argv, struct command_line_option *o)
 				 1, // int checkFileP
 				 1 // int signP
    );
-  if (ret == -1) {
+  if (ret == -1)
    return WHY("Manifest not added to Rhizome database");
-  } else {
-    /* If successfully added, overwrite the manifest file so that the Java component that is
-     * invoking this command can read it to obtain feedback on the result. */
-    if (manifestpath[0] && rhizome_write_manifest_file(mout, manifestpath) == -1) {
-      ret = WHY("Could not overwrite manifest file.");
-    }
-  }
+  /* If successfully added, overwrite the manifest file so that the Java component that is
+     invoking this command can read it to obtain feedback on the result. */
+  if (manifestpath[0] && rhizome_write_manifest_file(mout, manifestpath) == -1)
+    ret = WHY("Could not overwrite manifest file.");
  rhizome_manifest_free(m);
  if (mout != m)
    rhizome_manifest_free(mout);
@ -832,3 +829,4 @@ command_line_option command_line_options[]={
   "Set the DID for the specified SID.  Optionally supply PIN to unlock the SID record in the keyring."},
  {NULL,{NULL}}
 };
+
--- a/rhizome.c
+++ b/rhizome.c
@ -77,6 +77,11 @@ int rhizome_bundle_import(rhizome_manifest *m_in, rhizome_manifest **m_out, char

 /* Add a manifest/payload pair ("bundle") to the rhizome data store.

+   Returns:
+     0 if successful
+     2 if a duplicate is already in the store (same name, version and filehash)
+     -1 on error or failure
+
   Fills in any missing manifest fields (generating a new, random manifest ID if necessary),
   optionally performs consistency checks (see below), adds the manifest to the given groups (for
   which private keys must be held), optionally signs it, and inserts the manifest and payload into
@ -197,6 +202,10 @@ int rhizome_add_manifest(rhizome_manifest *m_in,
    rhizome_manifest_set_ll(m_in, "last_byte", m_in->fileLength);
  }

+  /* Make sure the manifest structure contains the version number, which may legitimately be -1 if
+     the caller did not provide a version. */
+  m_in->version = rhizome_manifest_get_ll(m_in, "version");
+
  /* Check if a manifest is already stored for the same payload with the same details.
     This catches the case of "dna rhizome add file <filename>" on the same file more than once.
     (Debounce!) */
@ -205,40 +214,37 @@ int rhizome_add_manifest(rhizome_manifest *m_in,
    return WHY("Errors encountered searching for duplicate manifest");
  if (dupm) {
    if (debug & DEBUG_RHIZOME)
-      fprintf(stderr, "Not adding manifest for payload name=\"%s\" hexhash=%s - duplicate found in rhizome store\n", name, m_in->fileHexHash);
-#if 0
-    /* TODO Upgrade the version of the duplicate? */
-    long long version = rhizome_manifest_get_ll(m_in, "version");
-    long long dupversion = rhizome_manifest_get_ll(dupm, "version");
-    if (version > dupversion) {
-      rhizome_manifest_set_ll(dupm, "version", version);
-      ...
-    }
-#endif
+      fprintf(stderr, "Found duplicate payload: name=\"%s\" version=%llu hexhash=%s -- not adding\n", name, dupm->version, dupm->fileHexHash);
+    /* If the caller wants the duplicate manifest, it must be finalised, otherwise discarded. */
    if (m_out) {
-      /* Finish completing the manifest */
      if (rhizome_manifest_finalise(dupm, 0))
 	return WHY("Failed to finalise manifest.\n");
      *m_out = dupm;
    }
    else
      rhizome_manifest_free(dupm);
-    return 0;
+    return 2;
  }

  /* Supply manifest version number if missing, so we can do the version check below */
-  if (rhizome_manifest_get(m_in, "version", NULL, 0) == NULL) {
-    rhizome_manifest_set_ll(m_in, "version", overlay_gettime_ms());
+  if (m_in->version == -1) {
+    m_in->version = overlay_gettime_ms();
+    rhizome_manifest_set_ll(m_in, "version", m_in->version);
  }

  /* If the manifest already has an ID */
  char *id = NULL;
  if ((id = rhizome_manifest_get(m_in, "id", NULL, 0))) {
-    /* Discard the new manifest it is older than the most recent known version with the same ID */
+    /* Discard the new manifest unless it is newer than the most recent known version with the same ID */
    long long storedversion = sqlite_exec_int64("SELECT version from manifests where id='%s';", id);
-    if (storedversion > rhizome_manifest_get_ll(m_in, "version")) {
+    if (debug & DEBUG_RHIZOME)
+      fprintf(stderr, "Found existing version=%lld, new version=%lld\n", storedversion, m_in->version);
+    if (m_in->version < storedversion) {
      return WHY("Newer version exists");
    }
+    if (m_in->version == storedversion) {
+      return WHY("Same version exists");
+    }
    /* Check if we know its private key */
    rhizome_hex_to_bytes(id, m_in->cryptoSignPublic, crypto_sign_edwards25519sha512batch_PUBLICKEYBYTES*2); 
    if (!rhizome_find_keypair_bytes(m_in->cryptoSignPublic, m_in->cryptoSignSecret))
--- a/rhizome_database.c
+++ b/rhizome_database.c
@ -142,7 +142,7 @@ long long sqlite_exec_int64(char *sqlformat,...)
 	 sqlite3_finalize(statement);
 	 return -1;
       }
-       long long result= sqlite3_column_int(statement,0);
+       long long result= sqlite3_column_int64(statement,0);
       sqlite3_finalize(statement);
       return result;
     }
@ -548,10 +548,10 @@ int rhizome_list_manifests(int limit, int offset)
      rhizome_manifest *m = rhizome_read_manifest_file(manifestblob, manifestblobsize, 0);
      const char *name = rhizome_manifest_get(m, "name", NULL, 0);
      long long date = rhizome_manifest_get_ll(m, "date");
-      printf("fileid=%s:manifestid=%s:version=%d:inserttime=%lld:length=%u:datavalid=%u:date=%lld:name=%s\n",
+      printf("fileid=%s:manifestid=%s:version=%lld:inserttime=%lld:length=%u:datavalid=%u:date=%lld:name=%s\n",
 	  sqlite3_column_text(statement, 0),
 	  sqlite3_column_text(statement, 3),
-	  sqlite3_column_int(statement, 5),
+	  (long long) sqlite3_column_int64(statement, 5),
 	  (long long) sqlite3_column_int64(statement, 6),
 	  sqlite3_column_int(statement, 1),
 	  sqlite3_column_int(statement, 2),
@ -768,7 +768,10 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
  if (!name)
      return WHY("Manifest has no name");
  char sqlcmd[1024];
-  int n = snprintf(sqlcmd, sizeof(sqlcmd), "SELECT manifests.id, manifests.manifest FROM filemanifests, manifests WHERE filemanifests.fileid = ? AND filemanifests.manifestid = manifests.id");
+  int n = snprintf(sqlcmd, sizeof(sqlcmd),
+      "SELECT manifests.id, manifests.manifest, manifests.version FROM filemanifests, manifests"
+      " WHERE filemanifests.manifestid = manifests.id AND filemanifests.fileid = ?"
+    );
  if (n >= sizeof(sqlcmd))
    return WHY("SQL command too long");
  int ret = 0;
@ -779,13 +782,15 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
  } else {
    if (debug & DEBUG_RHIZOME) fprintf(stderr, "fileHexHash = \"%s\"\n", m->fileHexHash);
    sqlite3_bind_text(statement, 1, m->fileHexHash, -1, SQLITE_STATIC);
+    sqlite3_bind_int64(statement, 2, m->version);
    size_t rows = 0;
    while (sqlite3_step(statement) == SQLITE_ROW) {
      ++rows;
      if (debug & DEBUG_RHIZOME) fprintf(stderr, "Row %d\n", rows);
-      if (!(   sqlite3_column_count(statement) == 2
+      if (!(   sqlite3_column_count(statement) == 3
 	    && sqlite3_column_type(statement, 0) == SQLITE_TEXT
 	    && sqlite3_column_type(statement, 1) == SQLITE_BLOB
+	    && sqlite3_column_type(statement, 2) == SQLITE_INTEGER
      )) { 
 	ret = WHY("Incorrect statement columns");
 	break;
@ -798,19 +803,25 @@ int rhizome_find_duplicate(const rhizome_manifest *m, rhizome_manifest **found)
      }
      const char *manifestblob = (char *) sqlite3_column_blob(statement, 1);
      size_t manifestblobsize = sqlite3_column_bytes(statement, 1); // must call after sqlite3_column_blob()
+      long long manifestversion = sqlite3_column_int64(statement, 2);
      rhizome_manifest *mq = rhizome_read_manifest_file(manifestblob, manifestblobsize, 0);
      const char *nameq = rhizome_manifest_get(mq, "name", NULL, 0);
+      long long versionq = rhizome_manifest_get_ll(mq, "version");
      const char *filehashq = rhizome_manifest_get(mq, "filehash", NULL, 0);
      long long lengthq = rhizome_manifest_get_ll(mq, "filesize");
-      if (debug & DEBUG_RHIZOME) fprintf(stderr, "Consider manifest.id=%s manifest.name=\"%s\"\n", manifestid, nameq);
+      if (debug & DEBUG_RHIZOME)
+	fprintf(stderr, "Consider manifest.id=%s manifest.name=\"%s\" manifest.version=%lld\n", manifestid, nameq, versionq);
      /* No need to compare "filehash" or "filesize" here, but we do so as a precaution if present */
      if (  nameq && !strcmp(nameq, name)
-	 && (!filehashq || strncmp(filehashq, m->fileHexHash, SHA512_DIGEST_STRING_LENGTH) == 0)
+	 && (versionq == -1 || versionq == manifestversion) // consistency check
+	 && (m->version == -1 || manifestversion == m->version)
 	 && (lengthq == -1 || lengthq == m->fileLength)
+	 && (!filehashq || strncmp(filehashq, m->fileHexHash, SHA512_DIGEST_STRING_LENGTH) == 0)
      ) {
 	memcpy(mq->fileHexHash, m->fileHexHash, SHA512_DIGEST_STRING_LENGTH);
 	mq->fileHashedP = 1;
 	mq->fileLength = m->fileLength;
+	mq->version = manifestversion;
 	*found = mq;
 	ret = 1;
 	if (debug & DEBUG_RHIZOME) fprintf(stderr, "found\n");
--- a/tests/dna_rhizome
+++ b/tests/dna_rhizome
@ -45,12 +45,41 @@ assert_rhizome_list() {
   done
 }

+assert_manifest_newer() {
+   local manifest1="$1"
+   local manifest2="$2"
+   # The new manifest must have a higher version than the original.
+   extract_manifest_version oldversion "$manifest1"
+   extract_manifest_version newversion "$manifest2"
+   assert [ $newversion -gt $oldversion ]
+   # The new manifest must have a different filehash from the original.
+   extract_manifest_filehash oldfilehash "$manifest1"
+   extract_manifest_filehash newfilehash "$manifest2"
+   assert [ $oldfilehash != $newfilehash ]
+}
+
 strip_signatures() {
   for file; do
      cat -v "$file" | sed -e '/^^@/,$d' >"tmp.$file" && mv -f "tmp.$file" "$file"
   done
 }

+extract_manifest_version() {
+   local _var="$1"
+   local _manifestfile="$2"
+   local _version=$(sed -n -e '/^version=[0-9]\+$/s/^version=//p' "$_manifestfile")
+   assert --message="$_manifestfile contains valid 'version=' line" [ -n "$_version" ]
+   [ -n "$_var" ] && eval $_var=$_version
+}
+
+extract_manifest_filehash() {
+   local _var="$1"
+   local _manifestfile="$2"
+   local _filehash=$(sed -n -e '/^filehash=[0-9a-fA-F]\+$/s/^filehash=//p' "$_manifestfile")
+   assert --message="$_manifestfile contains valid 'filehash=' line" [ -n "$_filehash" ]
+   eval $_var=$_filehash
+}
+
 doc_InitialEmptyList="Initial list is empty"
 setup_InitialEmptyList() {
   setup_dna_rhizome
@ -129,7 +158,7 @@ test_AddThenList() {
   assert_rhizome_list file1 file2
 }

-doc_AddDuplicate="Add same file detects duplicate"
+doc_AddDuplicate="Add same manifest detects duplicate"
 setup_AddDuplicate() {
   setup_dna_rhizome
   assert_rhizome_list
@ -147,13 +176,13 @@ test_AddDuplicate() {
   # Add first file again - nothing should change in its manifests, and it
   # should appear that the add command succeeded (with perhaps some grumbling
   # on stderr).
-   executeOk $dna rhizome add file file1 file1.manifestA
+   execute --exit-status=2 $dna rhizome add file file1 file1.manifestA
   assert [ -s file1.manifestA ]
   assert_rhizome_list file1 file2
   strip_signatures file1.manifest file1.manifestA
   assert diff file1.manifest file1.manifestA
   # Repeat for second file.
-   executeOk $dna rhizome add file file2 file2.manifestA
+   execute --exit-status=2 $dna rhizome add file file2 file2.manifestA
   assert [ -s file2.manifestA ]
   assert_rhizome_list file1 file2
   strip_signatures file2.manifest file2.manifestA
@ -175,42 +204,55 @@ test_AddMismatched() {
   assert_rhizome_list file1 file2
 }

-doc_AddUpdateNoVersion="Add new payload to existing manifest without new version"
-setup_AddUpdateNoVersion() {
+doc_AddUpdateSameVersion="Add new payload to existing manifest with same version"
+setup_AddUpdateSameVersion() {
   setup_AddDuplicate
   cp file1.manifest file1_2.manifest
   strip_signatures file1_2.manifest
   sed -i -e '/^date=/d' -e '/^filehash=/d' -e '/^filesize=/d' file1_2.manifest
   assertGrep --matches=0 file1_2.manifest '^filehash='
-   assertGrep file1_2.manifest '^version='
+   extract_manifest_version '' file1_2.manifest # asserts has version= line
+   assertGrep file1_2.manifest '^id='
   cp file1_2.manifest file1_2.manifest.orig
 }
-test_AddUpdateNoVersion() {
-   tfw_cat file1_2.manifest
+test_AddUpdateSameVersion() {
+   tfw_cat -v file1_2.manifest
   execute $dna rhizome add file file1_2 file1_2.manifest
-   assertExitStatus '!=' 0
+   assertExitStatus --stderr '!=' 0
+   tfw_cat -v file1_2.manifest
   assert cmp file1_2.manifest file1_2.manifest.orig
   # And rhizome store should be unchanged.
   assert_rhizome_list file1 file2
 }

-doc_AddUpdate="Add new version and payload to existing manifest"
-setup_AddUpdate() {
-   setup_AddUpdateNoVersion
-   version=$(sed -n -e '/^version=/s///p' file1.manifest)
+doc_AddUpdateNewVersion="Add new payload to existing manifest with new version"
+setup_AddUpdateNewVersion() {
+   setup_AddUpdateSameVersion
+   extract_manifest_version version file1_2.manifest
   let version=version+1
   sed -i -e "/^version=/s/=.*/=$version/" file1_2.manifest
   assertGrep --matches=1 file1_2.manifest "^version=$version$"
 }
-test_AddUpdate() {
+test_AddUpdateNewVersion() {
   tfw_cat file1_2.manifest
   executeOk $dna rhizome add file file1_2 file1_2.manifest
-   # The new manifest must now have a different filehash from the original.
-   filehash1=$(sed -n -e '/^filehash=/s///p' file1.manifest)
-   filehash1_2=$(sed -n -e '/^filehash=/s///p' file1_2.manifest)
-   assert [ -n "$filehash1" ]
-   assert [ -n "$filehash1_2" ]
-   assert [ "$filehash1" != "$filehash1_2" ]
+   assert_manifest_newer file1.manifest file1_2.manifest
+   # Rhizome store contents reflect new payload.
+   mv -f file1_2.manifest file1.manifest
+   assert_rhizome_list file1 file2
+}
+
+doc_AddUpdateAutoVersion="Add new payload to existing manifest with automatic version"
+setup_AddUpdateAutoVersion() {
+   setup_AddUpdateSameVersion
+   sed -i -e '/^version=/d' file1_2.manifest
+   assertGrep --matches=0 file1_2.manifest '^version='
+}
+test_AddUpdateAutoVersion() {
+   tfw_cat file1_2.manifest
+   sleep 0.001 # Ensure that at least one millisecond has elapsed
+   executeOk $dna rhizome add file file1_2 file1_2.manifest
+   assert_manifest_newer file1.manifest file1_2.manifest
   # Rhizome store contents reflect new payload.
   mv -f file1_2.manifest file1.manifest
   assert_rhizome_list file1 file2