Repository: cayleygraph/cayley
Pull request #914 (2020-01-30 22:26:17)

LinkedQL: Better Finals
 - Add `@id` to Select and SelectFirst steps making the output a valid JSON-LD document
 - In Documents use a gojsonld API on top of TagsIterator to frame the results to documents instead of custom logic

### Todo
 - [x] `iter_docs` should return a single graph map instead of slice of maps.
 - [x] Extract datasets merging logic
 - [x] Extract `fromValue()`

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/cayleygraph/cayley/914)
<!-- Reviewable:end -->



Patch:
diff --git a/go.mod b/go.mod
index fe266ab..734df3b 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,7 @@ go 1.12
 
 require (
 	github.com/badgerodon/peg v0.0.0-20130729175151-9e5f7f4d07ca
-	github.com/cayleygraph/quad v1.2.3
+	github.com/cayleygraph/quad v1.2.4
 	github.com/cockroachdb/apd v1.1.0 // indirect
 	github.com/containerd/continuity v0.0.0-20190426062206-aaeac12a7ffc // indirect
 	github.com/coreos/bbolt v1.3.3 // indirect
@@ -36,6 +36,7 @@ require (
 	github.com/opencontainers/selinux v1.0.0 // indirect
 	github.com/pelletier/go-toml v1.4.0 // indirect
 	github.com/peterh/liner v0.0.0-20170317030525-88609521dc4b
+	github.com/piprate/json-gold v0.3.0
 	github.com/prometheus/client_golang v0.9.3
 	github.com/rogpeppe/go-internal v1.5.0 // indirect
 	github.com/satori/go.uuid v1.2.0 // indirect
diff --git a/go.sum b/go.sum
index c298d4a..4614a9a 100644
--- a/go.sum
+++ b/go.sum
@@ -27,6 +27,8 @@ github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4=
 github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
 github.com/cayleygraph/quad v1.2.3 h1:2HXwZGElLBuFLz8/6FdfWtil2W2gdfvYxNzS337SOwo=
 github.com/cayleygraph/quad v1.2.3/go.mod h1:XOianlRdDK5Upno/6svE6APe/wD8XgYrL9smqK875nU=
+github.com/cayleygraph/quad v1.2.4 h1:+6u09WxA7zg9ILonK8DChwzWKLKsDkjyvX+CXXhI/mM=
+github.com/cayleygraph/quad v1.2.4/go.mod h1:XOianlRdDK5Upno/6svE6APe/wD8XgYrL9smqK875nU=
 github.com/cenkalti/backoff v2.1.1+incompatible h1:tKJnvO2kl0zmb/jA5UKAt4VoEVw1qxKWjE/Bpp46npY=
 github.com/cenkalti/backoff v2.1.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
 github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
diff --git a/query/linkedql/iter_docs.go b/query/linkedql/iter_docs.go
index 335f015..a7795b6 100644
--- a/query/linkedql/iter_docs.go
+++ b/query/linkedql/iter_docs.go
@@ -4,49 +4,48 @@ import (
 	"context"
 
 	"github.com/cayleygraph/cayley/query"
-	"github.com/cayleygraph/quad"
+	"github.com/piprate/json-gold/ld"
 )
 
-var _ query.Iterator = (*DocumentIterator)(nil)
-
-type document = map[string]interface{}
-type properties = map[string][]interface{}
-type idToProperties = map[quad.Value]properties
+var (
+	_ query.Iterator = (*DocumentIterator)(nil)
+)
 
 // DocumentIterator is an iterator of documents from the graph
 type DocumentIterator struct {
-	tagsIt     *TagsIterator
-	ids        []quad.Value
-	properties idToProperties
-	current    int
+	tagsIt    *TagsIterator
+	dataset   *ld.RDFDataset
+	err       error
+	exhausted bool
 }
 
 // NewDocumentIterator returns a new DocumentIterator for a QuadStore and Path.
 func NewDocumentIterator(valueIt *ValueIterator) *DocumentIterator {
 	tagsIt := &TagsIterator{ValueIt: valueIt, Selected: nil}
-	return &DocumentIterator{tagsIt: tagsIt, current: -1}
+	return &DocumentIterator{tagsIt: tagsIt, exhausted: false}
+}
+
+func (it *DocumentIterator) getDataset(ctx context.Context) (*ld.RDFDataset, error) {
+	d := ld.NewRDFDataset()
+	for it.tagsIt.Next(ctx) {
+		err := it.tagsIt.addResultsToDataset(d)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return d, nil
 }
 
 // Next implements query.Iterator.
 func (it *DocumentIterator) Next(ctx context.Context) bool {
-	if it.properties == nil {
-		it.properties = make(idToProperties)
-		for it.tagsIt.Next(ctx) {
-			id := it.tagsIt.ValueIt.Value()
-			tags := it.tagsIt.getTags()
-			it.ids = append(it.ids, id)
-			for k, v := range tags {
-				m, ok := it.properties[id]
-				if !ok {
-					m = make(properties)
-					it.properties[id] = m
-				}
-				m[k] = append(m[k], v)
-			}
+	if !it.exhausted {
+		d, err := it.getDataset(ctx)
+		if err != nil {
+			it.err = err
+		} else {
+			it.dataset = d
 		}
-	}
-	if it.current < len(it.ids)-1 {
-		it.current++
+		it.exhausted = true
 		return true
 	}
 	return false
@@ -54,25 +53,13 @@ func (it *DocumentIterator) Next(ctx context.Context) bool {
 
 // Result implements query.Iterator.
 func (it *DocumentIterator) Result() interface{} {
-	if it.current >= len(it.ids) {
-		return nil
-	}
-	id := it.ids[it.current]
-	// FIXME(iddan): don't cast to string when collation is Raw
-	var sid string
-	switch val := id.(type) {
-	case quad.IRI:
-		sid = string(val)
-	case quad.BNode:
-		sid = val.String()
-	}
-	d := document{
-		"@id": sid,
-	}
-	for k, v := range it.properties[id] {
-		d[k] = v
+	context := make(map[string]interface{})
+	opts := ld.NewJsonLdOptions("")
+	c, err := datasetToCompact(it.dataset, context, opts)
+	if err != nil {
+		it.err = err
 	}
-	return d
+	return c
 }
 
 // Err implements query.Iterator.
diff --git a/query/linkedql/iter_tags.go b/query/linkedql/iter_tags.go
index ef7e870..6eef2d7 100644
--- a/query/linkedql/iter_tags.go
+++ b/query/linkedql/iter_tags.go
@@ -2,10 +2,13 @@ package linkedql
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/cayleygraph/cayley/graph/refs"
 	"github.com/cayleygraph/cayley/query"
+	"github.com/cayleygraph/quad"
 	"github.com/cayleygraph/quad/jsonld"
+	"github.com/piprate/json-gold/ld"
 )
 
 var _ query.Iterator = (*TagsIterator)(nil)
@@ -13,8 +16,20 @@ var _ query.Iterator = (*TagsIterator)(nil)
 // TagsIterator is a result iterator for records consisting of selected tags
 // or all the tags in the query.
 type TagsIterator struct {
-	ValueIt  *ValueIterator
-	Selected []string
+	ValueIt   *ValueIterator
+	Selected  []string
+	ExcludeID bool
+	err       error
+}
+
+// NewTagsIterator creates a new TagsIterator
+func NewTagsIterator(valueIt *ValueIterator, selected []string, excludeID bool) TagsIterator {
+	return TagsIterator{
+		ValueIt:   valueIt,
+		Selected:  selected,
+		ExcludeID: excludeID,
+		err:       nil,
+	}
 }
 
 // Next implements query.Iterator.
@@ -22,32 +37,76 @@ func (it *TagsIterator) Next(ctx context.Context) bool {
 	return it.ValueIt.Next(ctx)
 }
 
-func (it *TagsIterator) getTags() map[string]interface{} {
+func (it *TagsIterator) addQuadFromRef(dataset *ld.RDFDataset, subject ld.Node, tag string, ref refs.Ref) error {
+	p := ld.NewIRI(tag)
+	o, err := jsonld.ToNode(it.ValueIt.getName(ref))
+	if err != nil {
+		return err
+	}
+	q := ld.NewQuad(subject, p, o, "")
+	dataset.Graphs["@default"] = append(dataset.Graphs["@default"], q)
+	return nil
+}
+
+func getSubject(it *TagsIterator) (ld.Node, error) {
+	r := it.ValueIt.scanner.Result()
+	identifier, ok := it.ValueIt.getName(r).(quad.Identifier)
+	if !ok {
+		return nil, fmt.Errorf("Expected subject to be an entity identifier but instead received: %v", identifier)
+	}
+	return jsonld.ToNode(identifier)
+}
+
+func (it *TagsIterator) addResultsToDataset(dataset *ld.RDFDataset) error {
+	s, err := getSubject(it)
+	if err != nil {
+		return err
+	}
+
 	refTags := make(map[string]refs.Ref)
+
 	it.ValueIt.scanner.TagResults(refTags)
 
-	tags := make(map[string]interface{})
-	// FIXME(iddan): only convert when collation is JSON/JSON-LD, leave as Ref otherwise
 	if len(it.Selected) == 0 {
 		for tag, ref := range refTags {
-			tags[tag] = jsonld.FromValue(it.ValueIt.getName(ref))
+			it.addQuadFromRef(dataset, s, tag, ref)
 		}
 	} else {
 		for _, tag := range it.Selected {
-			tags[tag] = jsonld.FromValue(it.ValueIt.getName(refTags[tag]))
+			it.addQuadFromRef(dataset, s, tag, refTags[tag])
 		}
 	}
 
-	return tags
+	return nil
 }
 
 // Result implements query.Iterator.
 func (it *TagsIterator) Result() interface{} {
-	return it.getTags()
+	// FIXME(iddan): only convert when collation is JSON/JSON-LD, leave as Ref otherwise
+	d := ld.NewRDFDataset()
+	err := it.addResultsToDataset(d)
+	if err != nil {
+		it.err = err
+		return nil
+	}
+	r, err := singleDocumentFromRDF(d)
+	if err != nil {
+		it.err = err
+		return nil
+	}
+	if !it.ExcludeID {
+		m := r.(map[string]interface{})
+		delete(m, "@id")
+		return m
+	}
+	return r
 }
 
 // Err implements query.Iterator.
 func (it *TagsIterator) Err() error {
+	if it.err != nil {
+		return it.err
+	}
 	return it.ValueIt.Err()
 }
 
diff --git a/query/linkedql/jsonld_util.go b/query/linkedql/jsonld_util.go
new file mode 100644
index 0000000..dd5f2de
--- /dev/null
+++ b/query/linkedql/jsonld_util.go
@@ -0,0 +1,36 @@
+package linkedql
+
+import (
+	"fmt"
+
+	"github.com/piprate/json-gold/ld"
+)
+
+// datasetToCompact transforms a RDF dataset to a compact JSON-LD document.
+func datasetToCompact(dataset *ld.RDFDataset, context interface{}, opts *ld.JsonLdOptions) (interface{}, error) {
+	api := ld.NewJsonLdApi()
+	proc := ld.NewJsonLdProcessor()
+	d, err := api.FromRDF(dataset, opts)
+	if err != nil {
+		return nil, err
+	}
+	c, err := proc.Compact(d, context, opts)
+	if err != nil {
+		return nil, err
+	}
+	return c, nil
+}
+
+// singleDocumentFromRDF transforms a RDF dataset to a single map JSON-LD document.
+func singleDocumentFromRDF(dataset *ld.RDFDataset) (interface{}, error) {
+	api := ld.NewJsonLdApi()
+	opts := ld.NewJsonLdOptions("")
+	documents, err := api.FromRDF(dataset, opts)
+	if err != nil {
+		return nil, err
+	}
+	if len(documents) != 1 {
+		return nil, fmt.Errorf("Unexpected number of documents")
+	}
+	return documents[0], nil
+}
diff --git a/query/linkedql/steps/jsonld_util.go b/query/linkedql/steps/jsonld_util.go
index b0e5aa1..7755a8f 100644
--- a/query/linkedql/steps/jsonld_util.go
+++ b/query/linkedql/steps/jsonld_util.go
@@ -8,7 +8,7 @@ type ldArray = []interface{}
 type ldMap = map[string]interface{}
 
 func unwrapValue(i interface{}) interface{} {
-	m, ok := i.(map[string]interface{})
+	m, ok := i.(ldMap)
 	if ok && len(m) == 1 {
 		v, ok := m["@value"]
 		if ok {
@@ -18,13 +18,21 @@ func unwrapValue(i interface{}) interface{} {
 	return i
 }
 
+func unwrapSingle(i interface{}) interface{} {
+	a, ok := i.(ldArray)
+	if ok && len(a) == 1 {
+		return a[0]
+	}
+	return i
+}
+
 // isomorphic checks whether source and target JSON-LD structures are the same
 // semantically. This function is not complete and is maintained for testing
 // purposes. Hopefully in the future it can be proven sufficient for general
 // purpose use.
 func isomorphic(source interface{}, target interface{}) error {
-	source = unwrapValue(source)
-	target = unwrapValue(target)
+	source = unwrapValue(unwrapSingle(source))
+	target = unwrapValue(unwrapSingle(target))
 	switch s := source.(type) {
 	case string:
 		t, ok := target.(string)
@@ -38,7 +46,7 @@ func isomorphic(source interface{}, target interface{}) error {
 	case ldArray:
 		t, ok := target.(ldArray)
 		if !ok {
-			return fmt.Errorf("Expected %#v to be an array but instead received %T", target, target)
+			return fmt.Errorf("Expected multiple values but instead received the single value: %#v", target)
 		}
 		if len(s) != len(t) {
 			return fmt.Errorf("Expected %#v and %#v to have the same length", s, t)
diff --git a/query/linkedql/steps/steps_final.go b/query/linkedql/steps/steps_final.go
index 166600a..4378d4e 100644
--- a/query/linkedql/steps/steps_final.go
+++ b/query/linkedql/steps/steps_final.go
@@ -18,8 +18,9 @@ var _ linkedql.IteratorStep = (*Select)(nil)
 
 // Select corresponds to .select().
 type Select struct {
-	Tags []string          `json:"tags"`
-	From linkedql.PathStep `json:"from"`
+	Properties []string          `json:"properties"`
+	From       linkedql.PathStep `json:"from"`
+	ExcludeID  bool              `json:"excludeID"`
 }
 
 // Description implements Step.
@@ -33,15 +34,17 @@ func (s *Select) BuildIterator(qs graph.QuadStore, ns *voc.Namespaces) (query.It
 	if err != nil {
 		return nil, err
 	}
-	return &linkedql.TagsIterator{ValueIt: valueIt, Selected: s.Tags}, nil
+	it := linkedql.NewTagsIterator(valueIt, s.Properties, s.ExcludeID)
+	return &it, nil
 }
 
 var _ linkedql.IteratorStep = (*SelectFirst)(nil)
 
 // SelectFirst corresponds to .selectFirst().
 type SelectFirst struct {
-	Tags []string          `json:"tags"`
-	From linkedql.PathStep `json:"from"`
+	Properties []string          `json:"properties"`
+	ExcludeID  bool              `json:"excludeID"`
+	From       linkedql.PathStep `json:"from"`
 }
 
 // Description implements Step.
@@ -63,7 +66,8 @@ func (s *SelectFirst) BuildIterator(qs graph.QuadStore, ns *voc.Namespaces) (que
 	if err != nil {
 		return nil, err
 	}
-	return &linkedql.TagsIterator{it, s.Tags}, nil
+	tagsIt := linkedql.NewTagsIterator(it, s.Properties, s.ExcludeID)
+	return &tagsIt, nil
 }
 
 var _ linkedql.IteratorStep = (*Value)(nil)


Test patch:
diff --git a/query/linkedql/steps/jsonld_util_test.go b/query/linkedql/steps/jsonld_util_test.go
index 98e0a49..e187fe5 100644
--- a/query/linkedql/steps/jsonld_util_test.go
+++ b/query/linkedql/steps/jsonld_util_test.go
@@ -59,7 +59,7 @@ var testCases = []struct {
 		name:     "Single non matching property with multiple values non matching length",
 		source:   map[string]interface{}{"http://example.com/name": []interface{}{"Alice", "Bob"}},
 		target:   map[string]interface{}{"http://example.com/name": []interface{}{"Alice"}},
-		expected: fmt.Errorf("Expected []interface {}{\"Alice\", \"Bob\"} and []interface {}{\"Alice\"} to have the same length"),
+		expected: fmt.Errorf("Expected multiple values but instead received the single value: \"Alice\""),
 	},
 	{
 		name: "Single matching nested",
@@ -113,6 +113,18 @@ var testCases = []struct {
 		target:   map[string]interface{}{"http://example.com/name": map[string]interface{}{"@value": "Alice"}},
 		expected: nil,
 	},
+	{
+		name:     "Single matching properties with @value string array string",
+		source:   map[string]interface{}{"http://example.com/name": []interface{}{map[string]interface{}{"@value": "Alice"}}},
+		target:   map[string]interface{}{"http://example.com/name": "Alice"},
+		expected: nil,
+	},
+	{
+		name:     "Single matching properties with string and @value string array",
+		source:   map[string]interface{}{"http://example.com/name": "Alice"},
+		target:   map[string]interface{}{"http://example.com/name": []interface{}{map[string]interface{}{"@value": "Alice"}}},
+		expected: nil,
+	},
 }
 
 func TestIsomorphic(t *testing.T) {
diff --git a/query/linkedql/steps/test-cases/Documents.json b/query/linkedql/steps/test-cases/Documents.json
index f4ad1c3..61f0902 100644
--- a/query/linkedql/steps/test-cases/Documents.json
+++ b/query/linkedql/steps/test-cases/Documents.json
@@ -33,24 +33,26 @@
       "linkedql:names": ["http://example.com/name", "http://example.com/likes"]
     }
   },
-  "results": [
-    {
-      "@id": "http://example.com/alice",
-      "http://example.com/likes": [
-        {
-          "@id": "http://example.com/bob"
-        }
-      ],
-      "http://example.com/name": ["Alice"]
-    },
-    {
-      "@id": "http://example.com/bob",
-      "http://example.com/likes": [
-        {
-          "@id": "http://example.com/alice"
-        }
-      ],
-      "http://example.com/name": ["Bob"]
-    }
-  ]
+  "results": {
+    "@graph": [
+      {
+        "@id": "http://example.com/alice",
+        "http://example.com/likes": [
+          {
+            "@id": "http://example.com/bob"
+          }
+        ],
+        "http://example.com/name": ["Alice"]
+      },
+      {
+        "@id": "http://example.com/bob",
+        "http://example.com/likes": [
+          {
+            "@id": "http://example.com/alice"
+          }
+        ],
+        "http://example.com/name": ["Bob"]
+      }
+    ]
+  }
 }
diff --git a/query/linkedql/steps/test-cases/documents.json b/query/linkedql/steps/test-cases/documents.json
index f4ad1c3..61f0902 100644
--- a/query/linkedql/steps/test-cases/documents.json
+++ b/query/linkedql/steps/test-cases/documents.json
@@ -33,24 +33,26 @@
       "linkedql:names": ["http://example.com/name", "http://example.com/likes"]
     }
   },
-  "results": [
-    {
-      "@id": "http://example.com/alice",
-      "http://example.com/likes": [
-        {
-          "@id": "http://example.com/bob"
-        }
-      ],
-      "http://example.com/name": ["Alice"]
-    },
-    {
-      "@id": "http://example.com/bob",
-      "http://example.com/likes": [
-        {
-          "@id": "http://example.com/alice"
-        }
-      ],
-      "http://example.com/name": ["Bob"]
-    }
-  ]
+  "results": {
+    "@graph": [
+      {
+        "@id": "http://example.com/alice",
+        "http://example.com/likes": [
+          {
+            "@id": "http://example.com/bob"
+          }
+        ],
+        "http://example.com/name": ["Alice"]
+      },
+      {
+        "@id": "http://example.com/bob",
+        "http://example.com/likes": [
+          {
+            "@id": "http://example.com/alice"
+          }
+        ],
+        "http://example.com/name": ["Bob"]
+      }
+    ]
+  }
 }