Skip to content

Commit 2794da2

Browse files
committed
refactor: split augmentation god file and dedupe scraper helpers
Split the 1457-line augmentation.service.ts into focused modules: - platform-linking.service.ts: URL parsing, scrapers, link* functions - augmentation-photo.service.ts: photo paths and platform photo fetching - provider-mapping.service.ts: provider mapping CRUD Extracted shared utilities and removed duplication: - fetchHtml utility replaces two hand-rolled https.get redirect blocks - normalizePhotoUrl utility consolidates URL normalization across augmentation-photo, platform-linking, and multi-platform-comparison - ensureAncestryLoggedIn and extractAncestryPhotoFromPage helpers dedupe Ancestry login flow and srcset parsing - isPlaceholderImage from base.scraper.ts replaces inline checks Fixed isLegacyFormat type guard that could crash on string/null input. Removed dead linkFamilySearch function and its only-caller helper.
1 parent 5440520 commit 2794da2

15 files changed

Lines changed: 983 additions & 1151 deletions

.changelog/NEXT.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111

1212
## Changed
1313

14+
- Split 1457-line `augmentation.service.ts` god file into focused services: `platform-linking.service.ts` (URL parsing, scrapers, link* functions), `augmentation-photo.service.ts` (photo paths, fetch-from-platform), `provider-mapping.service.ts` (provider mapping CRUD)
15+
- Extracted shared `fetchHtml` and `normalizePhotoUrl` utilities; consolidated duplicate `normalizePhotoUrl` from `multi-platform-comparison.service.ts`
16+
- Extracted `ensureAncestryLoggedIn` and `extractAncestryPhotoFromPage` helpers to dedupe Ancestry login + srcset logic
1417
- Renamed `coverage_gap` audit check to `unlinked_provider` for clarity
1518
- Split provider linkage check into primary (FamilySearch, Ancestry) at `info` severity and optional (WikiTree, 23andMe) at `hint` severity
1619
- Removed `unlinked_provider` from default enabled checks (opt-in only)
@@ -22,5 +25,6 @@
2225
## Fixed
2326

2427
- Platform comparison now treats equivalent date formats as matches (e.g., "1979-07-31" vs "31 JUL 1979")
28+
- `isLegacyFormat` augmentation type guard no longer crashes on string/null input
2529

2630
## Removed

