Skip to content

Centralize local photo resolution behind policy-based lookup #168

Description

@atomantic

Problem

The server has one canonical on-disk photo layout, but six service-local implementations still construct the filenames, try .jpg/.png, choose provider precedence, and reconstruct the route URL independently. findPhoto() only hides the two-extension check for one source, so callers that need a priority list reimplement the rest of the policy. A newly supported source, extension, or route change therefore has several independently drifting updates.

Evidence

Occurrence count: 6 independent local-photo lookup/selection procedures, plus the underpowered one-source primitive.

Complete implementation/call-site list for this root cause:

  • server/src/utils/paths.ts:25findPhoto(personId, suffix?) implements the base/suffix filename grammar and extension precedence, but returns only a path.
  • server/src/services/sparse-tree.service.ts:93 — repeats the display priority ancestry > wikitree > wiki > generic and all eight filename checks.
  • server/src/services/ancestry-tree.service.ts:17 — repeats the same display priority through four provider-specific predicates and reconstructs the same URLs.
  • server/src/services/favorites.service.ts:18 — independently checks augmentation Wikipedia state, then generic .jpg/.png and constructs its URL.
  • server/src/services/familysearch-upload.service.ts:251 — repeats an eight-item candidate list and source-to-route mapping; lines 284-287 duplicate generic/familysearch existence checking.
  • server/src/services/ancestry-upload.service.ts:38 — repeats a ten-item candidate list (including the re-upload exception) and source-to-route mapping.
  • server/src/services/multi-platform-comparison.service.ts:40 — manually reconstructs provider-suffixed .jpg/.png existence checks before downloading.

Representative duplicated selection logic:

const photoChecks = [
  { path: join(photosDir, `${canonicalId}-familysearch.jpg`), url: `/augment/${canonicalId}/familysearch-photo` },
  { path: join(photosDir, `${canonicalId}-familysearch.png`), url: `/augment/${canonicalId}/familysearch-photo` },
  // ... provider/source pairs repeated for every caller
];
for (const check of photoChecks) {
  if (existsSync(check.path)) return check;
}

The equivalent display flow in sparse-tree.service.ts:94-115 spells out the same source-plus-extension checks instead of using findPhoto().

Impact

Photo behavior already diverges: tree cards prefer Ancestry, WikiTree, Wikipedia, and generic images; favorites ignore locally stored Ancestry/WikiTree images; the two upload flows have separate, inversely motivated source orderings. Different orders are legitimate product policy, but reimplementing discovery and URL mapping makes those differences opaque and risks accidental inconsistency when the storage convention evolves. It also leaves every path responsible for preserving jpg-before-png behavior.

Implementation plan

  1. Extend server/src/utils/paths.ts with a small, filesystem-only resolver that owns the file naming and extension policy, not business precedence:

    type PhotoSource = 'familysearch' | 'ancestry' | 'wikitree' | 'wiki' | 'generic' | 'linkedin';
    type LocalPhoto = { source: PhotoSource; path: string; extension: 'jpg' | 'png' };
    findLocalPhoto(personId: string, sources: readonly PhotoSource[]): LocalPhoto | null;
    hasLocalPhoto(personId: string, source: PhotoSource): boolean;
    localPhotoRoute(personId: string, source: PhotoSource, apiPrefix?: string): string;

    Keep the existing findPhoto() as a compatibility wrapper or migrate its two current callers in the same change. Put the source-to-suffix and source-to-route mappings next to this resolver; apiPrefix preserves the upload services' current no-/api route strings. Do not introduce a generic provider framework.

  2. Migrate sparse-tree, ancestry-tree, and favorites to pass their existing display-order arrays and use the resolved URL. Preserve favorites' intentional Wikipedia-first policy.

  3. Migrate familysearch-upload and ancestry-upload to pass their existing upload-specific arrays. Preserve Ancestry's isFromAncestry distinction as a simple comparison of result.source, and retain FamilySearch's symlink comparison separately.

  4. Replace multi-platform-comparison's provider file-existence check with hasLocalPhoto; retain its download and cache behavior unchanged.

Acceptance criteria

  • A single resolver owns source suffixes, jpg/png precedence, and server photo URLs.
  • The six listed callers no longer manually form both .jpg and .png candidate filenames or independently map a local source to a route URL.
  • Existing source orders remain unchanged: tree/sparse display, favorites display, FamilySearch upload, and Ancestry upload may intentionally differ.
  • Generic and suffixed FamilySearch images retain their current distinction for FamilySearch upload comparison and symlink detection.
  • No database, JSON-cache, SQLite, or PostgreSQL behavior changes.

Verification

  • Add focused unit tests around the resolver with a temporary photo directory: jpg beats png; each source maps to its current filename and URL; first requested available source wins; missing files return null.
  • Add/adjust service tests covering the existing display and upload priority orders, including Ancestry's re-upload fallback and FamilySearch generic-versus-suffixed detection.
  • Run the affected server test suites and npm run build.

Dependencies and related work

Independent of the staged PostgreSQL migration (#120, #149-#155): these functions resolve files in the JSON-authoritative photo cache and do not query SQLite. It may be landed before or alongside #153/#155, but must not widen either migration slice.

Scope

Complexity: small, contained refactor across six services and one existing path utility. The shared boundary is justified because it replaces six copies of the same storage/extension/URL contract while callers retain their product-specific source order.

Non-goals: do not merge provider upload automation, change photo priority rules, alter photo persistence, introduce database metadata, or build a configurable photo-provider registry.

Metadata

Metadata

Assignees

No one assigned

    Labels

    dryDuplication audit findingplanClaimable backlog itemseverity:mediumMedium severity

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions