Skip to content

fix: Skip media entities with empty URI from OBJE without FILE#586

Open
isaacschepp wants to merge 23 commits intomainfrom
fix/empty-media-uri
Open

fix: Skip media entities with empty URI from OBJE without FILE#586
isaacschepp wants to merge 23 commits intomainfrom
fix/empty-media-uri

Conversation

@isaacschepp
Copy link
Copy Markdown
Collaborator

What and why

GEDCOM OBJE records with empty or missing FILE values created media entities with empty uri fields. These are invalid entities that can't reference any actual media file.

The fix

Added URI guards in both convertMedia (top-level OBJE) and convertEmbeddedMedia (inline OBJE). When media.URI == "" after processing, the entity is skipped with a log message instead of stored.

OBJE with BLOB data are unaffected — the BLOB path populates URI before the guard is reached.

Related issues

Fixes #492

Testing

  • TestImportMedia_EmptyFileSkipped — OBJE with empty FILE value
  • TestImportMedia_NoFileTagSkipped — OBJE with no FILE tag at all
  • All existing tests pass

Breaking changes

None. Previously, empty-URI media entities were created silently. Now they're skipped. No valid media data is lost.

OBJE records with empty or missing FILE values were creating media
entities with empty URI fields. Now both convertMedia and
convertEmbeddedMedia skip storage when URI is empty (no FILE and
no BLOB data).

Fixes #492
Copilot AI review requested due to automatic review settings March 31, 2026 18:02
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 31, 2026

Deploying genealogix with  Cloudflare Pages  Cloudflare Pages

Latest commit: 775b410
Status: ✅  Deploy successful!
Preview URL: https://6e6db6cf.genealogix.pages.dev
Branch Preview URL: https://fix-empty-media-uri.genealogix.pages.dev

View logs

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes GEDCOM import behavior where OBJE records with missing/empty FILE values were producing invalid GLX media entities with an empty uri, by skipping such media entities during conversion.

Changes:

  • Added guards in top-level and embedded OBJE conversion to skip media when media.URI == "", with log messages (fix for #492).
  • Added import tests covering empty FILE and missing FILE tag scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
go-glx/gedcom_media.go Skips media entities that would end up with an empty URI during OBJE conversion.
go-glx/gedcom_import_test.go Adds tests intended to ensure OBJE records without usable FILE data don’t create invalid media.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@isaacschepp
Copy link
Copy Markdown
Collaborator Author

Deleted 5 stale Go module caches (~7 KiB each) that were causing the \ message. The caches were saved empty before \ was added, and were being hit on every run (key matched \ hash). Only the healthy 43.53 MiB cache remains. Next CI run should populate fresh caches for the current \ hash.

- Delete MediaIDMap entry when skipping empty-URI OBJE to prevent
  dangling references from other records pointing to non-existent media
- Replace weak loop-based test with direct assert.Empty on Media map
@isaacschepp
Copy link
Copy Markdown
Collaborator Author

Both Copilot items addressed in 144dd5c:

  1. Stale MediaIDMap entry — Added delete(conv.MediaIDMap, objeRecord.XRef) in the early-return path. Prevents other GEDCOM records from resolving an OBJE XRef to a media ID that was never stored.

  2. Weak test — Replaced the loop-based assertion with assert.Empty(t, glxFile.Media) which directly verifies no media entities were created.

Copilot AI review requested due to automatic review settings March 31, 2026 19:28
@isaacschepp isaacschepp review requested due to automatic review settings March 31, 2026 19:28
Copilot AI review requested due to automatic review settings April 1, 2026 02:00
@isaacschepp isaacschepp review requested due to automatic review settings April 1, 2026 02:00
Copilot AI review requested due to automatic review settings April 1, 2026 02:04
@isaacschepp isaacschepp review requested due to automatic review settings April 1, 2026 02:04
Copilot AI review requested due to automatic review settings April 1, 2026 03:28
@isaacschepp isaacschepp review requested due to automatic review settings April 1, 2026 03:28
Copilot AI review requested due to automatic review settings April 1, 2026 03:29
@isaacschepp isaacschepp review requested due to automatic review settings April 1, 2026 03:29
Copilot AI review requested due to automatic review settings April 1, 2026 21:08
@isaacschepp isaacschepp review requested due to automatic review settings April 1, 2026 21:08
Copilot AI review requested due to automatic review settings April 1, 2026 21:30
@isaacschepp isaacschepp review requested due to automatic review settings April 1, 2026 21:30
Copilot AI review requested due to automatic review settings April 2, 2026 05:34
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 05:34
Copilot AI review requested due to automatic review settings April 2, 2026 15:19
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 15:19
Copilot AI review requested due to automatic review settings April 2, 2026 15:26
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 15:26
Copilot AI review requested due to automatic review settings April 2, 2026 15:39
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 15:39
Copilot AI review requested due to automatic review settings April 2, 2026 15:48
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 15:48
Copilot AI review requested due to automatic review settings April 2, 2026 15:59
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 15:59
Copilot AI review requested due to automatic review settings April 2, 2026 16:16
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 16:16
Copilot AI review requested due to automatic review settings April 2, 2026 16:23
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 16:23
Copilot AI review requested due to automatic review settings April 2, 2026 16:29
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 16:29
Copilot AI review requested due to automatic review settings April 2, 2026 16:36
@isaacschepp isaacschepp review requested due to automatic review settings April 2, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GEDCOM import: media entities created with empty URI from empty FILE values

2 participants