-
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 1 commit
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 | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ import {DataIntegrityProof} from '@digitalbazaar/data-integrity'; | |||||||||||||||||
| 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'; | ||||||||||||||||||
|
|
@@ -390,6 +391,7 @@ async function _getMatches({ | |||||||||||||||||
| // FIXME: add more generalized matching | ||||||||||||||||||
| result.matches = matches | ||||||||||||||||||
| .filter(_matchContextFilter({credentialQuery})) | ||||||||||||||||||
| .filter(_matchQueryByExampleFilter({credentialQuery})) | ||||||||||||||||||
| .filter(_openBadgeFilter({credentialQuery})); | ||||||||||||||||||
|
|
||||||||||||||||||
| // create derived VCs for each match based on specific `credentialQuery` | ||||||||||||||||||
|
|
@@ -418,6 +420,33 @@ function _handleLegacyDraftCryptosuites({presentation}) { | |||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Creates a filter function that matches credentials against a Query By Example | ||||||||||||||||||
| * specification using the reusable queryByExample module. This function acts | ||||||||||||||||||
| * as an adapter between the bedrock wallet's filter chain pattern and the | ||||||||||||||||||
| * reusable matchCredentials function. | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param {object} options - The options to use. | ||||||||||||||||||
| * @param {object} options.credentialQuery - The credential query containing | ||||||||||||||||||
| * the example to match against. | ||||||||||||||||||
| * | ||||||||||||||||||
| * @returns {Function} A filter function that returns true if the credential | ||||||||||||||||||
| * matches the Query By Example specification. | ||||||||||||||||||
|
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
Contributor
Author
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. That function got removed as not needed in this commit - 4d42d81 |
||||||||||||||||||
| */ | ||||||||||||||||||
| function _matchQueryByExampleFilter({credentialQuery}) { | ||||||||||||||||||
| const {example} = credentialQuery; | ||||||||||||||||||
| if(!(example && typeof example === 'object')) { | ||||||||||||||||||
| // no example to match against, allow all credentials | ||||||||||||||||||
| return () => true; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return ({record: {content}}) => { | ||||||||||||||||||
| // Use reusable module to check if this single credential matches | ||||||||||||||||||
| const matches = matchCredentials([content], 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. It looks like Hmm, but I see below that there's savings in only having to process
Open to other better ideas too (of course), but idea 1 in the list above (vs. any of the others in that list) keeps the interfaces and separation of concerns the cleanest, I think.
Contributor
Author
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.
Attempt to address your concern and going with option 1 in this commit - 4d42d81 |
||||||||||||||||||
| return matches.length > 0; | ||||||||||||||||||
| }; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| function _openBadgeFilter({credentialQuery}) { | ||||||||||||||||||
| return ({record: {content}}) => { | ||||||||||||||||||
| const {example} = credentialQuery; | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,243 @@ | ||||||
| /*! | ||||||
| * Copyright (c) 2018-2025 Digital Bazaar, Inc. All rights reserved. | ||||||
| */ | ||||||
| import JsonPointer from 'json-pointer'; | ||||||
|
|
||||||
| /** | ||||||
| * Matches credentials against a Query By Example specification. | ||||||
| * This function processes the full QueryByExample matching on a list of VCs | ||||||
| * that have already been preliminarily filtered (e.g., by top-level type). | ||||||
| * | ||||||
| * @param {Array} credentials - Array of credential objects to match against. | ||||||
| * @param {object} queryByExample - The Query By Example specification. | ||||||
| * @param {object} queryByExample.example - The example credential structure | ||||||
| * to match against. | ||||||
| * | ||||||
| * @returns {Array} Array of credentials that match the Query By Example | ||||||
| * specification. | ||||||
| */ | ||||||
| export function matchCredentials(credentials, queryByExample) { | ||||||
|
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. Use named params in exported APIs:
Suggested change
Contributor
Author
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. |
||||||
| const {example} = queryByExample; | ||||||
| if(!(example && typeof example === 'object')) { | ||||||
| // no example to match against, return all credentials | ||||||
| return credentials; | ||||||
| } | ||||||
|
|
||||||
| // Convert example to JSON pointers, excluding @context as it's handled | ||||||
| // separately | ||||||
| const expectedPointers = _convertExampleToPointers(example); | ||||||
|
Comment on lines
+105
to
+107
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. But
Contributor
Author
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.
Attempt to address your concern in this commit - c8fad33 |
||||||
|
|
||||||
| // DEBUG - remove after testing | ||||||
| // console.log('Query By Example Debug:', { | ||||||
| // example, | ||||||
| // expectedPointers, | ||||||
| // credentialCount: credentials.length | ||||||
|
bparth24 marked this conversation as resolved.
Outdated
|
||||||
| // }); | ||||||
|
|
||||||
| if(expectedPointers.length === 0) { | ||||||
| // no meaningful fields to match, return all credentials | ||||||
| return credentials; | ||||||
| } | ||||||
|
|
||||||
| return credentials.filter(credential => { | ||||||
| // Check each pointer against the credential content | ||||||
| return expectedPointers.every(({pointer, expectedValue}) => { | ||||||
| try { | ||||||
| const actualValue = JsonPointer.get(credential, pointer); | ||||||
| const result = _valuesMatch(actualValue, expectedValue); | ||||||
|
Comment on lines
+118
to
+119
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. I think we might need to be more careful here with arrays? We should make sure there are tests with arrays that have objects nested within them that are also arrays still work. This code might also benefit from using that other code I linked to where just the "deepest" pointers are considered, I'm not sure. In other words, it might be best to:
Contributor
Author
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.
Attempt to address your concern in this commit - c8fad33 |
||||||
|
|
||||||
| // DEBUG - remove after testing | ||||||
| // console.log('Matching:', | ||||||
| // {pointer, expectedValue, actualValue, result}); | ||||||
|
|
||||||
| return result; | ||||||
| } catch(e) { | ||||||
| // If pointer doesn't exist in credential, it's not a match | ||||||
| console.log('Pointer error:', pointer, e.message); | ||||||
| return false; | ||||||
| } | ||||||
| }); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Converts an example to an array of JSON pointer/value pairs. | ||||||
| * This function recursively processes the example object to extract all | ||||||
| * field paths and their expected values, excluding @context which is | ||||||
| * handled separately in the filtering pipeline. | ||||||
| * | ||||||
| * @param {object} example - The example object from Query By Example. | ||||||
| * | ||||||
| * @returns {Array<object>} Array of objects with {pointer, expectedValue} | ||||||
| * where pointer is a JSON pointer string (e.g., '/credentialSubject/name') | ||||||
| * and expectedValue is the expected value at the path. | ||||||
| */ | ||||||
| function _convertExampleToPointers(example) { | ||||||
| const pointers = []; | ||||||
|
|
||||||
| // Create a copy without @context since it's handled by _matchContextFilter | ||||||
| const exampleWithoutContext = {...example}; | ||||||
| delete exampleWithoutContext['@context']; | ||||||
|
|
||||||
| // Convert to JSON pointer dictionary and extract pointer/value pairs | ||||||
| try { | ||||||
| const dict = JsonPointer.dict(exampleWithoutContext); | ||||||
| for(const [pointer, value] of Object.entries(dict)) { | ||||||
| // Skip empty objects, arrays, or null/undefined values | ||||||
| if(_isMatchableValue(value)) { | ||||||
| pointers.push({ | ||||||
| pointer, | ||||||
| expectedValue: value | ||||||
| }); | ||||||
| } | ||||||
| } | ||||||
|
Comment on lines
+327
to
+338
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. We have this code over here: bedrock-web-wallet/lib/presentations.js Line 238 in 212ac42
Can we find a way to expose it as an exported API in this module that this module can also use internally? There might need to be a few different options when converting an object to pointers, but it seems like we could get more reuse here. There might be something common we can do with this as well: https://github.com/digitalbazaar/di-sd-primitives/blob/v3.1.0/lib/select.js#L9
Contributor
Author
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.
Attempt to address your concern in this commit - c8fad33 |
||||||
| } catch(e) { | ||||||
| // If JSON pointer conversion fails, return empty array | ||||||
| console.warn('Failed to convert example to JSON pointers:', e); | ||||||
| return []; | ||||||
| } | ||||||
| return pointers; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Determines if a value is suitable for matching. We skip empty objects, | ||||||
| * empty arrays, null, undefined, and other non-meaningful values. | ||||||
| * | ||||||
| * @param {*} value - The value to check. | ||||||
| * | ||||||
| * @returns {boolean} True if the value should be used for matching. | ||||||
| */ | ||||||
| function _isMatchableValue(value) { | ||||||
| // Skip null, undefined | ||||||
| if(value == null) { | ||||||
| return false; | ||||||
| } | ||||||
|
Comment on lines
+356
to
+359
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. Hmm, I think we need to be careful here -- perhaps a
Contributor
Author
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.
Attempt to address your concern in this commit - c8fad33 |
||||||
|
|
||||||
| // Skip empty arrays | ||||||
| if(Array.isArray(value) && value.length === 0) { | ||||||
| return false; | ||||||
| } | ||||||
|
Comment on lines
+361
to
+364
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. Hmm, I think empty arrays are wildcards that mean "any array will match". Clearly under-specified in the spec, but I think this was the aim.
Contributor
Author
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. |
||||||
|
|
||||||
| // Skip empty objects | ||||||
| if(typeof value === 'object' && !Array.isArray(value) && | ||||||
| Object.keys(value).length === 0) { | ||||||
| return false; | ||||||
| } | ||||||
|
Comment on lines
+367
to
+370
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. I think empty objects are considered "wildcards" -- meaning, any value will match. Clearly under-specified in the spec, but I think this was the aim.
Contributor
Author
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. |
||||||
|
|
||||||
| // All other values (strings, numbers, booleans, non-empty arrays/objects) | ||||||
| return true; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Determines if an actual value from a credential matches an expected value | ||||||
| * from a Query By Example specification. This handles various matching | ||||||
| * scenarios including arrays, different types, and normalization. | ||||||
| * | ||||||
| * @param {*} actualValue - The value found in the credential. | ||||||
| * @param {*} expectedValue - The expected value from the example. | ||||||
| * | ||||||
| * @returns {boolean} True if the values match according to Query By Example | ||||||
| * matching rules. | ||||||
| */ | ||||||
| function _valuesMatch(actualValue, expectedValue) { | ||||||
| // Handle null/undefined cases | ||||||
| if(actualValue == null && expectedValue == null) { | ||||||
| return true; | ||||||
| } | ||||||
| if(actualValue == null || expectedValue == null) { | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| // If both are arrays, check if they have common elements | ||||||
| if(Array.isArray(actualValue) && Array.isArray(expectedValue)) { | ||||||
| return _arraysHaveCommonElements(actualValue, expectedValue); | ||||||
| } | ||||||
|
|
||||||
| // If actual is array but expected is single value, check if array | ||||||
| // contains the value | ||||||
| if(Array.isArray(actualValue) && !Array.isArray(expectedValue)) { | ||||||
| return actualValue.some(item => _valuesMatch(item, expectedValue)); | ||||||
| } | ||||||
|
|
||||||
| // If expected is array but actual is single value, check if actual | ||||||
| // is in expected | ||||||
| if(!Array.isArray(actualValue) && Array.isArray(expectedValue)) { | ||||||
| return expectedValue.some(item => _valuesMatch(actualValue, item)); | ||||||
| } | ||||||
|
|
||||||
| // For objects, do deep equality comparison | ||||||
| if(typeof actualValue === 'object' && typeof expectedValue === 'object') { | ||||||
| return _objectsMatch(actualValue, expectedValue); | ||||||
| } | ||||||
|
|
||||||
| // For primitive values, do strict equality with string normalization | ||||||
| return _primitiveValuesMatch(actualValue, expectedValue); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Checks if two arrays have any common elements. | ||||||
| * | ||||||
| * @param {Array} arr1 - First array. | ||||||
| * @param {Array} arr2 - Second array. | ||||||
| * | ||||||
| * @returns {boolean} True if arrays have at least one common element. | ||||||
| */ | ||||||
| function _arraysHaveCommonElements(arr1, arr2) { | ||||||
| return arr1.some(item1 => | ||||||
| arr2.some(item2 => _valuesMatch(item1, item2)) | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Performs deep equality comparison for objects. | ||||||
| * | ||||||
| * @param {object} obj1 - First object. | ||||||
| * @param {object} obj2 - Second object. | ||||||
| * | ||||||
| * @returns {boolean} True if objects are deeply equal. | ||||||
| */ | ||||||
| function _objectsMatch(obj1, obj2) { | ||||||
| const keys1 = Object.keys(obj1); | ||||||
| const keys2 = Object.keys(obj2); | ||||||
|
|
||||||
| // Check if they have the same number of keys | ||||||
| if(keys1.length !== keys2.length) { | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| // Check if all keys and values match | ||||||
| return keys1.every(key => | ||||||
| keys2.includes(key) && _valuesMatch(obj1[key], obj2[key]) | ||||||
| ); | ||||||
|
Comment on lines
+192
to
+463
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. I don't think the object matching is "must match this object exactly", but rather, "must include these fields and values". So I suspect this function (or something else) needs an adjustment for this. In general, if you think about this query mechanism visually, the "example" is to be placed as an overlay on a possible candidate for a match -- and as long as everything in the example "overlaps" with something in the candidate, then the candidate is considered a match. 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. Would it be clearer if we provided wildcard values? Because the value thing being an actual value...which is then not used in the match...is very confusing.
Contributor
Author
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.
Attempt to address your concern in this commit - c8fad33 |
||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Compares primitive values (string, numbers, booleans) with appropriate | ||||||
| * normalization and type coercion. | ||||||
| * | ||||||
| * @param {*} actual - Actual primitive value. | ||||||
| * @param {*} expected - Expected primitive value. | ||||||
| * | ||||||
| * @returns {boolean} True if primitive value match. | ||||||
| */ | ||||||
| function _primitiveValuesMatch(actual, expected) { | ||||||
| // Strict equality first (handles numbers, booleans, exact strings) | ||||||
| if(actual === expected) { | ||||||
| return true; | ||||||
| } | ||||||
|
|
||||||
| // String comparison with normalization | ||||||
| if(typeof actual === 'string' && typeof expected === 'string') { | ||||||
| // Trim whitespace and compare case-sensitively | ||||||
| return actual.trim() === expected.trim(); | ||||||
| } | ||||||
|
|
||||||
| // Type coercion for string/number comparisons | ||||||
| if((typeof actual === 'string' && typeof expected === 'number') || | ||||||
| (typeof actual === 'number' && typeof expected === 'string')) { | ||||||
| return String(actual) === String(expected); | ||||||
| } | ||||||
|
|
||||||
| // No match | ||||||
| return false; | ||||||
| } | ||||||
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.