termToNQ serializes Collection

[ericprud]

When re-building and trying out rdflib, I couldn't fetch data.

The last line of fetcher.saveRequestMetadata (fetcher.ts:1647) adds a collection:

kb.add(req, this.ns.link('status'), kb.collection(), this.appNode)

IndexedForumula.add (store.tx:453) de-duplicates with:

    if (this.holds(subj, pred, objNode, why)) { // Takes time but saves duplicates

IndexedForumula.statementsMatching canonicalizes to search in index:

pattern[p] = this.canon(Node.fromValue(pat[p]))

CanonicalDataFactory.termToNQ (canonical-data-factor.ts:192) throws

Uncaught Error: Can't serialize nonstandard term type (was 'Collection')
    at Object.termToNQ (canonical-data-factory.ts:192)
    at Object.id (canonical-data-factory.ts:111)
    at IndexedFormula.id (formula.ts:245)
    at IndexedFormula.canon (store.ts:498)
    at IndexedFormula.statementsMatching (store.ts:1056)
    at IndexedFormula.anyStatementMatching (formula.ts:231)
    at IndexedFormula.holds (formula.ts:672)
    at IndexedFormula.add (store.ts:453)
    at Fetcher.saveRequestMetadata (fetcher.ts:1647)
    at Fetcher.fetchUri (fetcher.ts:1090)

This PR adds a case for Collection:

``` diff
+      case CollectionTermType:
+        return '(' + term.elements.map(t => this.termToNQ(t)).join(' ') + ')'

It's less efficient than de-duping with a more formal recursive hash function, which will hurt when serializing large lists, but that's an architecture issue.

(.gitignore includes dist/rdflib.min.js so I assume was wasn't supposed to include a build in the PR.)

[megoth]

I'm sorry that this is taking some time. I'm trying to get some PRs reviewed so we can release a new version of rdflib, including this.

I was wondering if we might want to write a test to cover this?

[ericprud]

I was wondering if we might want to write a test to cover this?

In principle, yes, but the tests have probably changed since I last forgot how they work. Also, RDFJS has replaced my memory of the rdflib.js interface.

I'd expect that something a simple as:

const Namespace = require('./namespace')
const Ns = {
  ex: Namespace('http://a.example/ns#'),
}
kb.add(ns.ex('s'), ns.ext('p'), kb.collection(), ???)

would do the job.

[timbl]

This seems like a plan. Maybe we should look in future at more sophisticated ways of checking for statements with nested complex types. (which for nested graphs would involve graph hashing).. this looks good to me now