Update expandMap and tests. #464
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidlehn
force-pushed
the
fix-expandmap
branch
from
0715d22 to
0967732
Compare
Codecov Report
@@ Coverage Diff @@
## main #464 +/- ##
===========================================
- Coverage 92.67% 25.24% -67.44%
===========================================
Files 23 23
Lines 2880 2880
===========================================
- Hits 2669 727 -1942
- Misses 211 2153 +1942
Continue to review full report at Codecov.
|
|
Hey @davidlehn, sorry totally missed your ping here, this looks like a great improvement over measuring the behaviour of the function in the tests. From memory I do remember noting that the function was called multiple times during expansion, I also remember tracing the various cases and them making sense, however unfortunately I did not write it down. |
|
@tplooker No problem. I'm dug into the issue now, so will come up with something to address what this was aiming at. I'm not quite sure what to do with the code as is. It seems awkward to write an I think I found part of the duplicated call issue. If anyone knows a use case for |
- Remove spaces in names by using string concatination.
- Count and check all calls of various types.
- Current use is for custom handling of warnings. - Replay events when using cached contexts. - Flexible chainable handlers. Can use functions, arrays, object code/function map shortcut, or a combination of these. - Use handler for current context warnings.
- Track and check event counts. - Add more expansionMap tests.
- Add 'relateive IRI after expansion' warning event. - Add 'invalid property expansion' warning event. - Update tests to check call counts.
- Add basic event handlers (can be used to construct more complex handlers) - log all events and continue - fail on unhandled events - fail on unacceptable events - log warnings and continue - strict mode to fail on all current warnings - Add default event handler feature.
Allow checking log of events. Adds more detailed checking to ensure the proper sequence of events and details occur.
- Add `safe` flag to call a special safe mode event handler. - Update events API and usage. - Add `hasEventHandler` for call site optimization. - Update handlers.
- Marked as experimental.
Merge both sections to avoid duplication since they do, or will, cover similar situations.
- Setup eventHandler defaults once at top level. - Use simple existence check to optimize if handlers need to be called. - Better naming for event capture handler.
- Reduce boilerplate for common testing pattern. - Pass in params and parts to test.
- Rename handlers. - Add strict handler docs. - Add `JsonLdEvent` type to events. - Update `safeEventHandler` to handle new event codes. - Add more event emitting points. - Update event testing and tests. - Add safe/strict testing flags. - Fix bugs and cleanup.
- Fix log typo. - Update tests.
- Add event filter to avoid reletive reference event while expanding to check if a value is `@json`. - Update tests. - Add safe/strict testing flags.
- Emit event when invalid. - Add tests.
- Move 'invalid reserved term' to safe validator. - Improve asserts. - Update test names.
- Unused as everything moved into "safe" mode.
- Add another langage check. - Add tests.
- Update event code naming. Similar to JSON-LD error code style. Code name is just the condition detected, not the action that might be taken. - Relative reference events now at call sites. - No-value events now at call sites. - Remove some special-case event handlers. Not needed when events happen outside `_expandIri`. - **BREAKING**: Handle special non-normaitve spec edge case where value for `@id` can expand into null. Behavior changing to *not* output invalid JSON-LD. This can change triples in odd edge cases. - Add eventCodeLog test feature to test sequence of events codes. - Update tests.
- More tests updates and fixes.
- Checking for flag. - Unsure how to use, but some code exists for future use.
- Leave comments about the `{"@id": null}` edge case tests but still
test with current behavior. Will address the issue later.
- Updated handling of edge case null values due to previous changes.
In the case where an `@id` is not a string, consider an object a blank node. This is needed to handle a crashing error where the code assumes ids are valid strings. The error could occur with direct invalid input, or in the edge case where non-normative invalid intermediate expanded JSON-LD is generated.
- Better `expected` checking. - Fix test flag typos.
- Add various toRDF relative reference events. - Update test support. - Add tests.
- The `eventHandler` design and implementation has not been finalized for external use yet.
- **BREAKING**: Remove `compactionMap` and `expansionMap`. Their known use cases are addressed with "safe mode" and future planned features. - Update docs. - Update tests. - Add partial event log tester to simplify checking only some fields. - Add `prepending` events for `@base` and `@vocab`. Likely to be removed since other events better expose what the real data issues are. May return for a debug mode. - Fix some tests.
- More appropriate events are already emitted for the real data issues. - This may be useful in the future for a debug mode.
Fix rebase issues.
|
This changed and grew quite a bit in another branch. Merging back to main through this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a continuation of the expansionMap work from:
#452
@tplooker This likely needs your input.
That expansionMap work was merged but never released. I wanted to get it out there and active for use in VCs and elsewhere to at least try and check for dropped types and such. It turns out I don't know how to use this feature!
expansionMapis now called lots more than before. It has many variations, and the lack of documentation is now rather painful, since it's not obvious what theexpansionMapcode should do for what situations.In the case of VCs, it seems like type scoped properties like
credentialSubjectend up with a couple calls, but maybe it shouldn't? Perhaps the callback is getting called while the algorithm does normal processing rather than just when there's an issue?In any case, I updated the tests to count all the calls to
expansionMap, and (maybe) what they were for. Just so it's clear in examples what is happening.I do think some more test cases are needed, if for nothing else than as an example. Have various inputs that have unknown properties, types, etc, and an example
expansionMaphandler that knows how to reject all that. Basically a common use case for strict signing software.