Repository: cayleygraph/cayley
Pull request #928 (2020-03-28 21:10:43)

LinkedQL: Improve PropertyPath
 - Use PropertyPath everywhere it should be
 - Make NewPropertyPath return a pointer to allow no properties as a valid PropertyPath

<!-- 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/928)
<!-- Reviewable:end -->



Patch:
diff --git a/internal/linkedql/schema/schema.go b/internal/linkedql/schema/schema.go
index b1b688c..c3b0dd0 100644
--- a/internal/linkedql/schema/schema.go
+++ b/internal/linkedql/schema/schema.go
@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"github.com/cayleygraph/cayley/query/linkedql"
+	// Steps are imported here so they be registered and documented in the schema
 	_ "github.com/cayleygraph/cayley/query/linkedql/steps"
 	"github.com/cayleygraph/quad"
 	"github.com/cayleygraph/quad/voc/owl"
@@ -26,7 +27,7 @@ var (
 	iteratorStep     = reflect.TypeOf((*linkedql.IteratorStep)(nil)).Elem()
 	entityIdentifier = reflect.TypeOf((*linkedql.EntityIdentifier)(nil)).Elem()
 	value            = reflect.TypeOf((*quad.Value)(nil)).Elem()
-	propertyPath     = reflect.TypeOf((*linkedql.PropertyPath)(nil)).Elem()
+	propertyPath     = reflect.TypeOf((*linkedql.PropertyPath)(nil))
 	stringMap        = reflect.TypeOf(map[string]string{})
 	graphPattern     = reflect.TypeOf(linkedql.GraphPattern(nil))
 )
diff --git a/query/linkedql/property_path.go b/query/linkedql/property_path.go
index 66ce486..c66ac0c 100644
--- a/query/linkedql/property_path.go
+++ b/query/linkedql/property_path.go
@@ -21,8 +21,8 @@ type PropertyPath struct {
 }
 
 // NewPropertyPath constructs a new PropertyPath
