Skip to content

WEBDEV-9424: Migrate search-service into elements - #172

Open
jbuckner wants to merge 6 commits into
WEBDEV-9419-item-metadatafrom
WEBDEV-9424-search-service
Open

jbuckner wants to merge 6 commits into
WEBDEV-9419-item-metadatafrom
WEBDEV-9424-search-service

Conversation

@jbuckner

@jbuckner jbuckner commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

Moves search-service into src/services/search-service/, ported from @internetarchive/search-service 2.7.2 (the published source). All 209 tests came over, converted from mocha and sinon to vitest. It imports the local item-metadata and result-type. Changes from a straight port:

  • The four enums (SearchType, AggregationSortType, FilterConstraint, SearchServiceErrorType) are const objects with a same-named type, with the same values, because the repo forbids enums.
  • subject on the hit classes is typed StringListField. Against item-metadata 1.5.0 the old StringField type no longer compiles (type only, no runtime change).
  • The entry point is index.ts since search-service.ts is the class.

Stacked on #171 (item-metadata). The branch also carries #156 (result-type) until it merges.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CjuFdHEwY7MowcqkUxxudu

jbuckner and others added 3 commits October 8, 2026 16:40
Ported from @internetarchive/result-type 0.0.1.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CjuFdHEwY7MowcqkUxxudu
* origin/WEBDEV-9417-result-type:
  WEBDEV-9417: Migrate result-type into elements
Ported from @internetarchive/search-service 2.7.2.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CjuFdHEwY7MowcqkUxxudu
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.8.1

🚀 View preview at
https://internetarchive.github.io/elements/pr/pr-172/

Built to branch ghpages at 2026-10-09 01:23 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

A field or value of __proto__ or constructor reached Object.prototype or
the Object constructor. FilterMapBuilder now ignores those keys.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CjuFdHEwY7MowcqkUxxudu
@codecov-commenter

codecov-commenter commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.97125% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.00%. Comparing base (db7d0df) to head (374c06a).

Files with missing lines Patch % Lines
...arch-service/search-backend/base-search-backend.ts 87.50% 7 Missing and 5 partials ⚠️
...rch-service/mock-response-generator.test-helper.ts 52.94% 8 Missing ⚠️
...earch-service/responses/search-response-details.ts 89.33% 0 Missing and 8 partials ⚠️
...search-service/models/hit-types/web-archive-hit.ts 44.44% 2 Missing and 3 partials ⚠️
...h-service/models/hit-types/favorited-search-hit.ts 42.85% 3 Missing and 1 partial ⚠️
...vices/search-service/search-param-url-generator.ts 92.85% 0 Missing and 4 partials ⚠️
.../services/search-service/models/search-metadata.ts 92.85% 1 Missing and 1 partial ⚠️
src/services/search-service/filter-map-builder.ts 97.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           WEBDEV-9419-item-metadata     #172      +/-   ##
=============================================================
+ Coverage                      91.73%   92.00%   +0.27%     
=============================================================
  Files                            126      152      +26     
  Lines                           4075     4701     +626     
  Branches                         858     1026     +168     
=============================================================
+ Hits                            3738     4325     +587     
- Misses                           147      163      +16     
- Partials                         190      213      +23     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jbuckner

jbuckner commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

Second commit is a security fix beyond the port. FilterMapBuilder.addFilter('__proto__', 'x', ...) wrote to Object.prototype (same in the published 2.7.2). It now ignores __proto__, constructor and prototype keys, with tests. Worth fixing upstream and checking where collection-browser feeds user input into addFilter.

jbuckner and others added 2 commits October 8, 2026 18:11
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CjuFdHEwY7MowcqkUxxudu
Only `__proto__` is ignored, and lookups use own-property checks, so a
facet value like "prototype" or "constructor" still works. setFilterMap
goes through mergeFilterMap. Same tests as WEBDEV-9479.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CjuFdHEwY7MowcqkUxxudu
@jbuckner

jbuckner commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator Author

Updated the FilterMapBuilder fix to match iaux-search-service#103 (WEBDEV-9479): only __proto__ is ignored and lookups use own-property checks, so values like "prototype" still work. Same tests as #103.

constructor(options?: SearchBackendOptionsInterface) {
super(options);
this.servicePath =
options?.servicePath ?? '/services/search/beta/page_production';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

NOTE: Sure wish we could get rid of that /beta part of the path...

@bfalling bfalling left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

This branch has not been deployed

No deployments
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.

3 participants