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

use empty string as base to support relative IRIs #182

Merged
merged 2 commits into from
Sep 23, 2019

Conversation

bergos
Copy link
Member

@bergos bergos commented Sep 16, 2019

When no baseIRI is given, relative IRIs are prepended with undefined in the current version. I guess this is not an expected behavior. With this PR, relative IRIs are resolved against an empty string, so they are kept without changes. In an older version it was already like this and it's still possible with a workaround ({ baseIRI: new String('') }).

This is useful for LDP to POST to a container with relative IRIs and some other more exotic use cases.

I wasn't sure how to get the test running without big changes. @RubenVerborgh would be great if you can give me a hint how you would like to have it implemented or you just take over that part.

@RubenVerborgh
Copy link
Member

RubenVerborgh commented Sep 16, 2019

Hi @bergos,

When no baseIRI is given, relative IRIs are prepended with undefined in the current version. I guess this is not an expected behavior.

Yeah… I honestly don't know what the best is to do here. The problem is that we are currently generating invalid NamedNodes. Should we just fail when a Turtle document has relative IRIs but no base?

@bergos
Copy link
Member Author

bergos commented Sep 16, 2019

I think it should be possible to get "invalid" NamedNodes. We kept that open in the RDF/JS Data Model spec for a good reason. The use cases are rare, but there are a few.

POST request to LDPC

The spec explicitly allows null relative URIs (5.2.3.7). On the server side the URI of the new resource is used as base IRI, but the logic could look like this:

  • parse request content
  • analyze content
  • create new resource URI based on the content

That means the content must be parsed before there is a base IRI. A dummy IRI could be used, but that's ugly from my point of view. Also one could argue that in LDP resource URIs should be independent of the content, but I would like to use the same logic for Hydra applications, which makes a lot of things much easier.

That means you would make my life a little bit harder if you fix the gap of the workaround and don't allow null/empty string base IRIs.

An alternative could be something like a strict mode or explicitly check for empty strings to enable this feature and only throw an error if the base IRI is === undefined.

@RubenVerborgh
Copy link
Member

Fair enough.

I wasn't sure how to get the test running without big changes.

Perhaps use fromId directly in the code you added, and add a check in shouldParse to see if they are strings (and skip if not)?

@bergos
Copy link
Member Author

bergos commented Sep 17, 2019

@RubenVerborgh I fixed the test by directly calling fromId() and a small fix in shouldParse(). The last commit also contains a small code style fix.

@RubenVerborgh RubenVerborgh merged commit 240656d into rdfjs:master Sep 23, 2019
@RubenVerborgh
Copy link
Member

Thanks @bergos, I appreciate it!

@bergos bergos mentioned this pull request Dec 2, 2019
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.

2 participants