Refactor and improve "journal append"

Distinguish between fatal and user-supplied-input errors, return
error descriptive text in a strbuf to allow dynamic content, apply
user-supplied field assgnments/deletions _after_ copying existing
manifest fields, use exit status 4 (invalid manifest) when applying
journal append to a non-journal or vice versa

httpd.h

@@ -105,7 +105,7 @@ typedef struct httpd_request
      */
     struct {
       // If this is really a (journal) append request
-      bool_t is_append;
+      bool_t appending;
       // Which part is currently being received
       const char *current_part;
       // Which parts have already been received

rhizome.c

@@ -117,24 +117,44 @@ int rhizome_fetch_delay_ms()
  *   file), or can be NULL.  If not NULL, then the file's name will be used to fill in the 'name'
  *   field of the manifest if it was not explicitly supplied in 'm' or in the existing manifest.
  *
+ * - 'nassignments' and 'assignments' describe an array of field assignments that override the
+ *   fields supplied in 'm' and also the fields in any existing manifest with the same Bundle Id.
+ *
+ * - 'reason' may either be NULL or points to a strbuf to which descriptive text is appended if the
+ *   manifest creation fails.
+ *
  * If the add is successful, modifies '*mout' to point to the constructed Manifest, which might be
- * 'm' or might be another manifest, and returns NULL.  It is the caller's responsibility to free
+ * 'm' or might be another manifest, and returns 0.  It is the caller's responsibility to free
  * '*mout'.
  *
