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

Graph "scrubbing" is too agressive #45

Closed
BigBlueHat opened this issue Jan 13, 2017 · 7 comments · Fixed by #53
Closed

Graph "scrubbing" is too agressive #45

BigBlueHat opened this issue Jan 13, 2017 · 7 comments · Fixed by #53

Comments

@BigBlueHat
Copy link
Member

BigBlueHat commented Jan 13, 2017

I'm working on a test for this (and understanding more about what changed), but here's the set of steps for playing with on the playground.

  1. empty graph
  2. put
{"@context": "https://schema.org/", "@id": "http://bigbluehat.com/#", "name": "BigBlueHat"}
  1. expect(triples).to.have.length(1) is true
  2. put
{"@context": "https://schema.org/", "name": "BigBlueHat"}
  1. expect(triples).to.have.length(2) is false...there's only one. It replaced (maybe?) the original statement that had the ID. 😕
  2. put the JSON-LD from # 2 (above)
  3. expect(triples).to.have.length(2) is true...um...OK...
  4. put the JSON-LD from # 4 (above)...trying again
  5. expect(triples).to.have.length(3) is true...gah! WAT?! I'm back down to 1 😢

Obviously...I'm missing something important here... 😀

@jmatsushita thoughts?

@jmatsushita
Copy link
Collaborator

jmatsushita commented Jan 13, 2017

The original put is quite aggressive which is why I've started contributing here and added a preserve option, you might want to use it by default when you open the db. I'm not exactly sure about what happens with step 4 though. It's likely it will be addressed by using preserve: true but it's still a bit mysterious.

@BigBlueHat
Copy link
Member Author

Yeah. I'm adding it as an option to the playground per put().

However, it's currently false in the playground. Which means I've also found this bit of mystery (which you can try in the console on the playground):

  1. Load some stuff into the graph (aka. click the "put" button).
  2. Run this in the console:
db.jsonld.put('{"@context": "https://schema.org/", "name": "BigBlueHat"}', console.log.bind(console));
  1. Say goodbye the entirety of the graph...except that statement...

Regardless of the above (which I assume are bugs), I think setting preserve to true by default is prudent. I'd much rather have messy data than no data. 😄

@jmatsushita
Copy link
Collaborator

jmatsushita commented Jan 13, 2017

Yes I think having preserve to true by default makes sense too. @mcollina Should we go all the way here? (i.e. change the default to preserve true)

@jmatsushita
Copy link
Collaborator

@BigBlueHat you might be interested in #46 as well

@jmatsushita
Copy link
Collaborator

Ok I've been able to reproduce the problem @BigBlueHat and since I can build the playground now I'll work on a fix.

@jmatsushita
Copy link
Collaborator

Well turns out there were several things at play here:

  • preserve did need to be true.
  • base shouldn't be put to _: which basically means everything is a blank node (and blank node really aren't event supposed to be persisted, or let's say it's not trivial how they should be)
  • there's a bug with setting "@context" as a string. Setting "@context" as a string timeout #47

So if you do correct the options and use a "@context" object then things work as expected. I'll get to the fix now.

@BigBlueHat
Copy link
Member Author

New Playground with options settings is up!

Consequently, you'll find some more interesting things...

Basically, there's all kinds of crazy variations depending on where @base shows up, what @id is set to, or if any of its missing.

Right now, the playground is passing in {base: "", preserve: true} by default, fwiw.

Oh, and db and app can be access from the console. app.displayTriples({}) is your friend there. 😄

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 a pull request may close this issue.

2 participants