Skip to content

Changing an ontology's pull location doesn't re-download; the stale file is parsed again #136

Description

@jvendetti

Summary

When an ontology's pullLocation changes, reprocessing does not fetch the new URL. The parser re-downloads only when the local copy is missing, so an existing-but-stale file is parsed again and the new pull location is ignored.

Cause

lib/ncbo_cron/ontology_submission_parser.rb:167:

# Check to make sure the file has been downloaded
if sub.pullLocation && (!sub.uploadFilePath || !File.exist?(sub.uploadFilePath))
  ...
  file, filename = sub.download_ontology_file

The condition asks "is there a local file?" when the question should be "is the local file still the right file?" Changing pullLocation doesn't make the old file disappear, so the condition stays false and download_ontology_file is never called.

Reproduce

From ncbo/bioportal_web_ui#435:

  1. Create an ontology with Upload local file, pointing at a file that fails to parse.
  2. Edit the submission, switch to Load from URL, and give it a valid ontology URL. pullLocation is set correctly.
  3. Reprocess the ontology.
  4. The original parse error comes back — the new URL was never fetched.

Setting uploadFilePath to nil by hand and reprocessing fixes it, because that makes the guard above fire and re-download. That workaround is the only current remedy, and it is what the guard's condition is accidentally requiring.

Why "just clear uploadFilePath" isn't the fix

uploadFilePath is the location of the physical file on the server, and it's populated for every submission — by uploads and by pulls alike, since download_and_store_ontology_file writes the downloaded copy there. Other code depends on it being present. In ontologies_api, GET /ontologies/:acronym/submissions/:id/download errors with "Upload File Path is not set for this submission" when it's empty, and the ontology-level download route reads it the same way.

So blanking it — whether from a client or at PATCH time — would break downloads for the window between the edit and the next successful process. The field needs to stay populated; what's missing is a way to recognize that its contents are stale.

Possible approaches

A. Re-download whenever a full reprocess runs on a pull-based submission. Replace the File.exist? condition with something closer to "pullLocation is set and this is a full reprocess". Simple and always correct. Costs a download on reprocesses that would previously have reused the cached file, and needs a decision about what to do when the remote is temporarily unreachable — falling back to the cached copy rather than failing the reprocess is probably right, since today's behavior is effectively that fallback.

B. Track which pull location produced the current file. Store the pullLocation used for the last download and re-download when it differs from the current one. More precise: only fetches when actually stale. Needs a new attribute in ontologies_linked_data, plus a decision for existing submissions where the provenance is unknown (treating them as stale once would be safe).

C. Force it at PATCH time in the API. Detect a changed pullLocation and mark the file stale. Localized, but if "mark stale" means clearing uploadFilePath it reintroduces the download gap above, so it would need a separate flag rather than a clear.

A looks like the best cost/benefit; B is the cleaner model if the extra attribute is acceptable.

Underlying modeling gap

A submission has no explicit record of which source it uses — pull URL, uploaded file, or metadata only. The BioPortal UI computes exactly that (an isRemote radio) but the value appears nowhere in ontologies_api, ontologies_linked_data, or ncbo_cron, so the source is inferred from which fields happen to be populated. That inference is what makes precedence ambiguous whenever both pullLocation and uploadFilePath are set, which is the normal state for a pulled ontology. Modeling the source type explicitly would make this class of bug hard to write, but it's a larger change than the fix above needs.

Context

Found while investigating ncbo/bioportal_web_ui#435. The UI-side half of that issue (switching away from a pull URL left the URL in place) is fixed in ncbo/bioportal_web_ui#535; this is the remaining server-side half. Related: ncbo/ontologies_api#248, where a file-bearing PATCH never queues reprocessing at all. Worth fixing that one first, since while it is broken a reprocess only happens when triggered by hand, which makes it harder to tell whether a fix here is working.

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