Skip to content
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

termFromId and termToId do not roundtrip on quoted literals #312

Closed
jeswr opened this issue Nov 24, 2022 · 3 comments
Closed

termFromId and termToId do not roundtrip on quoted literals #312

jeswr opened this issue Nov 24, 2022 · 3 comments

Comments

@jeswr
Copy link
Collaborator

jeswr commented Nov 24, 2022

As demonstrated by this failing test doing a round-trip of termFromId and termToId causes Literal TermTypes to become NamedNode term types because the internal id without literals gets used in

return term.id;
and the result is that termFromId interprets it as a namedNode because it does not contain quotes.

We need to either (a) add quotes to literals missing them when Literals are created, or (b) remove

return term.id;
which is the optimisation causing the bug.

Note that NamedNodes can also become literals if we create a NamedNode of the form new NamedNode('"s"')

@RubenVerborgh
Copy link
Member

I think the problem is in the test: new Literal('abc') is not meaningful, because the (internal) Literal constructor (which is the Term constructor) requires a correct id as argument. This can also be seen here: main...jeswr:N3.js:bug/literal-roundtrip-failure#diff-8dc308ca806460d3608d347268016fb538008d8f9926902f331cc3553ababbfaR228

Note that this differs from the exported literal function (with which the test would succeed):

N3.js/src/N3DataFactory.js

Lines 339 to 342 in 520054a

function literal(value, languageOrDataType) {
// Create a language-tagged string
if (typeof languageOrDataType === 'string')
return new Literal(`"${value}"@${languageOrDataType.toLowerCase()}`);

@jeswr
Copy link
Collaborator Author

jeswr commented Nov 30, 2022

because the (internal) Literal constructor

These constructors are actually exported (c.f.

N3.js/src/index.js

Lines 38 to 39 in 520054a

NamedNode,
Literal,
). Perhaps it then worth removing these exports in the next mver (which I believe will be required in #311 anyway since there are a couple of minor breaking changes required to migrate from RDF* -> RDF-star)?

@RubenVerborgh
Copy link
Member

Good idea, following up in #313

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

No branches or pull requests

2 participants