- * If the add fails for any reason, returns a pointer to a nul-terminated text string that describes
- * the reason, and does not alter '*mout'.
+ * If the add fails because of invalid field settings that violate Rhizome semantics (eg, a missing
+ * mandatory field, a malformed field name or value), then if 'reason' is not NULL, appends a text
+ * string to the 'reason' strbuf that describes the cause of the failure, does not alter '*mout',
+ * and returns 1.
+ *
+ * If the add fails because of a recoverable error (eg, database locking) then if 'reason' is not
+ * NULL, appends a text string to the 'reason' strbuf that describes the cause of the failure, does
+ * not alter '*mout', and returns 2.
+ *
+ * If the add fails because of an unrecoverable error (eg, out of memory, i/o failure)
+ * then if 'reason' is not NULL, appends a text string to the 'reason' strbuf that describes the
+ * cause of the failure, does not alter '*mout', and returns -1.
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-const char * rhizome_bundle_add_file(int appending,
-				     rhizome_manifest *m,
-				     rhizome_manifest **mout,
-				     const rhizome_bk_t *bsk,
-				     const sid_t *author,
-				     const char *file_path
-				    )
+enum rhizome_add_file_result rhizome_manifest_add_file(int appending,
+                                                       rhizome_manifest *m,
+                                                       rhizome_manifest **mout,
+                                                       const rhizome_bk_t *bsk,
+                                                       const sid_t *author,
+                                                       const char *file_path,
+                                                       unsigned nassignments,
+                                                       const struct rhizome_manifest_field_assignment *assignments,
+                                                       strbuf reason
+                                                      )
 {
-  const char *reason = NULL;
+  const char *cause = NULL;
+  enum rhizome_add_file_result result = RHIZOME_ADD_FILE_ERROR;
   rhizome_manifest *existing_manifest = NULL;
   rhizome_manifest *new_manifest = NULL;
   assert(m != NULL);
@@ -145,7 +165,7 @@ const char * rhizome_bundle_add_file(int appending,
     if (config.debug.rhizome)
       DEBUGF("Reading manifest from database: id=%s", alloca_tohex_rhizome_bid_t(m->cryptoSignPublic));
     if ((existing_manifest = rhizome_new_manifest()) == NULL) {
-      WHY(reason = "Manifest struct could not be allocated");
+      WHY(cause = "Manifest struct could not be allocated");
       goto error;
     }
     enum rhizome_bundle_status status = rhizome_retrieve_manifest(&m->cryptoSignPublic, existing_manifest);
@@ -160,22 +180,23 @@ const char * rhizome_bundle_add_file(int appending,
       // Found a manifest with the same bundle ID.  Unset its 'version', 'filesize' and 'filehash'
       // fields unless appending, then overwrite it with the supplied manifest.
       if (!appending) {
-	rhizome_manifest_del_version(existing_manifest);
-	rhizome_manifest_del_filesize(existing_manifest);
-	rhizome_manifest_del_filehash(existing_manifest);
+        rhizome_manifest_del_version(existing_manifest);
+        rhizome_manifest_del_filesize(existing_manifest);
+        rhizome_manifest_del_filehash(existing_manifest);
       }
       if (rhizome_manifest_overwrite(existing_manifest, m) == -1) {
-	WHY(reason = "Existing manifest could not be overwritten");
-	goto error;
+        WHY(cause = "Existing manifest could not be overwritten");
+        goto error;
       }
       new_manifest = existing_manifest;
       existing_manifest = NULL;
       break;
     case RHIZOME_BUNDLE_STATUS_BUSY:
-      WHY(reason = "Existing manifest not retrieved due to Rhizome store locking");
+      WARN(cause = "Existing manifest not retrieved due to Rhizome store locking");
+      result = RHIZOME_ADD_FILE_BUSY;
       goto error;
     case RHIZOME_BUNDLE_STATUS_ERROR:
-      WHY(reason = "Error retrieving existing manifest from Rhizome store");
+      WHY(cause = "Error retrieving existing manifest from Rhizome store");
       goto error;
     case RHIZOME_BUNDLE_STATUS_DUPLICATE:
     case RHIZOME_BUNDLE_STATUS_OLD:
@@ -194,26 +215,87 @@ const char * rhizome_bundle_add_file(int appending,
     // succeed.
     if (appending) {
       if (new_manifest->filesize == RHIZOME_SIZE_UNSET)
-	rhizome_manifest_set_filesize(new_manifest, 0);
+        rhizome_manifest_set_filesize(new_manifest, 0);
       if (new_manifest->tail == RHIZOME_SIZE_UNSET)
-	rhizome_manifest_set_tail(new_manifest, 0);
+        rhizome_manifest_set_tail(new_manifest, 0);
     }
   }
-  if (appending && !new_manifest->is_journal){
-    // TODO: return a special status code for this case
-    WHY(reason = "Cannot append to a non-journal");
+  // Apply the field assignments, overriding the existing manifest fields.
+  if (nassignments) {
+    unsigned i;
+    for (i = 0; i != nassignments; ++i) {
+      const struct rhizome_manifest_field_assignment *asg = &assignments[i];
+      rhizome_manifest_remove_field(new_manifest, asg->label, asg->labellen);
+      if (asg->value) {
+        const char *label = alloca_strndup(asg->label, asg->labellen);
+        enum rhizome_manifest_parse_status status = rhizome_manifest_parse_field(new_manifest, asg->label, asg->labellen, asg->value, asg->valuelen);
+        int status_ok = 0;
+        switch (status) {
+          case RHIZOME_MANIFEST_ERROR:
+            WHYF("Fatal error updating manifest field");
+            if (reason)
+              strbuf_sprintf(reason, "Fatal error updating manifest field: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            goto error;
+          case RHIZOME_MANIFEST_OK:
+            status_ok = 1;
+            break;
+          case RHIZOME_MANIFEST_SYNTAX_ERROR:
+            if (config.debug.rhizome)
+              DEBUGF("Manifest syntax error: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            if (reason)
+              strbuf_sprintf(reason, "Manifest syntax error: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            result = RHIZOME_ADD_FILE_INVALID;
+            goto error;
+          case RHIZOME_MANIFEST_DUPLICATE_FIELD:
+            // We already deleted the field, so if this happens, its a nasty bug
+            FATALF("Duplicate field should not occur: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+          case RHIZOME_MANIFEST_INVALID:
+            if (config.debug.rhizome)
+              DEBUGF("Manifest invalid field: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            if (reason)
+              strbuf_sprintf(reason, "Manifest invalid field: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            result = RHIZOME_ADD_FILE_INVALID;
+            goto error;
+          case RHIZOME_MANIFEST_MALFORMED:
+            if (config.debug.rhizome)
+              DEBUGF("Manifest malformed field: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            if (reason)
+              strbuf_sprintf(reason, "Manifest malformed field: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            result = RHIZOME_ADD_FILE_INVALID;
+            goto error;
+          case RHIZOME_MANIFEST_OVERFLOW:
+            if (config.debug.rhizome)
+              DEBUGF("Too many fields in manifest at: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            if (reason)
+              strbuf_sprintf(reason, "Too many fields in manifest at: %s=%s", label, alloca_toprint(-1, asg->value, asg->valuelen));
+            result = RHIZOME_ADD_FILE_INVALID;
+            goto error;
+        }
+        if (!status_ok)
+          FATALF("status = %d", status);
+      }
+    }
+  }
+  if (appending && !new_manifest->is_journal) {
+    cause = "Cannot append to a non-journal";
+    if (config.debug.rhizome)
+      DEBUG(cause);
+    result = RHIZOME_ADD_FILE_REQUIRES_JOURNAL;
     goto error;
   }
   if (!appending && new_manifest->is_journal) {
-    // TODO: return a special status code for this case
-    WHY(reason = "Cannot add a journal bundle (use append instead)");
+    cause = "Cannot add a journal bundle (use append instead)";
+    if (config.debug.rhizome)
+      DEBUG(cause);
+    result = RHIZOME_ADD_FILE_INVALID_FOR_JOURNAL;
     goto error;
   }
   if (bsk) {
     if (new_manifest->has_id) {
       if (!rhizome_apply_bundle_secret(new_manifest, bsk)) {
-	WHY(reason = "Supplied bundle secret does not match Bundle Id");
-	goto error;
+        WHY(cause = "Supplied bundle secret does not match Bundle Id");
+        result = RHIZOME_ADD_FILE_WRONG_SECRET;
+        goto error;
       }
     } else {
       rhizome_new_bundle_from_secret(new_manifest, bsk);
@@ -221,18 +303,23 @@ const char * rhizome_bundle_add_file(int appending,
   }
   // TODO: one day there will be no default service, but for now if no service
   // is specified, it defaults to 'file' (file sharing).
-  if (m->service == NULL)
+  if (new_manifest->service == NULL) {
+    WARNF("Manifest 'service' field not supplied - setting to '%s'", RHIZOME_SERVICE_FILE);
     rhizome_manifest_set_service(new_manifest, RHIZOME_SERVICE_FILE);
-  if ((reason = rhizome_fill_manifest(new_manifest, file_path, author ? author : NULL)) != NULL)
+  }
+  if ((cause = rhizome_fill_manifest(new_manifest, file_path, author ? author : NULL)) != NULL)
     goto error;
   *mout = new_manifest;
-  return NULL;
+  return RHIZOME_ADD_FILE_OK;
 error:
+  assert(result != RHIZOME_ADD_FILE_OK);
+  if (cause && reason)
+    strbuf_puts(reason, cause);
   if (new_manifest && new_manifest != m && new_manifest != existing_manifest)
     rhizome_manifest_free(new_manifest);
   if (existing_manifest)
     rhizome_manifest_free(existing_manifest);
-  return reason;
+  return result;
 }
 
 /* Import a bundle from a pair of files, one containing the manifest and the optional other

rhizome.h

@@ -301,6 +301,10 @@ void _rhizome_manifest_set_inserttime(struct __sourceloc, rhizome_manifest *, ti
 void _rhizome_manifest_set_author(struct __sourceloc, rhizome_manifest *, const sid_t *);
 void _rhizome_manifest_del_author(struct __sourceloc, rhizome_manifest *);
 
+#define rhizome_manifest_overwrite(dstm,srcm)   _rhizome_manifest_overwrite(__WHENCE__,(dstm),(srcm))
+
+int _rhizome_manifest_overwrite(struct __sourceloc, rhizome_manifest *m, const rhizome_manifest *srcm);
+
 enum rhizome_manifest_parse_status {
     RHIZOME_MANIFEST_ERROR = -1,          // unrecoverable error while constructing manifest
     RHIZOME_MANIFEST_OK = 0,              // field parsed ok; manifest updated
@@ -311,7 +315,21 @@ enum rhizome_manifest_parse_status {
     RHIZOME_MANIFEST_OVERFLOW = 5,        // maximum field count exceeded
 };
 
-int rhizome_manifest_overwrite(rhizome_manifest *m, const rhizome_manifest *srcm);
+/* This structure represents a manifest field assignment as received by the API
+ * operations "add file" or "journal append" or any other operation that takes an
+ * existing manifest and modifies it to produce a new one.
+ *
+ * The 'label' and 'value' strings are pointer-length instead of NUL terminated,
+ * to allow them to refer directly to fragments of an existing, larger text
+ * without requiring the caller to allocate new strings to hold them.
+ */
+struct rhizome_manifest_field_assignment {
+    const char *label;
+    size_t labellen;
+    const char *value;
+    size_t valuelen;
+};
+
 int rhizome_manifest_field_label_is_valid(const char *field_label, size_t field_label_len);
 int rhizome_manifest_field_value_is_valid(const char *field_value, size_t field_value_len);
 enum rhizome_manifest_parse_status
@@ -432,7 +450,26 @@ rhizome_manifest *_rhizome_new_manifest(struct __sourceloc);
 
 int rhizome_store_manifest(rhizome_manifest *m);
 int rhizome_store_file(rhizome_manifest *m,const unsigned char *key);
-const char * rhizome_bundle_add_file(int appending, rhizome_manifest *m, rhizome_manifest **mout, const rhizome_bk_t *bsk, const sid_t *author, const char *file_path);
+
+enum rhizome_add_file_result {
+    RHIZOME_ADD_FILE_ERROR = -1,
+    RHIZOME_ADD_FILE_OK = 0, // manifest created successfully
+    RHIZOME_ADD_FILE_INVALID, // manifest not created due to invalid input
+    RHIZOME_ADD_FILE_BUSY, // manifest not created because database busy
+    RHIZOME_ADD_FILE_REQUIRES_JOURNAL, // operation is only valid for a journal
+    RHIZOME_ADD_FILE_INVALID_FOR_JOURNAL, // operation is not valid for a journal
+    RHIZOME_ADD_FILE_WRONG_SECRET, // incorrect bundle secret supplied
+};
+enum rhizome_add_file_result rhizome_manifest_add_file(int appending,
+			      rhizome_manifest *m,
+			      rhizome_manifest **mout,
+			      const rhizome_bk_t *bsk,
+			      const sid_t *author,
+			      const char *file_path,
+			      unsigned nassignments,
+			      const struct rhizome_manifest_field_assignment *assignments,
+			      strbuf reason
+			     );
 int rhizome_bundle_import_files(rhizome_manifest *m, rhizome_manifest **m_out, const char *manifest_path, const char *filepath);
 
 int rhizome_manifest_set_name_from_path(rhizome_manifest *m, const char *filepath);

rhizome_bundle.c

@@ -1496,7 +1496,7 @@ int rhizome_manifest_set_name_from_path(rhizome_manifest *m, const char *filepat
  *  - if service is file, then use the payload file's basename for "name"
  *
  * Return NULL if successful, otherwise a pointer to a static text string describing the reason for
- * the failure.
+ * the failure (always an internal/unrecoverable error).
  */
 const char * rhizome_fill_manifest(rhizome_manifest *m, const char *filepath, const sid_t *authorSidp)
 {
@@ -1563,7 +1563,7 @@ const char * rhizome_fill_manifest(rhizome_manifest *m, const char *filepath, co
     return reason;
   }
   if (config.debug.rhizome)
-    DEBUGF("manifest service=%s", m->service);
+    DEBUGF("manifest contains service=%s", m->service);
 
   /* Fill in 'date' field to current time unless already set.
    */

rhizome_cli.c

@@ -142,18 +142,12 @@ static int app_rhizome_add_file(const struct cli_parsed *parsed, struct cli_cont
     return WHYF("invalid BSK: \"%s\"", bsktext);
   
   unsigned nfields = (parsed->varargi == -1) ? 0 : parsed->argc - (unsigned)parsed->varargi;
-  struct field {
-    const char *label;
-    size_t labellen;
-    const char *value;
-    size_t valuelen;
-  }
-    fields[nfields];
+  struct rhizome_manifest_field_assignment fields[nfields];
   if (nfields) {
     assert(parsed->varargi >= 0);
     unsigned i;
     for (i = 0; i < nfields; ++i) {
-      struct field *field = &fields[i];
+      struct rhizome_manifest_field_assignment *field = &fields[i];
       unsigned n = (unsigned)parsed->varargi + i;
       assert(n < parsed->argc);
       const char *arg = parsed->args[n];
@@ -209,42 +203,6 @@ static int app_rhizome_add_file(const struct cli_parsed *parsed, struct cli_cont
       goto finish;
     }
   }
-  if (nfields) {
-    unsigned i;
-    for (i = 0; i != nfields; ++i) {
-      struct field *field = &fields[i];
-      rhizome_manifest_remove_field(m, field->label, field->labellen);
-      if (field->value) {
-	const char *label = alloca_strndup(field->label, field->labellen);
-	enum rhizome_manifest_parse_status status = rhizome_manifest_parse_field(m, field->label, field->labellen, field->value, field->valuelen);
-	int status_ok = 0;
-	switch (status) {
-	  case RHIZOME_MANIFEST_ERROR:
-	    ret = WHY("Fatal error while updating manifest field");
-	    goto finish;
-	  case RHIZOME_MANIFEST_OK:
-	    status_ok = 1;
-	    break;
-	  case RHIZOME_MANIFEST_SYNTAX_ERROR:
-	    ret = WHYF("Manifest syntax error: %s=%s", label, alloca_toprint(-1, field->value, field->valuelen));
-	    goto finish;
-	  case RHIZOME_MANIFEST_DUPLICATE_FIELD:
-	    abort(); // should not happen, field was removed first
-	  case RHIZOME_MANIFEST_INVALID:
-	    ret = WHYF("Manifest invalid field: %s=%s", label, alloca_toprint(-1, field->value, field->valuelen));
-	    goto finish;
-	  case RHIZOME_MANIFEST_MALFORMED:
-	    ret = WHYF("Manifest malformed field: %s=%s", label, alloca_toprint(-1, field->value, field->valuelen));
-	    goto finish;
-	  case RHIZOME_MANIFEST_OVERFLOW:
-	    ret = WHYF("Too many fields in manifest at: %s=%s", label, alloca_toprint(-1, field->value, field->valuelen));
-	    goto finish;
-	}
-	if (!status_ok)
-	  FATALF("status = %d", status);
-      }
-    }
-  }
 
   /* If a manifest ID (bundle ID) was supplied on the command line, first ensure it does not
    * contradict any manifest ID present in the supplied manifest file, then insert it into the
@@ -262,10 +220,38 @@ static int app_rhizome_add_file(const struct cli_parsed *parsed, struct cli_cont
   /* Create an in-memory manifest for the file being added.
    */
   rhizome_manifest *mout = NULL;
-  if (rhizome_bundle_add_file(appending, m, &mout, bsktext ? &bsk : NULL, authorSidHex ? &authorSid : NULL, filepath) != NULL) {
-    ret = WHY("Cannot create manifest -- not added");
+  enum rhizome_add_file_result result = rhizome_manifest_add_file(appending, m, &mout,
+								  bsktext ? &bsk : NULL,
+								  authorSidHex ? &authorSid : NULL,
+								  filepath,
+								  nfields, fields,
+								  NULL);
+  int result_valid = 0;
+  switch (result) {
+  case RHIZOME_ADD_FILE_ERROR:
+    ret = -1;
+    goto finish;
+  case RHIZOME_ADD_FILE_OK:
+    result_valid = 1;
+    break;
+  case RHIZOME_ADD_FILE_INVALID:
+    ret = RHIZOME_BUNDLE_STATUS_INVALID; // TODO separate enum for CLI return codes
+    goto finish;
+  case RHIZOME_ADD_FILE_BUSY:
+    ret = RHIZOME_BUNDLE_STATUS_BUSY; // TODO separate enum for CLI return codes
+    goto finish;
+  case RHIZOME_ADD_FILE_REQUIRES_JOURNAL:
+    ret = RHIZOME_BUNDLE_STATUS_INVALID; // TODO separate enum for CLI return codes
+    goto finish;
+  case RHIZOME_ADD_FILE_INVALID_FOR_JOURNAL:
+    ret = RHIZOME_BUNDLE_STATUS_INVALID; // TODO separate enum for CLI return codes
+    goto finish;
+  case RHIZOME_ADD_FILE_WRONG_SECRET:
+    ret = RHIZOME_BUNDLE_STATUS_READONLY; // TODO separate enum for CLI return codes
     goto finish;
   }
+  if (!result_valid)
+    FATALF("result = %d", result);
   assert(mout != NULL);
   if (mout != m) {
     rhizome_manifest_free(m);

rhizome_restful.c

@@ -357,7 +357,7 @@ int restful_rhizome_insert(httpd_request *r, const char *remainder)
 
 int restful_rhizome_append(httpd_request *r, const char *remainder)
 {
-  r->u.insert.is_append = 1;
+  r->u.insert.appending = 1;
   return restful_rhizome_insert(r, remainder);
 }
 
@@ -377,32 +377,67 @@ static int insert_make_manifest(httpd_request *r)
 {
   if (!r->u.insert.received_manifest)
     return http_response_form_part(r, "Missing", PART_MANIFEST, NULL, 0);
-  if ((r->manifest = rhizome_new_manifest())) {
-    if (r->u.insert.manifest.length == 0)
-      return 0;
-    assert(r->u.insert.manifest.length <= sizeof r->manifest->manifestdata);
-    memcpy(r->manifest->manifestdata, r->u.insert.manifest.buffer, r->u.insert.manifest.length);
-    r->manifest->manifest_all_bytes = r->u.insert.manifest.length;
-    int n = rhizome_manifest_parse(r->manifest);
-    switch (n) {
-      case 0:
-	if (!r->manifest->malformed)
-	  return 0;
-	// fall through
-      case 1:
-	rhizome_manifest_free(r->manifest);
-	r->manifest = NULL;
-	r->bundle_status = RHIZOME_BUNDLE_STATUS_INVALID;
-	return http_request_rhizome_response(r, 403, "Malformed manifest", NULL);
-      default:
-	WHYF("rhizome_manifest_parse() returned %d", n);
-	// fall through
-      case -1:
-	r->bundle_status = RHIZOME_BUNDLE_STATUS_ERROR;
+  if ((r->manifest = rhizome_new_manifest()) == NULL)
+    return http_request_rhizome_response(r, 500, "Internal Error: Out of manifests", NULL);
+  assert(r->u.insert.manifest.length <= sizeof r->manifest->manifestdata);
+  memcpy(r->manifest->manifestdata, r->u.insert.manifest.buffer, r->u.insert.manifest.length);
+  r->manifest->manifest_all_bytes = r->u.insert.manifest.length;
+  int n = rhizome_manifest_parse(r->manifest);
+  switch (n) {
+    case 0:
+      if (!r->manifest->malformed)
 	break;
-    }
+      // fall through
+    case 1:
+      rhizome_manifest_free(r->manifest);
+      r->manifest = NULL;
+      r->bundle_status = RHIZOME_BUNDLE_STATUS_INVALID;
+      return http_request_rhizome_response(r, 403, "Malformed manifest", NULL);
+    default:
+      WHYF("rhizome_manifest_parse() returned %d", n);
+      // fall through
+    case -1:
+      r->bundle_status = RHIZOME_BUNDLE_STATUS_ERROR;
+      break;
   }
-  return 500;
+  rhizome_manifest *mout = NULL;
+  char message[150];
+  enum rhizome_add_file_result result = rhizome_manifest_add_file(r->u.insert.appending, r->manifest, &mout,
+								  r->u.insert.received_secret ? &r->u.insert.bundle_secret : NULL,
+								  r->u.insert.received_author ? &r->u.insert.author: NULL,
+								  NULL, 0, NULL, strbuf_local(message, sizeof message));
+  int result_valid = 0;
+  switch (result) {
+  case RHIZOME_ADD_FILE_ERROR:
+    return http_request_rhizome_response(r, 500, message, NULL);
+  case RHIZOME_ADD_FILE_OK:
+    result_valid = 1;
+    break;
+  case RHIZOME_ADD_FILE_INVALID:
+    r->bundle_status = RHIZOME_BUNDLE_STATUS_INVALID; // TODO separate enum for CLI return codes
+    return http_request_rhizome_response(r, 403, message, NULL);
+  case RHIZOME_ADD_FILE_BUSY:
+    r->bundle_status = RHIZOME_BUNDLE_STATUS_BUSY; // TODO separate enum for CLI return codes
+    return http_request_rhizome_response(r, 403, message, NULL);
+  case RHIZOME_ADD_FILE_REQUIRES_JOURNAL:
+    r->bundle_status = RHIZOME_BUNDLE_STATUS_INVALID; // TODO separate enum for CLI return codes
+    return http_request_rhizome_response(r, 403, message, NULL);
+  case RHIZOME_ADD_FILE_INVALID_FOR_JOURNAL:
+    r->bundle_status = RHIZOME_BUNDLE_STATUS_INVALID; // TODO separate enum for CLI return codes
+    return http_request_rhizome_response(r, 403, message, NULL);
+  case RHIZOME_ADD_FILE_WRONG_SECRET:
+    r->bundle_status = RHIZOME_BUNDLE_STATUS_READONLY; // TODO separate enum for CLI return codes
+    return http_request_rhizome_response(r, 403, message, NULL);
+  }
+  if (!result_valid)
+    FATALF("result = %d", result);
+  assert(mout != NULL);
+  if (mout != r->manifest) {
+    rhizome_manifest_free(r->manifest);
+    r->manifest = mout;
+  }
+  assert(r->manifest != NULL);
+  return 0;
 }
 
 static int insert_mime_part_header(struct http_request *hr, const struct mime_part_headers *h)
@@ -452,10 +487,10 @@ static int insert_mime_part_header(struct http_request *hr, const struct mime_pa
     )
       rhizome_manifest_set_name_from_path(r->manifest, h->content_disposition.filename);
     // Start writing the payload content into the Rhizome store.
-    if (r->u.insert.is_append) {
+    if (r->u.insert.appending) {
       r->payload_status = rhizome_write_open_journal(&r->u.insert.write, r->manifest, 0, RHIZOME_SIZE_UNSET);
       if (r->payload_status == RHIZOME_PAYLOAD_STATUS_ERROR) {
-	WHYF("rhizome_write_open_journal() returned %s", rhizome_payload_status_message(r->payload_status));
+	WHYF("rhizome_write_open_journal() returned %d %s", r->payload_status, rhizome_payload_status_message(r->payload_status));
 	return 500;
       }
     } else {
@@ -463,7 +498,7 @@ static int insert_mime_part_header(struct http_request *hr, const struct mime_pa
       // not contain a 'filesize' field.
       r->payload_status = rhizome_write_open_manifest(&r->u.insert.write, r->manifest);
       if (r->payload_status == RHIZOME_PAYLOAD_STATUS_ERROR) {
-	WHYF("rhizome_write_open_manifest() returned %s", rhizome_payload_status_message(r->payload_status));
+	WHYF("rhizome_write_open_manifest() returned %d %s", r->payload_status, rhizome_payload_status_message(r->payload_status));
 	return 500;
       }
     }
@@ -545,33 +580,6 @@ static int insert_mime_part_end(struct http_request *hr)
     int result = insert_make_manifest(r);
     if (result)
       return result;
-    if (r->u.insert.received_secret) {
-      if (r->manifest->has_id) {
-	if (!rhizome_apply_bundle_secret(r->manifest, &r->u.insert.bundle_secret)) {
-	  http_request_simple_response(&r->http, 403, "Secret does not match Bundle Id");
-	  return 403;
-	}
-      } else {
-	if (rhizome_new_bundle_from_secret(r->manifest, &r->u.insert.bundle_secret) == -1) {
-	  http_request_simple_response(&r->http, 500, "Internal error: Failed to create bundle from secret");
-	  return 500;
-	}
-      }
-    }
-    if (r->manifest->service == NULL)
-      rhizome_manifest_set_service(r->manifest, RHIZOME_SERVICE_FILE);
-    const char *reason = rhizome_fill_manifest(r->manifest, NULL, r->u.insert.received_author ? &r->u.insert.author: NULL);
-    if (reason != NULL) {
-      http_request_simple_response(&r->http, 500, alloca_sprintf(-1, "Internal error: %s", reason));
-      return 500;
-    }
-    assert(r->manifest != NULL);
-    if (r->u.insert.is_append) {
-      if (r->manifest->filesize == RHIZOME_SIZE_UNSET)
-	rhizome_manifest_set_filesize(r->manifest, 0);
-      if (!r->manifest->is_journal)
-	rhizome_manifest_set_tail(r->manifest, 0);
-    }
   }
   else if (r->u.insert.current_part == PART_PAYLOAD) {
     r->u.insert.received_payload = 1;
@@ -604,10 +612,6 @@ static int restful_rhizome_insert_end(struct http_request *hr)
     return http_response_form_part(r, "Missing", PART_PAYLOAD, NULL, 0);
   // Fill in the missing manifest fields and ensure payload and manifest are consistent.
   assert(r->manifest != NULL);
-  if (!r->u.insert.is_append && r->manifest->is_journal)
-    return http_request_rhizome_response(r, 403, "Insert not supported for journals", NULL);
-  else if (r->u.insert.is_append && !r->manifest->is_journal)
-    return http_request_rhizome_response(r, 403, "Append not supported for non-journals", NULL);
   assert(r->u.insert.write.file_length != RHIZOME_SIZE_UNSET);
   int status_valid = 0;
   if (config.debug.rhizome)
@@ -654,8 +658,9 @@ static int restful_rhizome_insert_end(struct http_request *hr)
     return http_request_rhizome_response(r, 500, NULL, NULL);
   }
   // Finalise the manifest and add it to the store.
-  if (r->u.insert.is_append)
+  if (r->u.insert.appending)
     rhizome_manifest_set_version(r->manifest, r->manifest->filesize);
+
   if (r->manifest->filesize) {
     if (!r->manifest->has_filehash)
       rhizome_manifest_set_filehash(r->manifest, &r->u.insert.write.id);

tests/rhizomeops

@@ -1014,8 +1014,8 @@ setup_JournalAddCreate() {
 }
 test_JournalAddCreate() {
    # TODO: servald should return a status code reserved for this case, instead
-   # of the generic 255 error
-   execute --exit-status=255 $servald rhizome add file $SIDB1 file1 file1.manifest
+   # of the 4 error (which means "invalid manifest")
+   execute --exit-status=4 $servald rhizome add file $SIDB1 file1 file1.manifest
    tfw_cat --stdout --stderr
 }
 
@@ -1036,8 +1036,8 @@ setup_JournalAddUpdate() {
 }
 test_JournalAddUpdate() {
    # TODO: servald should return a status code reserved for this case, instead
-   # of the generic 255 error
-   execute --exit-status=255 $servald rhizome add file $SIDB1 file1x file1x.manifest
+   # of the 4 error (which means "invalid manifest")
+   execute --exit-status=4 $servald rhizome add file $SIDB1 file1x file1x.manifest
    tfw_cat --stdout --stderr
 }
 
@@ -1054,8 +1054,8 @@ setup_AppendFile() {
 }
 test_AppendFile() {
    # TODO: servald should return a status code reserved for this case, instead
-   # of the generic 255 error
-   execute --exit-status=255 $servald rhizome journal append $SIDB1 $BID file2
+   # of the 4 error (which means "invalid manifest")
+   execute --exit-status=4 $servald rhizome journal append $SIDB1 $BID file2
    tfw_cat --stdout --stderr
 }
 

tests/rhizomerestful

@@ -992,7 +992,7 @@ test_RhizomeInsertJournal() {
    assertExitStatus == 0
    assertStdoutIs 403
    assertJq http.body 'contains({"http_status_code": 403})'
-   assertJqGrep --ignore-case http.body '.http_status_message' 'not supported.*journal'
+   assertJqGrep --ignore-case http.body '.http_status_message' 'cannot add.*journal'
    executeOk_servald rhizome list
    assert_rhizome_list
 }