Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use filename instead of name for id in HarvestList #3082

Merged
merged 2 commits into from
May 11, 2020
Merged

Conversation

dafeder
Copy link
Member

@dafeder dafeder commented May 8, 2020

We are using the name element from file_scan_directory to create source_ids that are stored in the migrate map tables for harvest. name attempts to remove the file extension (specifically, it uses the basename element from PHP's pathinfo(), so asdf1234.json in the cache would be simply stored as source ID "asdf1234." However, files are not cached with a file extension. In most cases this doesn't matter, but if there is a period (.) in ID, what comes after it can be mistaken for a file extension.

So, source ID knb-lter-jrn.210001001.61 gets cached the the filename "public://dkan-harvest-cache/[harvest-machine-name]/knb-lter-jrn.210001001.61", but when gathered by the getIdList() function will then be interpreted as simply the id knb-lter-jrn.210001001. Harvest will then think this is an orphaned dataset and unpublish it.

Since we don't cache either JSON or XML files with extensions, using filename should remove this problem with no ill effects.

QA Steps

  1. Create a harvest from https://api.jsonbin.io/b/5eb59eb9a47fdd6af15fd015 - it should bring in 4 datasets.
  2. Delete one of the datasets
  3. Re-run the Harvest. The deleted dataset should come back

Tests

I did not add a new test for this, it felt like an edge case. Reviewer can lmk if they think a unit test is needed.

@dafeder dafeder added the 1.x DKAN classic label May 8, 2020
@dharizza dharizza merged commit 8a12c74 into 7.x-1.x May 11, 2020
@dharizza dharizza deleted the harvest-id-name branch May 11, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x DKAN classic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants