-
Notifications
You must be signed in to change notification settings - Fork 1
Implement full Query By Example matching. #87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import {config} from '@bedrock/web'; | ||
|
|
||
| const isDebugEnabled = () => { | ||
| // Check config first | ||
| if(config.wallet?.debug?.queryByExample) { | ||
| return true; | ||
| } | ||
|
|
||
| // Node.js environment | ||
| if(typeof process !== 'undefined' && | ||
| process.env?.DEBUG_QUERY_BY_EXAMPLE === 'true') { | ||
| return true; | ||
| } | ||
|
|
||
| // Browser environment | ||
| if(typeof window !== 'undefined' && | ||
| window.DEBUG_QUERY_BY_EXAMPLE === true) { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| }; | ||
|
|
||
| export function debugLog(...args) { | ||
| if(isDebugEnabled()) { | ||
| console.log('[QBE DEBUG]', ...args); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,9 +9,11 @@ import {createDiscloseCryptosuite as createBbsDiscloseCryptosuite} from | |||||||||||||
| import {createDiscloseCryptosuite as createEcdsaSdDiscloseCryptosuite} from | ||||||||||||||
| '@digitalbazaar/ecdsa-sd-2023-cryptosuite'; | ||||||||||||||
| import {DataIntegrityProof} from '@digitalbazaar/data-integrity'; | ||||||||||||||
| import {debugLog} from './debug.js'; | ||||||||||||||
| import {documentLoader} from './documentLoader.js'; | ||||||||||||||
| import {ensureLocalCredentials} from './ageCredentialHelpers.js'; | ||||||||||||||
| import jsonpointer from 'json-pointer'; | ||||||||||||||
| import {matchCredentials} from './queryByExample.js'; | ||||||||||||||
| import {profileManager} from './state.js'; | ||||||||||||||
| import {supportedSuites} from './cryptoSuites.js'; | ||||||||||||||
| import {v4 as uuid} from 'uuid'; | ||||||||||||||
|
|
@@ -389,8 +391,25 @@ async function _getMatches({ | |||||||||||||
| // match any open badge achievement ID | ||||||||||||||
| // FIXME: add more generalized matching | ||||||||||||||
| result.matches = matches | ||||||||||||||
| .filter(_matchContextFilter({credentialQuery})) | ||||||||||||||
| .filter(_openBadgeFilter({credentialQuery})); | ||||||||||||||
| .filter(_matchContextFilter({credentialQuery})); | ||||||||||||||
|
|
||||||||||||||
| // Full query by example matching implemented via queryByExample module | ||||||||||||||
| // Process all credentials at once for efficiency | ||||||||||||||
| if(credentialQuery?.example) { | ||||||||||||||
|
|
||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| const allContents = result.matches.map(match => match.record.content); | ||||||||||||||
| const matchingContents = matchCredentials({ | ||||||||||||||
| credentials: allContents, queryByExample: credentialQuery | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| // Map results back to original records using reference comparison | ||||||||||||||
| result.matches = result.matches.filter(match => | ||||||||||||||
| matchingContents.includes(match.record.content) | ||||||||||||||
| ); | ||||||||||||||
|
Comment on lines
+406
to
+408
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style fix and could be more efficient:
Suggested change
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| result.matches = | ||||||||||||||
| result.matches.filter(_openBadgeFilter({credentialQuery})); | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| // create derived VCs for each match based on specific `credentialQuery` | ||||||||||||||
| const updatedQuery = {...vprQuery, credentialQuery}; | ||||||||||||||
|
|
@@ -399,7 +418,6 @@ async function _getMatches({ | |||||||||||||
| vprQuery: updatedQuery, matches: result.matches | ||||||||||||||
| }); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return results; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -459,6 +477,9 @@ function _matchContextFilter({credentialQuery}) { | |||||||||||||
| async function _matchQueryByExample({ | ||||||||||||||
| verifiablePresentationRequest, query, credentialStore, matches | ||||||||||||||
| }) { | ||||||||||||||
| debugLog('_matchQueryByExample called with query type:', | ||||||||||||||
| query.type); // DEBUG | ||||||||||||||
|
|
||||||||||||||
| matches.push(...await _getMatches({ | ||||||||||||||
| verifiablePresentationRequest, vprQuery: query, credentialStore | ||||||||||||||
| })); | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something minor to consider here about this export:
Our long term goal is to move the
queryByExamplecode to a separate module where anyone could import that API (from there) if desired. However, once we do that, we'd have this API surface here (if we continue to export this), that we will have to do backwards compatible support for until we do a major release to remove it. I expect that we're exporting this now for testing purposes.I think we'd probably export it with an underscore
_queryByExampleto signal this (and leave a comment that it is exposed for testing purposes only) ... and we wouldn't mention exposing this in API in the changelog. We can make that change now if you'd like, or we can just support it until we do a major change in the future -- it's not that big of a deal, but worth keeping this sort of future planning in mind when exporting new API, so thought I'd mention it.