PLAN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ Summary: 105 findings across 60+ files. 1 shared utility to extract (SSE Manager
441441

442442
### Architecture & SOLID
443443
Architecture findings are tracked but not auto-remediated (all Complex, high risk of regression):
444-
- [ ] **[HIGH]** `server/src/services/augmentation.service.ts` — 1457-line god file, 5 responsibilities. (Complex)
444+
- [x] ~~**[HIGH]** `server/src/services/augmentation.service.ts` — 1457-line god file, 5 responsibilities. (Complex)~~ (Split into: core CRUD in augmentation.service.ts, platform-linking.service.ts, augmentation-photo.service.ts, provider-mapping.service.ts)
445445
- [ ] **[HIGH]** `server/src/services/multi-platform-comparison.service.ts` — 1095-line god file. (Complex)
446446
- [ ] **[HIGH]** `server/src/services/favorites.service.ts` — 883-line god file. (Complex)
447447
- [ ] **[HIGH]** `server/src/services/database.service.ts` — 1073-line god file. (Complex)

server/src/routes/augmentation.routes.ts

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,16 @@ import fs from 'fs';
33
import path from 'path';
44
import type { ProviderPersonMapping, PlatformType } from '@fsf/shared';
55
import { augmentationService } from '../services/augmentation.service.js';
6+
import { linkWikipedia, linkAncestry, linkWikiTree, linkLinkedIn } from '../services/platform-linking.service.js';
7+
import {
8+
getWikiPhotoPath, hasWikiPhoto,
9+
getAncestryPhotoPath, hasAncestryPhoto,
10+
getWikiTreePhotoPath, hasWikiTreePhoto,
11+
getFamilySearchPhotoPath, hasFamilySearchPhoto,
12+
getLinkedInPhotoPath, hasLinkedInPhoto,
13+
fetchPhotoFromPlatform,
14+
} from '../services/augmentation-photo.service.js';
15+
import { getProviderMappings, addProviderMapping, removeProviderMapping } from '../services/provider-mapping.service.js';
616
import { logger } from '../lib/logger.js';
717
import { sanitizePersonId, isValidUrl } from '../utils/validation.js';
818
import { PHOTOS_DIR } from '../utils/paths.js';
@@ -102,7 +112,7 @@ router.get('/:personId', async (req: Request, res: Response) => {
102112
});
103113

104114
// Link a Wikipedia article to a person
105-
router.post('/:personId/wikipedia', linkPlatform('wikipedia.org', 'Wikipedia', (id, url) => augmentationService.linkWikipedia(id, url)));
115+
router.post('/:personId/wikipedia', linkPlatform('wikipedia.org', 'Wikipedia', (id, url) => linkWikipedia(id, url)));
106116

107117
// Update custom augmentation data
108118
router.put('/:personId', async (req: Request, res: Response) => {
@@ -130,43 +140,43 @@ router.put('/:personId', async (req: Request, res: Response) => {
130140
});
131141

132142
// Serve Wikipedia photo
133-
router.get('/:personId/wiki-photo', servePhoto(id => augmentationService.getWikiPhotoPath(id), 'Wiki'));
143+
router.get('/:personId/wiki-photo', servePhoto(id => getWikiPhotoPath(id), 'Wiki'));
134144

135145
// Check if wiki photo exists
136-
router.get('/:personId/wiki-photo/exists', checkPhotoExists(id => augmentationService.hasWikiPhoto(id)));
146+
router.get('/:personId/wiki-photo/exists', checkPhotoExists(id => hasWikiPhoto(id)));
137147

138148
// Link an Ancestry profile to a person
139-
router.post('/:personId/ancestry', linkPlatform('ancestry.com', 'Ancestry', (id, url) => augmentationService.linkAncestry(id, url)));
149+
router.post('/:personId/ancestry', linkPlatform('ancestry.com', 'Ancestry', (id, url) => linkAncestry(id, url)));
140150

141151
// Serve Ancestry photo
142-
router.get('/:personId/ancestry-photo', servePhoto(id => augmentationService.getAncestryPhotoPath(id), 'Ancestry'));
152+
router.get('/:personId/ancestry-photo', servePhoto(id => getAncestryPhotoPath(id), 'Ancestry'));
143153

144154
// Check if ancestry photo exists
145-
router.get('/:personId/ancestry-photo/exists', checkPhotoExists(id => augmentationService.hasAncestryPhoto(id)));
155+
router.get('/:personId/ancestry-photo/exists', checkPhotoExists(id => hasAncestryPhoto(id)));
146156

147157
// Link a WikiTree profile to a person
148-
router.post('/:personId/wikitree', linkPlatform('wikitree.com', 'WikiTree', (id, url) => augmentationService.linkWikiTree(id, url)));
158+
router.post('/:personId/wikitree', linkPlatform('wikitree.com', 'WikiTree', (id, url) => linkWikiTree(id, url)));
149159

150160
// Serve WikiTree photo
151-
router.get('/:personId/wikitree-photo', servePhoto(id => augmentationService.getWikiTreePhotoPath(id), 'WikiTree'));
161+
router.get('/:personId/wikitree-photo', servePhoto(id => getWikiTreePhotoPath(id), 'WikiTree'));
152162

153163
// Check if wikitree photo exists
154-
router.get('/:personId/wikitree-photo/exists', checkPhotoExists(id => augmentationService.hasWikiTreePhoto(id)));
164+
router.get('/:personId/wikitree-photo/exists', checkPhotoExists(id => hasWikiTreePhoto(id)));
155165

156166
// Serve FamilySearch photo
157-
router.get('/:personId/familysearch-photo', servePhoto(id => augmentationService.getFamilySearchPhotoPath(id), 'FamilySearch'));
167+
router.get('/:personId/familysearch-photo', servePhoto(id => getFamilySearchPhotoPath(id), 'FamilySearch'));
158168

159169
// Check if FamilySearch photo exists
160-
router.get('/:personId/familysearch-photo/exists', checkPhotoExists(id => augmentationService.hasFamilySearchPhoto(id)));
170+
router.get('/:personId/familysearch-photo/exists', checkPhotoExists(id => hasFamilySearchPhoto(id)));
161171

162172
// Link a LinkedIn profile to a person
163-
router.post('/:personId/linkedin', linkPlatform('linkedin.com', 'LinkedIn', (id, url) => augmentationService.linkLinkedIn(id, url)));
173+
router.post('/:personId/linkedin', linkPlatform('linkedin.com', 'LinkedIn', (id, url) => linkLinkedIn(id, url)));
164174

165175
// Serve LinkedIn photo
166-
router.get('/:personId/linkedin-photo', servePhoto(id => augmentationService.getLinkedInPhotoPath(id), 'LinkedIn'));
176+
router.get('/:personId/linkedin-photo', servePhoto(id => getLinkedInPhotoPath(id), 'LinkedIn'));
167177

168178
// Check if LinkedIn photo exists
169-
router.get('/:personId/linkedin-photo/exists', checkPhotoExists(id => augmentationService.hasLinkedInPhoto(id)));
179+
router.get('/:personId/linkedin-photo/exists', checkPhotoExists(id => hasLinkedInPhoto(id)));
170180

171181
// Fetch and download photo from a linked platform
172182
router.post('/:personId/fetch-photo/:platform', async (req: Request, res: Response) => {
@@ -179,7 +189,7 @@ router.post('/:personId/fetch-photo/:platform', async (req: Request, res: Respon
179189
return;
180190
}
181191

182-
const data = await augmentationService.fetchPhotoFromPlatform(personId, platform as any).catch(err => {
192+
const data = await fetchPhotoFromPlatform(personId, platform as any).catch(err => {
183193
logger.error('augment', `Error fetching photo from ${platform}: ${err.message}`);
184194
res.status(500).json({ success: false, error: err.message });
185195
return null;
@@ -193,7 +203,7 @@ router.post('/:personId/fetch-photo/:platform', async (req: Request, res: Respon
193203
// Get all provider mappings for a person
194204
router.get('/:personId/provider-links', (req: Request, res: Response) => {
195205
const personId = sanitizePersonId(req.params.personId);
196-
const mappings = augmentationService.getProviderMappings(personId);
206+
const mappings = getProviderMappings(personId);
197207
res.json({ success: true, data: mappings });
198208
});
199209

@@ -226,7 +236,7 @@ router.post('/:personId/provider-link', (req: Request, res: Response) => {
226236
matchedBy: matchedBy || 'manual',
227237
};
228238

229-
const data = augmentationService.addProviderMapping(personId, mapping);
239+
const data = addProviderMapping(personId, mapping);
230240
res.json({ success: true, data });
231241
});
232242

@@ -235,7 +245,7 @@ router.delete('/:personId/provider-link/:providerId', (req: Request, res: Respon
235245
const personId = sanitizePersonId(req.params.personId);
236246
const { providerId } = req.params;
237247

238-
const data = augmentationService.removeProviderMapping(personId, providerId);
248+
const data = removeProviderMapping(personId, providerId);
239249

240250
if (!data) {
241251
res.status(404).json({ success: false, error: 'No augmentation data found' });

server/src/services/ancestry-hints.service.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { Page } from 'playwright';
99
import type { AncestryHintProgress, AncestryHintResult } from '@fsf/shared';
1010
import { browserService } from './browser.service.js';
1111
import { augmentationService } from './augmentation.service.js';
12+
import { parseAncestryUrl } from './platform-linking.service.js';
1213
import { providerService } from './provider.service.js';
1314
import { PROVIDER_DEFAULTS } from './scrapers/base.scraper.js';
1415
import { logger } from '../lib/logger.js';
@@ -169,7 +170,7 @@ async function processPersonHints(
169170
}
170171

171172
// Parse the Ancestry URL to get treeId and personId
172-
const parsed = augmentationService.parseAncestryUrl(ancestryPlatform.url);
173+
const parsed = parseAncestryUrl(ancestryPlatform.url);
173174
if (!parsed) {
174175
return {
175176
personId,
@@ -335,7 +336,7 @@ async function* processPersonHintsWithProgress(
335336
}
336337

337338
// Parse the Ancestry URL
338-
const parsed = augmentationService.parseAncestryUrl(ancestryPlatform.url);
339+
const parsed = parseAncestryUrl(ancestryPlatform.url);
339340
if (!parsed) {
340341
tracker.finish();
341342
yield {

server/src/services/ancestry-tree.service.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type {
66
Person
77
} from '@fsf/shared';
88
import { databaseService } from './database.service.js';
9-
import { augmentationService } from './augmentation.service.js';
9+
import { hasAncestryPhoto, hasWikiTreePhoto, hasWikiPhoto } from './augmentation-photo.service.js';
1010
import { scraperService } from './scraper.service.js';
1111
import { logger } from '../lib/logger.js';
1212

@@ -16,17 +16,17 @@ import { logger } from '../lib/logger.js';
1616
*/
1717
function resolvePhotoUrl(personId: string): string | undefined {
1818
// Try Ancestry photo first (highest quality usually)
19-
if (augmentationService.hasAncestryPhoto(personId)) {
19+
if (hasAncestryPhoto(personId)) {
2020
return `/api/augment/${personId}/ancestry-photo`;
2121
}
2222

2323
// Try WikiTree photo
24-
if (augmentationService.hasWikiTreePhoto(personId)) {
24+
if (hasWikiTreePhoto(personId)) {
2525
return `/api/augment/${personId}/wikitree-photo`;
2626
}
2727

2828
// Try Wikipedia photo (via augmentation)
29-
if (augmentationService.hasWikiPhoto(personId)) {
29+
if (hasWikiPhoto(personId)) {
3030
return `/api/augment/${personId}/wiki-photo`;
3131
}
3232

server/src/services/ancestry-update.service.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { AncestryUpdateProgress } from '@fsf/shared';
99
import { sqliteService } from '../db/sqlite.service.js';
1010
import { idMappingService } from './id-mapping.service.js';
1111
import { augmentationService } from './augmentation.service.js';
12+
import { parseAncestryUrl } from './platform-linking.service.js';
1213
import { ancestryHintsService } from './ancestry-hints.service.js';
1314
import { providerService } from './provider.service.js';
1415
import { browserService } from './browser.service.js';
@@ -101,7 +102,7 @@ function getAncestryLink(personId: string): { url: string; treeId: string; ances
101102

102103
if (!ancestryPlatform?.url) return null;
103104

104-
const parsed = augmentationService.parseAncestryUrl(ancestryPlatform.url);
105+
const parsed = parseAncestryUrl(ancestryPlatform.url);
105106
if (!parsed) return null;
106107

107108
return {

server/src/services/ancestry-upload.service.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { existsSync, readFileSync } from 'fs';
1010
import { join } from 'path';
1111
import { browserService } from './browser.service.js';
1212
import { augmentationService } from './augmentation.service.js';
13+
import { parseAncestryUrl } from './platform-linking.service.js';
1314
import { idMappingService } from './id-mapping.service.js';
1415
import { credentialsService } from './credentials.service.js';
1516
import { personService } from './person.service.js';
@@ -26,7 +27,7 @@ function getAncestryIds(canonicalId: string): { treeId: string; ancestryPersonId
2627
const augmentation = augmentationService.getAugmentation(canonicalId);
2728
const ancestryPlatform = augmentation?.platforms?.find(p => p.platform === 'ancestry');
2829
if (!ancestryPlatform?.url) return null;
29-
return augmentationService.parseAncestryUrl(ancestryPlatform.url);
30+
return parseAncestryUrl(ancestryPlatform.url);
3031
}
3132

3233
/**

0 commit comments

Comments
 (0)