storage: Remove unused indexing interface

At one point the in-memory store implemented an indexing strategy so
that variable bindings could be returned for non-ground references to
base documents. However, we eventually disabled in-memory indexing
and we have not re-added it since
3ebbeede6c. At this point, indexing can
be removed completely.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>

internal/storage/mock/mock.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/storage"
 	"github.com/open-policy-agent/opa/storage/inmem"
 )
@@ -171,11 +170,6 @@ func (s *Store) DeletePolicy(ctx context.Context, txn storage.Transaction, name
 	return s.inmem.DeletePolicy(ctx, getRealTxn(txn), name)
 }
 
-// Build just shims the call to the underlying inmem store
-func (s *Store) Build(ctx context.Context, txn storage.Transaction, ref ast.Ref) (storage.Index, error) {
-	return s.inmem.Build(ctx, getRealTxn(txn), ref)
-}
-
 // NewTransaction will create a new transaction on the underlying inmem store
 // but wraps it with a mock Transaction. These are then tracked on the store.
 func (s *Store) NewTransaction(ctx context.Context, params ...storage.TransactionParams) (storage.Transaction, error) {

server/server_test.go

@@ -3586,7 +3586,6 @@ func TestServerClearsCompilerConflictCheck(t *testing.T) {
 type queryBindingErrStore struct {
 	storage.WritesNotSupported
 	storage.PolicyNotSupported
-	storage.IndexingNotSupported
 }
 
 func (s *queryBindingErrStore) Read(ctx context.Context, txn storage.Transaction, path storage.Path) (interface{}, error) {

storage/inmem/index.go

@@ -1,350 +0,0 @@
-// Copyright 2016 The OPA Authors.  All rights reserved.
-// Use of this source code is governed by an Apache2
-// license that can be found in the LICENSE file.
-
-package inmem
-
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"hash/fnv"
-	"strings"
-	"sync"
-
-	"github.com/open-policy-agent/opa/ast"
-	"github.com/open-policy-agent/opa/storage"
-	"github.com/open-policy-agent/opa/util"
-)
-
-// indices contains a mapping of non-ground references to values to sets of bindings.
-//
-//  +------+------------------------------------+
-//  | ref1 | val1 | bindings-1, bindings-2, ... |
-//  |      +------+-----------------------------+
-//  |      | val2 | bindings-m, bindings-m, ... |
-//  |      +------+-----------------------------+
-//  |      | .... | ...                         |
-//  +------+------+-----------------------------+
-//  | ref2 | .... | ...                         |
-//  +------+------+-----------------------------+
-//  | ...                                       |
-//  +-------------------------------------------+
-//
-// The "value" is the data value stored at the location referred to by the ground
-// reference obtained by plugging bindings into the non-ground reference that is the
-// index key.
-//
-type indices struct {
-	mu    sync.Mutex
-	table map[int]*indicesNode
-}
-
-type indicesNode struct {
-	key  ast.Ref
-	val  *bindingIndex
-	next *indicesNode
-}
-
-func newIndices() *indices {
-	return &indices{
-		table: map[int]*indicesNode{},
-	}
-}
-
-func (ind *indices) Build(ctx context.Context, store storage.Store, txn storage.Transaction, ref ast.Ref) (*bindingIndex, error) {
-
-	ind.mu.Lock()
-	defer ind.mu.Unlock()
-
-	if exist := ind.get(ref); exist != nil {
-		return exist, nil
-	}
-
-	index := newBindingIndex()
-
-	if err := iterStorage(ctx, store, txn, ref, ast.EmptyRef(), ast.NewValueMap(), index.Add); err != nil {
-		return nil, err
-	}
-
-	hashCode := ref.Hash()
-	head := ind.table[hashCode]
-	entry := &indicesNode{
-		key:  ref,
-		val:  index,
-		next: head,
-	}
-
-	ind.table[hashCode] = entry
-
-	return index, nil
-}
-
-func (ind *indices) get(ref ast.Ref) *bindingIndex {
-	node := ind.getNode(ref)
-	if node != nil {
-		return node.val
-	}
-	return nil
-}
-
-func (ind *indices) iter(iter func(ast.Ref, *bindingIndex) error) error {
-	for _, head := range ind.table {
-		for entry := head; entry != nil; entry = entry.next {
-			if err := iter(entry.key, entry.val); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-func (ind *indices) getNode(ref ast.Ref) *indicesNode {
-	hashCode := ref.Hash()
-	for entry := ind.table[hashCode]; entry != nil; entry = entry.next {
-		if entry.key.Equal(ref) {
-			return entry
-		}
-	}
-	return nil
-}
-
-func (ind *indices) String() string {
-	buf := []string{}
-	for _, head := range ind.table {
-		for entry := head; entry != nil; entry = entry.next {
-			str := fmt.Sprintf("%v: %v", entry.key, entry.val)
-			buf = append(buf, str)
-		}
-	}
-	return "{" + strings.Join(buf, ", ") + "}"
-}
-
-// bindingIndex contains a mapping of values to bindings.
-type bindingIndex struct {
-	table map[int]*indexNode
-}
-
-type indexNode struct {
-	key  interface{}
-	val  *bindingSet
-	next *indexNode
-}
-
-func newBindingIndex() *bindingIndex {
-	return &bindingIndex{
-		table: map[int]*indexNode{},
-	}
-}
-
-func (ind *bindingIndex) Add(val interface{}, bindings *ast.ValueMap) {
-
-	node := ind.getNode(val)
-	if node != nil {
-		node.val.Add(bindings)
-		return
-	}
-
-	hashCode := hash(val)
-	bindingsSet := newBindingSet()
-	bindingsSet.Add(bindings)
-
-	entry := &indexNode{
-		key:  val,
-		val:  bindingsSet,
-		next: ind.table[hashCode],
-	}
-
-	ind.table[hashCode] = entry
-}
-
-func (ind *bindingIndex) Lookup(_ context.Context, _ storage.Transaction, val interface{}, iter storage.IndexIterator) error {
-	node := ind.getNode(val)
-	if node == nil {
-		return nil
-	}
-	return node.val.Iter(iter)
-}
-
-func (ind *bindingIndex) getNode(val interface{}) *indexNode {
-	hashCode := hash(val)
-	head := ind.table[hashCode]
-	for entry := head; entry != nil; entry = entry.next {
-		if util.Compare(entry.key, val) == 0 {
-			return entry
-		}
-	}
-	return nil
-}
-
-func (ind *bindingIndex) String() string {
-
-	buf := []string{}
-
-	for _, head := range ind.table {
-		for entry := head; entry != nil; entry = entry.next {
-			str := fmt.Sprintf("%v: %v", entry.key, entry.val)
-			buf = append(buf, str)
-		}
-	}
-
-	return "{" + strings.Join(buf, ", ") + "}"
-}
-
-type bindingSetNode struct {
-	val  *ast.ValueMap
-	next *bindingSetNode
-}
-
-type bindingSet struct {
-	table map[int]*bindingSetNode
-}
-
-func newBindingSet() *bindingSet {
-	return &bindingSet{
-		table: map[int]*bindingSetNode{},
-	}
-}
-
-func (set *bindingSet) Add(val *ast.ValueMap) {
-	node := set.getNode(val)
-	if node != nil {
-		return
-	}
-	hashCode := val.Hash()
-	head := set.table[hashCode]
-	set.table[hashCode] = &bindingSetNode{val, head}
-}
-
-func (set *bindingSet) Iter(iter func(*ast.ValueMap) error) error {
-	for _, head := range set.table {
-		for entry := head; entry != nil; entry = entry.next {
-			if err := iter(entry.val); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-func (set *bindingSet) String() string {
-	buf := []string{}
-	set.Iter(func(bindings *ast.ValueMap) error {
-		buf = append(buf, bindings.String())
-		return nil
-	})
-	return "{" + strings.Join(buf, ", ") + "}"
-}
-
-func (set *bindingSet) getNode(val *ast.ValueMap) *bindingSetNode {
-	hashCode := val.Hash()
-	for entry := set.table[hashCode]; entry != nil; entry = entry.next {
-		if entry.val.Equal(val) {
-			return entry
-		}
-	}
-	return nil
-}
-
-func hash(v interface{}) int {
-	switch v := v.(type) {
-	case []interface{}:
-		var h int
-		for _, e := range v {
-			h += hash(e)
-		}
-		return h
-	case map[string]interface{}:
-		var h int
-		for k, v := range v {
-			h += hash(k) + hash(v)
-		}
-		return h
-	case string:
-		h := fnv.New64a()
-		h.Write([]byte(v))
-		return int(h.Sum64())
-	case bool:
-		if v {
-			return 1
-		}
-		return 0
-	case nil:
-		return 0
-	case json.Number:
-		h := fnv.New64a()
-		h.Write([]byte(v))
-		return int(h.Sum64())
-	}
-	panic(fmt.Sprintf("illegal argument: %v (%T)", v, v))
-}
-
-func iterStorage(ctx context.Context, store storage.Store, txn storage.Transaction, nonGround, ground ast.Ref, bindings *ast.ValueMap, iter func(interface{}, *ast.ValueMap)) error {
-
-	if len(nonGround) == 0 {
-		path, err := storage.NewPathForRef(ground)
-		if err != nil {
-			return err
-		}
-		node, err := store.Read(ctx, txn, path)
-		if err != nil {
-			if storage.IsNotFound(err) {
-				return nil
-			}
-			return err
-		}
-		iter(node, bindings)
-		return nil
-	}
-
-	head := nonGround[0]
-	tail := nonGround[1:]
-
-	headVar, isVar := head.Value.(ast.Var)
-
-	if !isVar || len(ground) == 0 {
-		ground = append(ground, head)
-		return iterStorage(ctx, store, txn, tail, ground, bindings, iter)
-	}
-
-	path, err := storage.NewPathForRef(ground)
-	if err != nil {
-		return err
-	}
-
-	node, err := store.Read(ctx, txn, path)
-	if err != nil {
-		if storage.IsNotFound(err) {
-			return nil
-		}
-		return err
-	}
-
-	switch node := node.(type) {
-	case map[string]interface{}:
-		for key := range node {
-			ground = append(ground, ast.StringTerm(key))
-			cpy := bindings.Copy()
-			cpy.Put(headVar, ast.String(key))
-			err := iterStorage(ctx, store, txn, tail, ground, cpy, iter)
-			if err != nil {
-				return err
-			}
-			ground = ground[:len(ground)-1]
-		}
-	case []interface{}:
-		for i := range node {
-			idx := ast.IntNumberTerm(i)
-			ground = append(ground, idx)
-			cpy := bindings.Copy()
-			cpy.Put(headVar, idx.Value)
-			err := iterStorage(ctx, store, txn, tail, ground, cpy, iter)
-			if err != nil {
-				return err
-			}
-			ground = ground[:len(ground)-1]
-		}
-	}
-
-	return nil
-}

storage/inmem/index_test.go

@@ -1,152 +0,0 @@
-// Copyright 2016 The OPA Authors.  All rights reserved.
-// Use of this source code is governed by an Apache2
-// license that can be found in the LICENSE file.
-package inmem
-
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"testing"
-
-	"github.com/open-policy-agent/opa/ast"
-	"github.com/open-policy-agent/opa/storage"
-	"github.com/open-policy-agent/opa/util"
-)
-
-func TestIndicesBuild(t *testing.T) {
-
-	tests := []struct {
-		note     string
-		ref      string
-		value    interface{}
-		expected string
-	}{
-		{"single var", "data.a[i]", json.Number("2"), `[{"i": 1}]`},
-		{"two var", "data.d[x][y]", "baz", `[{"x": "e", "y": 1}]`},
-		{"partial ground", `data.c[i]["y"][j]`, nil, `[{"i": 0, "j": 0}]`},
-		{"multiple bindings", "data.g[x][y]", json.Number("0"), `[
-			{"x": "a", "y": 1},
-			{"x": "a", "y": 2},
-			{"x": "a", "y": 3},
-			{"x": "b", "y": 0},
-			{"x": "b", "y": 2},
-			{"x": "b", "y": 3},
-			{"x": "c", "y": 0},
-			{"x": "c", "y": 1},
-			{"x": "c", "y": 2}
-		]`},
-	}
-
-	for i, tc := range tests {
-		runIndexBuildTestCase(t, i+1, tc.note, tc.ref, tc.expected, tc.value)
-	}
-
-}
-
-func TestIndicesAdd(t *testing.T) {
-
-	data := loadSmallTestData()
-	ctx := context.Background()
-	store := NewFromObject(data)
-	txn := storage.NewTransactionOrDie(ctx, store)
-
-	indices := newIndices()
-	ref := ast.MustParseRef("data.d[x][y]")
-
-	index, err := indices.Build(ctx, store, txn, ref)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// new value to add
-	var val1 interface{}
-	err = util.UnmarshalJSON([]byte(`{"x":[1,true]}`), &val1)
-	if err != nil {
-		panic(err)
-	}
-	bindings1 := loadExpectedBindings(`[{"x": "e", "y": 2}]`)[0]
-	index.Add(val1, bindings1)
-	assertBindingsEqual(t, "new value", index, val1, `[{"x": "e", "y": 2}]`)
-
-	// existing value
-	val2 := "baz"
-	bindings2 := loadExpectedBindings(`[{"x": "e", "y": 3}]`)[0]
-	index.Add(val2, bindings2)
-	assertBindingsEqual(t, "existing value", index, val2, `[{"x": "e", "y": 1}, {"x": "e", "y": 3}]`)
-	index.Add(val2, bindings2)
-	assertBindingsEqual(t, "same value (no change)", index, val2, `[{"x": "e", "y": 1}, {"x": "e", "y": 3}]`)
-}
-
-func runIndexBuildTestCase(t *testing.T, i int, note string, refStr string, expectedStr string, value interface{}) {
-
-	ctx := context.Background()
-	data := loadSmallTestData()
-	store := NewFromObject(data)
-	txn := storage.NewTransactionOrDie(ctx, store)
-	indices := newIndices()
-
-	ref := ast.MustParseRef(refStr)
-
-	if indices.get(ref) != nil {
-		t.Errorf("Test case %d (%v): Did not expect indices to contain %v yet", i, note, ref)
-		return
-	}
-
-	index, err := indices.Build(ctx, store, txn, ref)
-	if err != nil {
-		t.Errorf("Test case %d (%v): Did not expect error from build: %v", i, note, err)
-		return
-	}
-
-	assertBindingsEqual(t, fmt.Sprintf("Test case %d (%v)", i, note), index, value, expectedStr)
-}
-
-func assertBindingsEqual(t *testing.T, note string, index *bindingIndex, value interface{}, expectedStr string) {
-
-	expected := loadExpectedBindings(expectedStr)
-
-	err := index.Lookup(context.Background(), nil, value, func(bindings *ast.ValueMap) error {
-		for j := range expected {
-			if expected[j].Equal(bindings) {
-				tmp := expected[:j]
-				expected = append(tmp, expected[j+1:]...)
-				return nil
-			}
-		}
-		return fmt.Errorf("unexpected bindings: %v", bindings)
-	})
-
-	if err != nil {
-		t.Errorf("%v: Did not expect error from index iteration: %v", note, err)
-		return
-	}
-
-	if len(expected) > 0 {
-		t.Errorf("%v: Missing expected bindings: %v", note, expected)
-		return
-	}
-}
-
-func loadExpectedBindings(input string) []*ast.ValueMap {
-	var data []map[string]interface{}
-	if err := util.UnmarshalJSON([]byte(input), &data); err != nil {
-		panic(err)
-	}
-	var expected []*ast.ValueMap
-	for _, bindings := range data {
-		buf := ast.NewValueMap()
-		for k, v := range bindings {
-			switch v := v.(type) {
-			case string:
-				buf.Put(ast.Var(k), ast.String(v))
-			case json.Number:
-				buf.Put(ast.Var(k), ast.Number(v))
-			default:
-				panic("unreachable")
-			}
-		}
-		expected = append(expected, buf)
-	}
-	return expected
-}

storage/inmem/inmem.go

@@ -22,7 +22,6 @@ import (
 	"sync"
 	"sync/atomic"
 
-	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/storage"
 	"github.com/open-policy-agent/opa/util"
 )
@@ -33,7 +32,6 @@ func New() storage.Store {
 		data:     map[string]interface{}{},
 		triggers: map[*handle]storage.TriggerConfig{},
 		policies: map[string][]byte{},
-		indices:  newIndices(),
 	}
 }
 
@@ -72,7 +70,6 @@ type store struct {
 	data     map[string]interface{}            // raw data
 	policies map[string][]byte                 // raw policies
 	triggers map[*handle]storage.TriggerConfig // registered triggers
-	indices  *indices                          // data ref indices
 }
 
 type handle struct {
@@ -103,7 +100,6 @@ func (db *store) Commit(ctx context.Context, txn storage.Transaction) error {
 	if underlying.write {
 		db.rmu.Lock()
 		event := underlying.Commit()
-		db.indices = newIndices()
 		db.runOnCommitTriggers(ctx, txn, event)
 		// Mark the transaction stale after executing triggers so they can
 		// perform store operations if needed.
@@ -200,20 +196,6 @@ func (db *store) Write(ctx context.Context, txn storage.Transaction, op storage.
 	return underlying.Write(op, path, *val)
 }
 
-func (db *store) Build(ctx context.Context, txn storage.Transaction, ref ast.Ref) (storage.Index, error) {
-	underlying, err := db.underlying(txn)
-	if err != nil {
-		return nil, err
-	}
-	if underlying.write {
-		return nil, &storage.Error{
-			Code:    storage.IndexingNotSupportedErr,
-			Message: "in-memory store does not support indexing on write transactions",
-		}
-	}
-	return db.indices.Build(ctx, db, txn, ref)
-}
-
 func (h *handle) Unregister(ctx context.Context, txn storage.Transaction) {
 	underlying, err := h.db.underlying(txn)
 	if err != nil {

storage/inmem/txn.go

@@ -408,7 +408,6 @@ func deepCopy(val interface{}) interface{} {
 }
 
 func ptr(data interface{}, path storage.Path) (interface{}, error) {
-
 	node := data
 	for i := range path {
 		key := path[i]

storage/interface.go

@@ -6,8 +6,6 @@ package storage
 
 import (
 	"context"
-
-	"github.com/open-policy-agent/opa/ast"
 )
 
 // Transaction defines the interface that identifies a consistent snapshot over
@@ -20,7 +18,6 @@ type Transaction interface {
 type Store interface {
 	Trigger
 	Policy
-	Indexing
 
 	// NewTransaction is called create a new transaction in the store.
 	NewTransaction(ctx context.Context, params ...TransactionParams) (Transaction, error)
@@ -195,25 +192,3 @@ func (TriggersNotSupported) Register(context.Context, Transaction, TriggerConfig
 type TriggerHandle interface {
 	Unregister(ctx context.Context, txn Transaction)
 }
-
-// IndexIterator defines the interface for iterating over index results.
-type IndexIterator func(*ast.ValueMap) error
-
-// Indexing defines the interface for building an index.
-type Indexing interface {
-	Build(ctx context.Context, txn Transaction, ref ast.Ref) (Index, error)
-}
-
-// Index defines the interface for searching a pre-built index.
-type Index interface {
-	Lookup(ctx context.Context, txn Transaction, value interface{}, iter IndexIterator) error
-}
-
-// IndexingNotSupported provides default implementations of the Indexing
-// interface which may be used if the backend does not support indexing.
-type IndexingNotSupported struct{}
-
-// Build always returns an error indicating indexing is not supported.
-func (IndexingNotSupported) Build(context.Context, Transaction, ast.Ref) (Index, error) {
-	return nil, indexingNotSupportedError()
-}

topdown/topdown_test.go

@@ -2719,7 +2719,6 @@ type contextPropagationStore struct {
 	storage.WritesNotSupported
 	storage.TriggersNotSupported
 	storage.PolicyNotSupported
-	storage.IndexingNotSupported
 	calls []interface{}
 }