fix(occurrence): fix for #526#528
Merged
Merged
Conversation
djtfmartin
commented
Jun 23, 2026
djtfmartin
commented
Contributor
- Fix for The archive for download is empty #526
- changes in DownloadDwcActor so that exceptions are propogated and fail in the event of error
djtfmartin
force-pushed
the
fix/526-ext-duplicate-keys
branch
from
June 23, 2026 12:07
59aeced to
9220d2c
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses occurrence download failures where duplicate normalized extension field names caused extension CSV writing to fail (issue #526), leading to empty or missing download archives. It updates DownloadDwcaActor to tolerate duplicates and adjusts exception propagation, and adds a unit test around extension-field normalization.
Changes:
- Handle duplicate normalized extension keys when building extension record maps (avoid
Collectors.toMapduplicate-key failure). - Replace Guava
Throwables.propagateusage withRuntimeExceptionwrapping in the actor. - Add a unit test for
toExtensionRecordnormalization behavior, plusakka-testkitfor test support.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
occurrence-download/src/main/java/org/gbif/occurrence/download/file/dwca/akka/DownloadDwcaActor.java |
Avoids failure on duplicate normalized extension keys; adjusts exception propagation; adds local normalization helper. |
occurrence-download/src/test/java/org/gbif/occurrence/download/file/dwca/akka/DownloadDwcaActorTest.java |
Adds coverage for extension record normalization and duplicate-key handling. |
occurrence-download/pom.xml |
Adds akka-testkit as a test dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
djtfmartin
force-pushed
the
fix/526-ext-duplicate-keys
branch
from
June 23, 2026 12:14
0c992c5 to
fe4560e
Compare
djtfmartin
force-pushed
the
fix/526-ext-duplicate-keys
branch
from
June 23, 2026 12:21
bf4b229 to
f1a9d07
Compare
marcos-lg
requested changes
Jun 23, 2026
| String key = normalizeFieldName(HiveColumns.columnFor(e.getKey())); | ||
| String val = e.getValue(); | ||
| if (normalized.containsKey(key)) { | ||
| // Keep the first value, but log the duplicate for diagnostics |
Contributor
There was a problem hiding this comment.
I think we need to return both terms instead of taking the first one so we download the extension as it is originally. Both are valid terms, they could have different values even if it were weird. I think this is what we do in big downloads(would be good to check)
Contributor
Author
There was a problem hiding this comment.
How can we do this without changing the headers in downloads to include some sort of prefix (and potentially breaking third party scripts) ?
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
occurrence-download/src/test/java/org/gbif/occurrence/download/file/dwca/akka/DownloadDwcaActorTest.java:79
- The ActorSystem created for this test is never terminated, which can leak threads and cause the test JVM to hang. Ensure it’s shut down (ideally in a finally/@AfterEach).
}
| expected.put(DcElement.source.simpleName().toLowerCase(), "US National Herbarium, Department of Botany, NMNH, Smithsonian Institution"); | ||
| expected.put(DcElement.creator.simpleName().toLowerCase(), "Conveyor Belt"); | ||
| expected.put(AcTerm.providerLiteral.simpleName().toLowerCase(), "Smithsonian Institution, NMNH, Botany"); | ||
| expected.put("`format`", "image/jpeg"); |
djtfmartin
force-pushed
the
fix/526-ext-duplicate-keys
branch
from
June 23, 2026 12:40
eefc8fe to
9689acb
Compare
djtfmartin
force-pushed
the
fix/526-ext-duplicate-keys
branch
from
June 23, 2026 14:15
033b9d2 to
ebc60aa
Compare
marcos-lg
approved these changes
Jun 23, 2026
Comment on lines
279
to
282
| } catch (Exception e) { | ||
| getSender().tell(e, getSelf()); // inform our master | ||
| throw Throwables.propagate(e); | ||
| throw (e instanceof RuntimeException) ? (RuntimeException) e : new RuntimeException(e); | ||
| } |
Comment on lines
+76
to
+77
| assertEquals(expected, result); | ||
| system.terminate(); |
| expected.put(AcTerm.licenseLogoURL.simpleName().toLowerCase(), "https://www.si.edu/sites/default/files/icons/cc0.svg"); | ||
| expected.put("dc_rights", "CC0"); // this one wins | ||
| expected.put(DcTerm.description.simpleName().toLowerCase(), "Barcode 03538523"); | ||
| expected.put("dcterms_rights", "CC0"); // this one win |
Comment on lines
+193
to
+215
| Map<String, Integer> duplicatesMap = new HashMap<>(); | ||
| row.forEach((key, value) -> { | ||
| if (duplicatesMap.containsKey(key.simpleName().toLowerCase())) { | ||
| duplicatesMap.computeIfPresent(key.simpleName().toLowerCase(), (s, i) -> ++i); | ||
| } else { | ||
| duplicatesMap.put(key.simpleName().toLowerCase(), 1); | ||
| } | ||
| }); | ||
|
|
||
| // fix for https://github.com/gbif/occurrence/issues/526 | ||
| // handle duplicate keys (which are duplicates when the base url is stripped for the CSV) | ||
| // Build the normalized map by iterating so we can log duplicates when they occur. | ||
| Map<String, String> normalized = new LinkedHashMap<>(); | ||
| for (Map.Entry<Term, String> e : row.entrySet()) { | ||
| Term term = e.getKey(); | ||
| String fName = term.simpleName(); | ||
| if (duplicatesMap.get(fName) != null && duplicatesMap.get(fName) > 1) { | ||
| fName = term.prefixedName(); | ||
| } | ||
| String normalizeFieldName = TermNormalizationUtils.normalizeFieldName(fName); | ||
| normalized.put(normalizeFieldName, e.getValue()); | ||
| } | ||
| extensionData.putAll(normalized); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.