Skip to content

PATCH to a submission never queues reprocessing after a file upload #248

Description

@jvendetti

Summary

PATCH /ontologies/:acronym/submissions/:id with a file upload stores the file but never queues the submission for processing. The uploaded file becomes the submission's uploadFilePath, yet nothing reprocesses it, so the ontology keeps serving the previously parsed content until someone kicks off a reprocess by hand from the Admin console.

This was observed in practice: editing an ontology through the BioPortal UI to switch from a pull URL to a newly uploaded local file stored the new file correctly, but no processing was triggered.

Cause

Two helpers in helpers/application_helper.rb ask the same question with different operators:

def request_has_file?                      # line 489 — gates the reprocess
  @params.any? { |p, v| v.instance_of?(Hash) && v.key?(:tempfile) && v[:tempfile].instance_of?(Tempfile) }
end

def file_from_request                      # line 495 — performs the upload
  @params.each_value do |value|
    if value.is_a?(Hash) && value.key?(:tempfile) && value[:tempfile].instance_of?(Tempfile)

Sinatra's params are a Sinatra::IndifferentHash, which is a subclass of Hash, and its convert_value turns every nested hash into another IndifferentHash. So for a multipart file part:

  • value.is_a?(Hash)true, so file_from_request finds the file and add_file_to_submission stores it.
  • value.instance_of?(Hash)false, because instance_of? is strict class equality. So request_has_file? returns false.

That means the guard in controllers/ontology_submissions_controller.rb:80 never fires on the upload branch:

if (params.keys & REQUIRES_REPROCESS).length > 0 || request_has_file?
  cron = NcboCron::Models::OntologySubmissionParser.new
  cron.queue_submission(submission, { all: true })
end

Confirmed against Sinatra 4.2.1:

value class:         Sinatra::IndifferentHash
is_a?(Hash):         true      <- file_from_request uses this
instance_of?(Hash):  false     <- request_has_file? uses this

request_has_file? appears to have no code path that can return true, so this affects every file-bearing PATCH, not just the switch-source case.

Not affected

POST /submissions and POST /ontologies/:acronym/submissions are fine — create_submission calls queue_submission unconditionally, so brand-new submissions always process. A PATCH that changes one of the REQUIRES_REPROCESS attributes also still queues correctly. Only the file-upload trigger is broken.

Suggested fix

Changing instance_of? to is_a? would work, but the underlying problem is two predicates that must agree and don't. Deriving one from the other removes the possibility of them drifting again:

def request_has_file?
  !file_from_request.last.nil?
end

(file_from_request returns [filename, tempfile], and only scans the params hash, so calling it twice per request is cheap.)

Worth a regression test that PATCHes a submission with a file and asserts the submission was queued.

Context

Found while investigating ncbo/bioportal_web_ui#435 (editing an ontology's location). A separate stale-pull-file problem in ncbo_cron, where a changed pullLocation is never re-downloaded, is being tracked on its own.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions