From 132cfd6feccd38c05dcc8ea8829b28450f1aace2 Mon Sep 17 00:00:00 2001
From: gardners <paul@servalproject.org>
Date: Wed, 3 Oct 2012 07:06:06 +0930
Subject: [PATCH] Fixed bug in rhizome_store_file(), where it tried to open the
 associated file before checking if it was already in the database. Rhizome
 Direct can supply a manifest without associated file if the file is already
 in the database, and so it was breaking. Also removed "assert
 bundle_received_by"'s from rhizome direct pull and sync tests because they
 are not needed, and were failing because the same file contents was used for
 the files being exchanged, and so file storage was not occurring, and thus
 the log message being looked for was not being produced. Push, pull and sync
 tests in rhizomeprotocol now pass, leaving only two tests in error.  #9

---
 rhizome_database.c    | 59 ++++++++++++++++++++++++-------------------
 tests/rhizomeprotocol |  4 ---
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/rhizome_database.c b/rhizome_database.c
index 5368c760..966fcf50 100644
--- a/rhizome_database.c
+++ b/rhizome_database.c
@@ -643,15 +643,20 @@ int rhizome_store_bundle(rhizome_manifest *m)
   unsigned char bar[RHIZOME_BAR_BYTES];
   rhizome_manifest_to_bar(m,bar);
 
-  /* Store the file */
+  /* Store the file (but not if it is already in the database) */
+  // TODO encrypted payloads - pass encryption key here. Filehash should be of
+  // encrypted data.
+  // We should add the file in the same transaction, but closing the blob seems
+  // to cause some issues.
   char filehash[RHIZOME_FILEHASH_STRLEN + 1];
   if (m->fileLength > 0) {
     if (!m->fileHashedP)
       return WHY("Manifest payload hash unknown");
     strncpy(filehash, m->fileHexHash, sizeof filehash);
     str_toupper_inplace(filehash);
-    // TODO encrypted payloads - pass encryption key here
-    // We should add the file in the same transaction, but closing the blob seems to cause some issues.
+
+    /* rhizome_store_file() checks if it is already in the database, so we just
+       call it normally. */
     if (rhizome_store_file(m, NULL))
       return WHY("Could not store file");
   } else {
@@ -852,6 +857,31 @@ int rhizome_store_file(rhizome_manifest *m,const unsigned char *key)
   if (!m->fileHashedP)
     return WHY("Cannot store bundle file until it has been hashed");
 
+  /* See if the file is already stored, and if so, don't bother storing it again.
+     Do this check BEFORE trying to open the associated file, because if the caller
+     has received a manifest and checked that it exists in the database, it may 
+     (sensibly) elect not supply the file. Rhizome Direct does this. */
+  long long count = 0;
+  if (sqlite_exec_int64(&count, "SELECT COUNT(*) FROM FILES WHERE id='%s' AND datavalid<>0;", hash) < 1) {
+    WHY("Failed to count stored files");
+    goto error;
+  }
+  if (count >= 1) {
+    /* File is already stored, so just update the highestPriority field if required. */
+    long long storedPriority = -1;
+    if (sqlite_exec_int64(&storedPriority, "SELECT highestPriority FROM FILES WHERE id='%s' AND datavalid!=0", hash) == -1) {
+      WHY("Failed to select highest priority");
+      goto error;
+    }
+    if (storedPriority<priority) {
+      if (sqlite_exec_void("UPDATE FILES SET highestPriority=%d WHERE id='%s';", priority, hash) == -1) {
+	WHY("SQLite failed to update highestPriority field for stored file.");
+	goto error;
+      }
+    }
+    return 0;
+  }
+
   int fd=open(file,O_RDONLY);
   if (fd == -1) {
     WHY_perror("open");
@@ -884,29 +914,6 @@ int rhizome_store_file(rhizome_manifest *m,const unsigned char *key)
     goto error;
   }
 
-  /* See if the file is already stored, and if so, don't bother storing it again */
-  long long count = 0;
-  if (sqlite_exec_int64(&count, "SELECT COUNT(*) FROM FILES WHERE id='%s' AND datavalid<>0;", hash) < 1) {
-    WHY("Failed to count stored files");
-    goto error;
-  }
-  if (count >= 1) {
-    /* File is already stored, so just update the highestPriority field if required. */
-    long long storedPriority = -1;
-    if (sqlite_exec_int64(&storedPriority, "SELECT highestPriority FROM FILES WHERE id='%s' AND datavalid!=0", hash) == -1) {
-      WHY("Failed to select highest priority");
-      goto error;
-    }
-    if (storedPriority<priority) {
-      if (sqlite_exec_void("UPDATE FILES SET highestPriority=%d WHERE id='%s';", priority, hash) == -1) {
-	WHY("SQLite failed to update highestPriority field for stored file.");
-	goto error;
-      }
-    }
-    close(fd);
-    return 0;
-  }
-
   /* Okay, so there are no records that match, but we should delete any half-baked record (with datavalid=0) so that the insert below doesn't fail.
    Don't worry about the return result, since it might not delete any records. */
   sqlite_exec_void("DELETE FROM FILES WHERE datavalid=0;");
diff --git a/tests/rhizomeprotocol b/tests/rhizomeprotocol
index 187ab16c..40ee797e 100755
--- a/tests/rhizomeprotocol
+++ b/tests/rhizomeprotocol
@@ -303,8 +303,6 @@ setup_Pull() {
 }
 test_Pull() {
    executeOk_servald rhizome direct pull
-   assert bundle_received_by $BID1 $VERSION1 +B
-   assert ! bundle_received_by $BID2 $VERSION2 +A
    set_instance +A
    executeOk_servald rhizome list ''
    assert_rhizome_list file1
@@ -320,8 +318,6 @@ setup_Sync() {
 }
 test_Sync() {
    executeOk_servald rhizome direct sync
-   assert bundle_received_by $BID2 $VERSION2 +A
-   assert bundle_received_by $BID1 $VERSION1 +B
    set_instance +A
    executeOk_servald rhizome list ''
    assert_rhizome_list file1 file2