-func NewPropertyPath(p PropertyPathI) PropertyPath {
-	return PropertyPath{PropertyPathI: p}
+func NewPropertyPath(p PropertyPathI) *PropertyPath {
+	return &PropertyPath{PropertyPathI: p}
 }
 
 // Description implements Step.
@@ -95,13 +95,18 @@ func (p PropertyIRIs) BuildPath(qs graph.QuadStore, ns *voc.Namespaces) (*path.P
 // PropertyIRIStrings is a slice of property IRI strings.
 type PropertyIRIStrings []string
 
-// BuildPath implements PropertyPath.
-func (p PropertyIRIStrings) BuildPath(qs graph.QuadStore, ns *voc.Namespaces) (*path.Path, error) {
+// PropertyIRIs casts PropertyIRIStrings into PropertyIRIs
+func (p PropertyIRIStrings) PropertyIRIs() PropertyIRIs {
 	var iris PropertyIRIs
 	for _, iri := range p {
 		iris = append(iris, PropertyIRI(iri))
 	}
-	return iris.BuildPath(qs, ns)
+	return iris
+}
+
+// BuildPath implements PropertyPath.
+func (p PropertyIRIStrings) BuildPath(qs graph.QuadStore, ns *voc.Namespaces) (*path.Path, error) {
+	return p.PropertyIRIs().BuildPath(qs, ns)
 }
 
 // PropertyIRI is an IRI of a Property
diff --git a/query/linkedql/steps/both.go b/query/linkedql/steps/both.go
index 2d0d33c..eb75d7d 100644
--- a/query/linkedql/steps/both.go
+++ b/query/linkedql/steps/both.go
@@ -17,8 +17,8 @@ var _ linkedql.PathStep = (*Both)(nil)
 
 // Both corresponds to .both().
 type Both struct {
-	From       linkedql.PathStep     `json:"from"`
-	Properties linkedql.PropertyPath `json:"properties"`
+	From       linkedql.PathStep      `json:"from"`
+	Properties *linkedql.PropertyPath `json:"properties"`
 }
 
 // Description implements Step.
diff --git a/query/linkedql/steps/has.go b/query/linkedql/steps/has.go
index 683889c..459bac0 100644
--- a/query/linkedql/steps/has.go
+++ b/query/linkedql/steps/has.go
@@ -18,9 +18,9 @@ var _ linkedql.PathStep = (*Has)(nil)
 
 // Has corresponds to .has().
 type Has struct {
-	From     linkedql.PathStep     `json:"from"`
-	Property linkedql.PropertyPath `json:"property"`
-	Values   []quad.Value          `json:"values"`
+	From     linkedql.PathStep      `json:"from"`
+	Property *linkedql.PropertyPath `json:"property"`
+	Values   []quad.Value           `json:"values"`
 }
 
 // Description implements Step.
diff --git a/query/linkedql/steps/has_reverse.go b/query/linkedql/steps/has_reverse.go
index 5ae1fae..2c6287c 100644
--- a/query/linkedql/steps/has_reverse.go
+++ b/query/linkedql/steps/has_reverse.go
@@ -18,9 +18,9 @@ var _ linkedql.PathStep = (*HasReverse)(nil)
 
 // HasReverse corresponds to .hasR().
 type HasReverse struct {
-	From     linkedql.PathStep     `json:"from"`
-	Property linkedql.PropertyPath `json:"property"`
-	Values   []quad.Value          `json:"values"`
+	From     linkedql.PathStep      `json:"from"`
+	Property *linkedql.PropertyPath `json:"property"`
+	Values   []quad.Value           `json:"values"`
 }
 
 // Description implements Step.
diff --git a/query/linkedql/steps/properties.go b/query/linkedql/steps/properties.go
index beb6c09..aadc4bb 100644
--- a/query/linkedql/steps/properties.go
+++ b/query/linkedql/steps/properties.go
@@ -1,6 +1,8 @@
 package steps
 
 import (
+	"fmt"
+
 	"github.com/cayleygraph/cayley/graph"
 	"github.com/cayleygraph/cayley/query"
 	"github.com/cayleygraph/cayley/query/linkedql"
@@ -18,9 +20,8 @@ var _ linkedql.PathStep = (*Properties)(nil)
 
 // Properties corresponds to .properties().
 type Properties struct {
-	From linkedql.PathStep `json:"from"`
-	// TODO(iddan): Use linkedql.PropertyPath
-	Names []quad.IRI `json:"names"`
+	From  linkedql.PathStep      `json:"from"`
+	Names *linkedql.PropertyPath `json:"names"`
 }
 
 // Description implements Step.
@@ -34,6 +35,26 @@ func (s *Properties) BuildIterator(qs graph.QuadStore, ns *voc.Namespaces) (quer
 	return linkedql.NewValueIteratorFromPathStep(s, qs, ns)
 }
 
+func resolveNames(names *linkedql.PropertyPath) (linkedql.PropertyIRIs, error) {
+	if names == nil {
+		return nil, fmt.Errorf("Not implemented: should tag all properties")
+	}
+	switch n := names.PropertyPathI.(type) {
+	case linkedql.PropertyStep:
+		return nil, fmt.Errorf("Not implemented: should use step to resolve to properties")
+	case linkedql.PropertyIRIs:
+		return n, nil
+	case linkedql.PropertyIRIStrings:
+		return n.PropertyIRIs(), nil
+	case linkedql.PropertyIRI:
+		return linkedql.PropertyIRIs{n}, nil
+	case linkedql.PropertyIRIString:
+		return linkedql.PropertyIRIs{linkedql.PropertyIRI(n)}, nil
+	default:
+		return nil, fmt.Errorf("Unexpected type")
+	}
+}
+
 // BuildPath implements linkedql.PathStep.
 func (s *Properties) BuildPath(qs graph.QuadStore, ns *voc.Namespaces) (*path.Path, error) {
 	fromPath, err := s.From.BuildPath(qs, ns)
@@ -41,14 +62,14 @@ func (s *Properties) BuildPath(qs graph.QuadStore, ns *voc.Namespaces) (*path.Pa
 		return nil, err
 	}
 	p := fromPath
-	if s.Names != nil {
-		for _, name := range s.Names {
-			name = name.FullWith(ns)
-			tag := string(name)
-			p = p.Save(name, tag)
-		}
-	} else {
-		panic("Not implemented: should tag all properties")
+	names, err := resolveNames(s.Names)
+	if err != nil {
+		return nil, err
+	}
+	for _, n := range names {
+		name := quad.IRI(n).FullWith(ns)
+		tag := string(name)
+		p = p.Save(name, tag)
 	}
 	return p, nil
 }
diff --git a/query/linkedql/steps/reverse_properties.go b/query/linkedql/steps/reverse_properties.go
index 275b7f7..f0380ac 100644
--- a/query/linkedql/steps/reverse_properties.go
+++ b/query/linkedql/steps/reverse_properties.go
@@ -18,9 +18,8 @@ var _ linkedql.PathStep = (*ReverseProperties)(nil)
 
 // ReverseProperties corresponds to .reverseProperties().
 type ReverseProperties struct {
-	From linkedql.PathStep `json:"from"`
-	// TODO(iddan): Use property path
-	Names []quad.IRI `json:"names"`
+	From  linkedql.PathStep      `json:"from"`
+	Names *linkedql.PropertyPath `json:"names"`
 }
 
 // Description implements Step.
@@ -40,8 +39,12 @@ func (s *ReverseProperties) BuildPath(qs graph.QuadStore, ns *voc.Namespaces) (*
 		return nil, err
 	}
 	p := fromPath
-	for _, name := range s.Names {
-		name = name.FullWith(ns)
+	names, err := resolveNames(s.Names)
+	if err != nil {
+		return nil, err
+	}
+	for _, n := range names {
+		name := quad.IRI(n).FullWith(ns)
 		tag := string(name)
 		p = fromPath.SaveReverse(name, tag)
 	}
diff --git a/query/linkedql/steps/visit.go b/query/linkedql/steps/visit.go
index 559213f..d9329a7 100644
--- a/query/linkedql/steps/visit.go
+++ b/query/linkedql/steps/visit.go
@@ -17,8 +17,8 @@ var _ linkedql.PathStep = (*Visit)(nil)
 
 // Visit corresponds to .view().
 type Visit struct {
-	From       linkedql.PathStep     `json:"from"`
-	Properties linkedql.PropertyPath `json:"properties"`
+	From       linkedql.PathStep      `json:"from"`
+	Properties *linkedql.PropertyPath `json:"properties"`
 }
 
 // Description implements Step.
diff --git a/query/linkedql/steps/visit_reverse.go b/query/linkedql/steps/visit_reverse.go
index c42cf45..de57a15 100644
--- a/query/linkedql/steps/visit_reverse.go
+++ b/query/linkedql/steps/visit_reverse.go
@@ -17,8 +17,8 @@ var _ linkedql.PathStep = (*VisitReverse)(nil)
 
 // VisitReverse corresponds to .viewReverse().
 type VisitReverse struct {
-	From       linkedql.PathStep     `json:"from"`
-	Properties linkedql.PropertyPath `json:"properties"`
+	From       linkedql.PathStep      `json:"from"`
+	Properties *linkedql.PropertyPath `json:"properties"`
 }
 
 // Description implements Step.


Test patch:
diff --git a/query/linkedql/steps/steps_test.go b/query/linkedql/steps/steps_test.go
index 7757470..eeb4d48 100644
--- a/query/linkedql/steps/steps_test.go
+++ b/query/linkedql/steps/steps_test.go
@@ -376,7 +376,7 @@ var testCases = []struct {
 		query: &Select{
 			From: &Properties{
 				From:  &Vertex{Values: []quad.Value{}},
-				Names: []quad.IRI{likes},
+				Names: linkedql.NewPropertyPath(linkedql.PropertyIRI(likes)),
 			},
 		},
 		results: []interface{}{
@@ -419,7 +419,7 @@ var testCases = []struct {
 		query: &Select{
 			From: &ReverseProperties{
 				From:  &Vertex{Values: []quad.Value{}},
-				Names: []quad.IRI{likes},
+				Names: linkedql.NewPropertyPath(linkedql.PropertyIRI(likes)),
 			},
 		},
 		results: []interface{}{map[string]interface{}{
@@ -509,11 +509,11 @@ var testCases = []struct {
 			From: &Optional{
 				From: &Properties{
 					From:  &Vertex{Values: []quad.Value{}},
-					Names: []quad.IRI{name},
+					Names: linkedql.NewPropertyPath(linkedql.PropertyIRI(name)),
 				},
 				Step: &Properties{
 					From:  &Placeholder{},
-					Names: []quad.IRI{likes},
+					Names: linkedql.NewPropertyPath(linkedql.PropertyIRI(likes)),
 				},
 			},
 		},
@@ -576,7 +576,7 @@ var testCases = []struct {
 		query: &Documents{
 			From: &Properties{
 				From:  &Vertex{Values: []quad.Value{}},
-				Names: []quad.IRI{name, likes},
+				Names: linkedql.NewPropertyPath(linkedql.PropertyIRIs{linkedql.PropertyIRI(name), linkedql.PropertyIRI(likes)}),
 			},
 		},
 		results: []interface{}{