diff --git a/appengine/appengine.go b/appengine/appengine.go index dcd363c..1f8706e 100644 --- a/appengine/appengine.go +++ b/appengine/appengine.go @@ -17,6 +17,7 @@ package main import ( + "fmt" "github.com/barakmich/glog" "os" "time" @@ -45,11 +46,11 @@ var ( timeout = 30 * time.Second ) -func configFrom(file string) *config.Config { +func configFrom(file string) (*config.Config, error) { // Find the file... if file != "" { if _, err := os.Stat(file); os.IsNotExist(err) { - glog.Fatalln("Cannot find specified configuration file", file, ", aborting.") + return nil, fmt.Errorf("Cannot find specified configuration file", file) } } else if _, err := os.Stat("/cayley_appengine.cfg"); err == nil { file = "/cayley_appengine.cfg" @@ -59,7 +60,7 @@ func configFrom(file string) *config.Config { } cfg, err := config.Load(file) if err != nil { - glog.Fatalln(err) + return nil, err } if cfg.DatabasePath == "" { @@ -92,15 +93,19 @@ func configFrom(file string) *config.Config { cfg.ReadOnly = cfg.ReadOnly || readOnly - return cfg + return cfg, nil } func init() { glog.SetToStderr(true) - cfg := configFrom("cayley_appengine.cfg") + cfg, err := configFrom("cayley_appengine.cfg") + if err != nil { + glog.Fatalln("Error loading config:", err) + } + handle, err := db.Open(cfg) if err != nil { - glog.Fatal(err) + glog.Fatalln("Error opening database:", err) } http.SetupRoutes(handle, cfg) } diff --git a/graph/bolt/all_iterator.go b/graph/bolt/all_iterator.go index 4cd017e..d8fae27 100644 --- a/graph/bolt/all_iterator.go +++ b/graph/bolt/all_iterator.go @@ -33,6 +33,7 @@ type AllIterator struct { dir quad.Direction qs *QuadStore result *Token + err error buffer [][]byte offset int done bool @@ -121,6 +122,7 @@ func (it *AllIterator) Next() bool { }) if err != nil { glog.Error("Error nexting in database: ", err) + it.err = err it.done = true return false } @@ -134,6 +136,10 @@ func (it *AllIterator) Next() bool { return true } +func (it *AllIterator) Err() error { + return it.err +} + func (it *AllIterator) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } @@ -168,10 +174,11 @@ func (it *AllIterator) Contains(v graph.Value) bool { return true } -func (it *AllIterator) Close() { +func (it *AllIterator) Close() error { it.result = nil it.buffer = nil it.done = true + return nil } func (it *AllIterator) Size() (int64, bool) { @@ -204,3 +211,5 @@ func (it *AllIterator) Stats() graph.IteratorStats { Size: s, } } + +var _ graph.Nexter = &AllIterator{} diff --git a/graph/bolt/iterator.go b/graph/bolt/iterator.go index efb11d6..d080391 100644 --- a/graph/bolt/iterator.go +++ b/graph/bolt/iterator.go @@ -50,6 +50,7 @@ type Iterator struct { offset int done bool size int64 + err error } func NewIterator(bucket []byte, d quad.Direction, value graph.Value, qs *QuadStore) *Iterator { @@ -105,10 +106,11 @@ func (it *Iterator) Clone() graph.Iterator { return out } -func (it *Iterator) Close() { +func (it *Iterator) Close() error { it.result = nil it.buffer = nil it.done = true + return nil } func (it *Iterator) isLiveValue(val []byte) bool { @@ -170,6 +172,7 @@ func (it *Iterator) Next() bool { if err != nil { if err != errNotExist { glog.Errorf("Error nexting in database: %v", err) + it.err = err } it.done = true return false @@ -184,6 +187,10 @@ func (it *Iterator) Next() bool { return true } +func (it *Iterator) Err() error { + return it.err +} + func (it *Iterator) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } @@ -316,3 +323,5 @@ func (it *Iterator) Stats() graph.IteratorStats { Size: s, } } + +var _ graph.Nexter = &Iterator{} diff --git a/graph/bolt/quadstore.go b/graph/bolt/quadstore.go index 5b0e2e2..ecc9a81 100644 --- a/graph/bolt/quadstore.go +++ b/graph/bolt/quadstore.go @@ -92,7 +92,10 @@ func newQuadStore(path string, options graph.Options) (graph.QuadStore, error) { } qs.db = db // BoolKey returns false on non-existence. IE, Sync by default. - qs.db.NoSync, _ = options.BoolKey("nosync") + qs.db.NoSync, _, err = options.BoolKey("nosync") + if err != nil { + return nil, err + } err = qs.getMetadata() if err != nil { return nil, err diff --git a/graph/gaedatastore/iterator.go b/graph/gaedatastore/iterator.go index ffdc8fe..8f3ef18 100644 --- a/graph/gaedatastore/iterator.go +++ b/graph/gaedatastore/iterator.go @@ -41,6 +41,7 @@ type Iterator struct { offset int last string result graph.Value + err error } var ( @@ -129,12 +130,13 @@ func (it *Iterator) Reset() { it.result = nil } -func (it *Iterator) Close() { +func (it *Iterator) Close() error { it.buffer = nil it.offset = 0 it.done = true it.last = "" it.result = nil + return nil } func (it *Iterator) Tagger() *graph.Tagger { @@ -267,7 +269,8 @@ func (it *Iterator) Next() bool { } if err != nil { glog.Errorf("Error fetching next entry %v", err) - break + it.err = err + return false } if !skip { it.buffer = append(it.buffer, k.StringID()) @@ -288,6 +291,10 @@ func (it *Iterator) Next() bool { return true } +func (it *Iterator) Err() error { + return it.err +} + func (it *Iterator) Size() (int64, bool) { return it.size, true } @@ -323,3 +330,5 @@ func (it *Iterator) Stats() graph.IteratorStats { Size: size, } } + +var _ graph.Nexter = &Iterator{} diff --git a/graph/gaedatastore/quadstore.go b/graph/gaedatastore/quadstore.go index 10dfb39..8a7b12d 100644 --- a/graph/gaedatastore/quadstore.go +++ b/graph/gaedatastore/quadstore.go @@ -154,7 +154,7 @@ func getContext(opts graph.Options) (appengine.Context, error) { req := opts["HTTPRequest"].(*http.Request) if req == nil { err := errors.New("HTTP Request needed") - glog.Fatalln(err) + glog.Errorln(err) return nil, err } return appengine.NewContext(req), nil diff --git a/graph/iterator.go b/graph/iterator.go index 01a0358..281ea64 100644 --- a/graph/iterator.go +++ b/graph/iterator.go @@ -100,6 +100,9 @@ type Iterator interface { // Contains returns whether the value is within the set held by the iterator. Contains(Value) bool + // Err returns any error that was encountered by the Iterator. + Err() error + // Start iteration from the beginning Reset() @@ -136,7 +139,7 @@ type Iterator interface { Describe() Description // Close the iterator and do internal cleanup. - Close() + Close() error // UID returns the unique identifier of the iterator. UID() uint64 @@ -155,7 +158,9 @@ type Description struct { type Nexter interface { // Next advances the iterator to the next value, which will then be available through - // the Result method. It returns false if no further advancement is possible. + // the Result method. It returns false if no further advancement is possible, or if an + // error was encountered during iteration. Err should be consulted to distinguish + // between the two cases. Next() bool Iterator diff --git a/graph/iterator/all_iterator.go b/graph/iterator/all_iterator.go index d937ce4..26e6302 100644 --- a/graph/iterator/all_iterator.go +++ b/graph/iterator/all_iterator.go @@ -55,7 +55,9 @@ func (it *Int64) Reset() { it.at = it.min } -func (it *Int64) Close() {} +func (it *Int64) Close() error { + return nil +} func (it *Int64) Clone() graph.Iterator { out := NewInt64(it.min, it.max) @@ -103,6 +105,10 @@ func (it *Int64) Next() bool { return graph.NextLogOut(it, val, true) } +func (it *Int64) Err() error { + return nil +} + // DEPRECATED func (it *Int64) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) @@ -160,3 +166,5 @@ func (it *Int64) Stats() graph.IteratorStats { Contains: it.runstats.Contains, } } + +var _ graph.Nexter = &Int64{} diff --git a/graph/iterator/and_iterator.go b/graph/iterator/and_iterator.go index 3764600..418a469 100644 --- a/graph/iterator/and_iterator.go +++ b/graph/iterator/and_iterator.go @@ -30,6 +30,7 @@ type And struct { checkList []graph.Iterator result graph.Value runstats graph.IteratorStats + err error } // Creates a new And iterator. @@ -153,9 +154,14 @@ func (it *And) Next() bool { return graph.NextLogOut(it, curr, true) } } + it.err = it.primaryIt.Err() return graph.NextLogOut(it, nil, false) } +func (it *And) Err() error { + return it.err +} + func (it *And) Result() graph.Value { return it.result } @@ -182,9 +188,26 @@ func (it *And) checkContainsList(val graph.Value, lastResult graph.Value) bool { for i, c := range it.checkList { ok = c.Contains(val) if !ok { + it.err = c.Err() + if it.err != nil { + return false + } + if lastResult != nil { for j := 0; j < i; j++ { + // One of the iterators has determined that this value doesn't + // match. However, the iterators that came before in the list + // may have returned "ok" to Contains(). We need to set all + // the tags back to what the previous result was -- effectively + // seeking back exactly one -- so we check all the prior iterators + // with the (already verified) result and throw away the result, + // which will be 'true' it.checkList[j].Contains(lastResult) + + it.err = it.checkList[j].Err() + if it.err != nil { + return false + } } } break @@ -241,10 +264,19 @@ func (it *And) NextPath() bool { if it.primaryIt.NextPath() { return true } + it.err = it.primaryIt.Err() + if it.err != nil { + return false + } for _, sub := range it.internalIterators { if sub.NextPath() { return true } + + it.err = sub.Err() + if it.err != nil { + return false + } } return false } @@ -254,14 +286,23 @@ func (it *And) cleanUp() {} // Close this iterator, and, by extension, close the subiterators. // Close should be idempotent, and it follows that if it's subiterators -// follow this contract, the And follows the contract. -func (it *And) Close() { +// follow this contract, the And follows the contract. It closes all +// subiterators it can, but returns the first error it encounters. +func (it *And) Close() error { it.cleanUp() - it.primaryIt.Close() + + err := it.primaryIt.Close() for _, sub := range it.internalIterators { - sub.Close() + _err := sub.Close() + if _err != nil && err == nil { + err = _err + } } + + return err } // Register this as an "and" iterator. func (it *And) Type() graph.Type { return graph.And } + +var _ graph.Nexter = &And{} diff --git a/graph/iterator/and_iterator_test.go b/graph/iterator/and_iterator_test.go index 3f96519..1a9c131 100644 --- a/graph/iterator/and_iterator_test.go +++ b/graph/iterator/and_iterator_test.go @@ -15,6 +15,7 @@ package iterator import ( + "errors" "testing" "github.com/google/cayley/graph" @@ -138,5 +139,20 @@ func TestAllIterators(t *testing.T) { if and.Next() { t.Error("Too many values") } - +} + +func TestAndIteratorErr(t *testing.T) { + wantErr := errors.New("unique") + allErr := newTestIterator(false, wantErr) + + and := NewAnd() + and.AddSubIterator(allErr) + and.AddSubIterator(NewInt64(1, 5)) + + if and.Next() != false { + t.Errorf("And iterator did not pass through initial 'false'") + } + if and.Err() != wantErr { + t.Errorf("And iterator did not pass through underlying Err") + } } diff --git a/graph/iterator/fixed_iterator.go b/graph/iterator/fixed_iterator.go index 4a8afb9..fe112bc 100644 --- a/graph/iterator/fixed_iterator.go +++ b/graph/iterator/fixed_iterator.go @@ -63,7 +63,9 @@ func (it *Fixed) Reset() { it.lastIndex = 0 } -func (it *Fixed) Close() {} +func (it *Fixed) Close() error { + return nil +} func (it *Fixed) Tagger() *graph.Tagger { return &it.tags @@ -143,6 +145,10 @@ func (it *Fixed) Next() bool { return graph.NextLogOut(it, out, true) } +func (it *Fixed) Err() error { + return nil +} + // DEPRECATED func (it *Fixed) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) @@ -186,3 +192,5 @@ func (it *Fixed) Stats() graph.IteratorStats { Size: int64(len(it.values)), } } + +var _ graph.Nexter = &Fixed{} diff --git a/graph/iterator/hasa_iterator.go b/graph/iterator/hasa_iterator.go index 3788b40..a799bdd 100644 --- a/graph/iterator/hasa_iterator.go +++ b/graph/iterator/hasa_iterator.go @@ -52,6 +52,7 @@ type HasA struct { resultIt graph.Iterator result graph.Value runstats graph.IteratorStats + err error } // Construct a new HasA iterator, given the quad subiterator, and the quad @@ -152,7 +153,11 @@ func (it *HasA) Contains(val graph.Value) bool { it.resultIt.Close() } it.resultIt = it.qs.QuadIterator(it.dir, val) - return graph.ContainsLogOut(it, val, it.NextContains()) + ok := it.NextContains() + if it.err != nil { + return false + } + return graph.ContainsLogOut(it, val, ok) } // NextContains() is shared code between Contains() and GetNextResult() -- calls next on the @@ -170,6 +175,7 @@ func (it *HasA) NextContains() bool { return true } } + it.err = it.resultIt.Err() return false } @@ -185,7 +191,15 @@ func (it *HasA) NextPath() bool { if it.primaryIt.NextPath() { return true } - result := it.NextContains() + it.err = it.primaryIt.Err() + if it.err != nil { + return false + } + + result := it.NextContains() // Sets it.err if there's an error + if it.err != nil { + return false + } glog.V(4).Infoln("HASA", it.UID(), "NextPath Returns", result, "") return result } @@ -202,6 +216,7 @@ func (it *HasA) Next() bool { it.resultIt = &Null{} if !graph.Next(it.primaryIt) { + it.err = it.primaryIt.Err() return graph.NextLogOut(it, 0, false) } tID := it.primaryIt.Result() @@ -210,6 +225,10 @@ func (it *HasA) Next() bool { return graph.NextLogOut(it, val, true) } +func (it *HasA) Err() error { + return it.err +} + func (it *HasA) Result() graph.Value { return it.result } @@ -238,12 +257,19 @@ func (it *HasA) Stats() graph.IteratorStats { } } -// Close the subiterator, the result iterator (if any) and the HasA. -func (it *HasA) Close() { +// Close the subiterator, the result iterator (if any) and the HasA. It closes +// all subiterators it can, but returns the first error it encounters. +func (it *HasA) Close() error { + err := it.primaryIt.Close() + if it.resultIt != nil { - it.resultIt.Close() + _err := it.resultIt.Close() + if err == nil { + err = _err + } } - it.primaryIt.Close() + + return err } // Register this iterator as a HasA. @@ -252,3 +278,5 @@ func (it *HasA) Type() graph.Type { return graph.HasA } func (it *HasA) Size() (int64, bool) { return it.Stats().Size, false } + +var _ graph.Nexter = &HasA{} diff --git a/graph/iterator/hasa_iterator_test.go b/graph/iterator/hasa_iterator_test.go new file mode 100644 index 0000000..d7498cf --- /dev/null +++ b/graph/iterator/hasa_iterator_test.go @@ -0,0 +1,37 @@ +// Copyright 2014 The Cayley Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package iterator + +import ( + "errors" + "testing" + + "github.com/google/cayley/quad" +) + +func TestHasAIteratorErr(t *testing.T) { + wantErr := errors.New("unique") + errIt := newTestIterator(false, wantErr) + + // TODO(andrew-d): pass a non-nil quadstore + hasa := NewHasA(nil, errIt, quad.Subject) + + if hasa.Next() != false { + t.Errorf("HasA iterator did not pass through initial 'false'") + } + if hasa.Err() != wantErr { + t.Errorf("HasA iterator did not pass through underlying Err") + } +} diff --git a/graph/iterator/iterator.go b/graph/iterator/iterator.go index 954ddba..371025c 100644 --- a/graph/iterator/iterator.go +++ b/graph/iterator/iterator.go @@ -88,6 +88,10 @@ func (it *Null) Next() bool { return false } +func (it *Null) Err() error { + return nil +} + func (it *Null) Result() graph.Value { return nil } @@ -110,9 +114,13 @@ func (it *Null) Size() (int64, bool) { func (it *Null) Reset() {} -func (it *Null) Close() {} +func (it *Null) Close() error { + return nil +} // A null iterator costs nothing. Use it! func (it *Null) Stats() graph.IteratorStats { return graph.IteratorStats{} } + +var _ graph.Nexter = &Null{} diff --git a/graph/iterator/iterator_test.go b/graph/iterator/iterator_test.go new file mode 100644 index 0000000..d20635c --- /dev/null +++ b/graph/iterator/iterator_test.go @@ -0,0 +1,29 @@ +package iterator + +import ( + "github.com/google/cayley/graph" +) + +// A testing iterator that returns the given values for Next() and Err(). +type testIterator struct { + *Fixed + + NextVal bool + ErrVal error +} + +func newTestIterator(next bool, err error) graph.Iterator { + return &testIterator{ + Fixed: NewFixed(Identity), + NextVal: next, + ErrVal: err, + } +} + +func (it *testIterator) Next() bool { + return it.NextVal +} + +func (it *testIterator) Err() error { + return it.ErrVal +} diff --git a/graph/iterator/linksto_iterator.go b/graph/iterator/linksto_iterator.go index 99a8df5..930e929 100644 --- a/graph/iterator/linksto_iterator.go +++ b/graph/iterator/linksto_iterator.go @@ -46,6 +46,7 @@ type LinksTo struct { nextIt graph.Iterator result graph.Value runstats graph.IteratorStats + err error } // Construct a new LinksTo iterator around a direction and a subiterator of @@ -125,6 +126,7 @@ func (it *LinksTo) Contains(val graph.Value) bool { it.result = val return graph.ContainsLogOut(it, val, true) } + it.err = it.primaryIt.Err() return graph.ContainsLogOut(it, val, false) } @@ -164,8 +166,17 @@ func (it *LinksTo) Next() bool { return graph.NextLogOut(it, it.nextIt, true) } + // If there's an error in the 'next' iterator, we save it and we're done. + it.err = it.nextIt.Err() + if it.err != nil { + return false + } + // Subiterator is empty, get another one if !graph.Next(it.primaryIt) { + // Possibly save error + it.err = it.primaryIt.Err() + // We're out of nodes in our subiterator, so we're done as well. return graph.NextLogOut(it, 0, false) } @@ -176,19 +187,34 @@ func (it *LinksTo) Next() bool { return it.Next() } +func (it *LinksTo) Err() error { + return it.err +} + func (it *LinksTo) Result() graph.Value { return it.result } -// Close our subiterators. -func (it *LinksTo) Close() { - it.nextIt.Close() - it.primaryIt.Close() +// Close closes the iterator. It closes all subiterators it can, but +// returns the first error it encounters. +func (it *LinksTo) Close() error { + err := it.nextIt.Close() + + _err := it.primaryIt.Close() + if _err != nil && err == nil { + err = _err + } + + return err } // We won't ever have a new result, but our subiterators might. func (it *LinksTo) NextPath() bool { - return it.primaryIt.NextPath() + ok := it.primaryIt.NextPath() + if !ok { + it.err = it.primaryIt.Err() + } + return ok } // Register the LinksTo. @@ -214,3 +240,5 @@ func (it *LinksTo) Stats() graph.IteratorStats { func (it *LinksTo) Size() (int64, bool) { return it.Stats().Size, false } + +var _ graph.Nexter = &LinksTo{} diff --git a/graph/iterator/materialize_iterator.go b/graph/iterator/materialize_iterator.go index 00b2102..f6421d8 100644 --- a/graph/iterator/materialize_iterator.go +++ b/graph/iterator/materialize_iterator.go @@ -49,6 +49,7 @@ type Materialize struct { hasRun bool aborted bool runstats graph.IteratorStats + err error } func NewMaterialize(sub graph.Iterator) *Materialize { @@ -69,11 +70,11 @@ func (it *Materialize) Reset() { it.index = -1 } -func (it *Materialize) Close() { - it.subIt.Close() +func (it *Materialize) Close() error { it.containsMap = nil it.values = nil it.hasRun = false + return it.subIt.Close() } func (it *Materialize) Tagger() *graph.Tagger { @@ -108,6 +109,7 @@ func (it *Materialize) Clone() graph.Iterator { if it.hasRun { out.hasRun = true out.aborted = it.aborted + out.err = it.err out.values = it.values out.containsMap = it.containsMap out.actualSize = it.actualSize @@ -199,8 +201,13 @@ func (it *Materialize) Next() bool { if !it.hasRun { it.materializeSet() } + if it.err != nil { + return false + } if it.aborted { - return graph.Next(it.subIt) + n := graph.Next(it.subIt) + it.err = it.subIt.Err() + return n } it.index++ @@ -211,12 +218,19 @@ func (it *Materialize) Next() bool { return graph.NextLogOut(it, it.Result(), true) } +func (it *Materialize) Err() error { + return it.err +} + func (it *Materialize) Contains(v graph.Value) bool { graph.ContainsLogIn(it, v) it.runstats.Contains += 1 if !it.hasRun { it.materializeSet() } + if it.err != nil { + return false + } if it.aborted { return it.subIt.Contains(v) } @@ -236,6 +250,9 @@ func (it *Materialize) NextPath() bool { if !it.hasRun { it.materializeSet() } + if it.err != nil { + return false + } if it.aborted { return it.subIt.NextPath() } @@ -283,7 +300,8 @@ func (it *Materialize) materializeSet() { it.actualSize += 1 } } - if it.aborted { + it.err = it.subIt.Err() + if it.err == nil && it.aborted { if glog.V(2) { glog.V(2).Infoln("Aborting subiterator") } @@ -293,3 +311,5 @@ func (it *Materialize) materializeSet() { } it.hasRun = true } + +var _ graph.Nexter = &Materialize{} diff --git a/graph/iterator/materialize_iterator_test.go b/graph/iterator/materialize_iterator_test.go new file mode 100644 index 0000000..8188584 --- /dev/null +++ b/graph/iterator/materialize_iterator_test.go @@ -0,0 +1,70 @@ +// Copyright 2014 The Cayley Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package iterator + +import ( + "errors" + "testing" +) + +func TestMaterializeIteratorError(t *testing.T) { + wantErr := errors.New("unique") + errIt := newTestIterator(false, wantErr) + + // This tests that we properly return 0 results and the error when the + // underlying iterator returns an error. + mIt := NewMaterialize(errIt) + + if mIt.Next() != false { + t.Errorf("Materialize iterator did not pass through underlying 'false'") + } + if mIt.Err() != wantErr { + t.Errorf("Materialize iterator did not pass through underlying Err") + } +} + +func TestMaterializeIteratorErrorAbort(t *testing.T) { + wantErr := errors.New("unique") + errIt := newTestIterator(false, wantErr) + + // This tests that we properly return 0 results and the error when the + // underlying iterator is larger than our 'abort at' value, and then + // returns an error. + or := NewOr() + or.AddSubIterator(NewInt64(1, int64(abortMaterializeAt+1))) + or.AddSubIterator(errIt) + + mIt := NewMaterialize(or) + + // We should get all the underlying values... + for i := 0; i < abortMaterializeAt+1; i++ { + if !mIt.Next() { + t.Errorf("Materialize iterator returned spurious 'false' on iteration %d", i) + return + } + if mIt.Err() != nil { + t.Errorf("Materialize iterator returned non-nil Err on iteration %d", i) + return + } + } + + // ... and then the error value. + if mIt.Next() != false { + t.Errorf("Materialize iterator did not pass through underlying 'false'") + } + if mIt.Err() != wantErr { + t.Errorf("Materialize iterator did not pass through underlying Err") + } +} diff --git a/graph/iterator/not_iterator.go b/graph/iterator/not_iterator.go index db394f8..98232d6 100644 --- a/graph/iterator/not_iterator.go +++ b/graph/iterator/not_iterator.go @@ -13,6 +13,7 @@ type Not struct { allIt graph.Iterator result graph.Value runstats graph.IteratorStats + err error } func NewNot(primaryIt, allIt graph.Iterator) *Not { @@ -87,9 +88,14 @@ func (it *Not) Next() bool { return graph.NextLogOut(it, curr, true) } } + it.err = it.allIt.Err() return graph.NextLogOut(it, nil, false) } +func (it *Not) Err() error { + return it.err +} + func (it *Not) Result() graph.Value { return it.result } @@ -105,6 +111,12 @@ func (it *Not) Contains(val graph.Value) bool { return graph.ContainsLogOut(it, val, false) } + it.err = it.primaryIt.Err() + if it.err != nil { + // Explicitly return 'false', since an error occurred. + return false + } + it.result = val return graph.ContainsLogOut(it, val, true) } @@ -115,9 +127,17 @@ func (it *Not) NextPath() bool { return false } -func (it *Not) Close() { - it.primaryIt.Close() - it.allIt.Close() +// Close closes the primary and all iterators. It closes all subiterators +// it can, but returns the first error it encounters. +func (it *Not) Close() error { + err := it.primaryIt.Close() + + _err := it.allIt.Close() + if _err != nil && err == nil { + err = _err + } + + return err } func (it *Not) Type() graph.Type { return graph.Not } @@ -161,3 +181,5 @@ func (it *Not) Describe() graph.Description { Iterators: subIts, } } + +var _ graph.Nexter = &Not{} diff --git a/graph/iterator/not_iterator_test.go b/graph/iterator/not_iterator_test.go index ac63b7c..d701513 100644 --- a/graph/iterator/not_iterator_test.go +++ b/graph/iterator/not_iterator_test.go @@ -1,6 +1,7 @@ package iterator import ( + "errors" "reflect" "testing" ) @@ -42,3 +43,19 @@ func TestNotIteratorBasics(t *testing.T) { } } } + +func TestNotIteratorErr(t *testing.T) { + wantErr := errors.New("unique") + allIt := newTestIterator(false, wantErr) + + toComplementIt := NewFixed(Identity) + + not := NewNot(toComplementIt, allIt) + + if not.Next() != false { + t.Errorf("Not iterator did not pass through initial 'false'") + } + if not.Err() != wantErr { + t.Errorf("Not iterator did not pass through underlying Err") + } +} diff --git a/graph/iterator/optional_iterator.go b/graph/iterator/optional_iterator.go index bc65acc..dbf654c 100644 --- a/graph/iterator/optional_iterator.go +++ b/graph/iterator/optional_iterator.go @@ -38,6 +38,7 @@ type Optional struct { subIt graph.Iterator lastCheck bool result graph.Value + err error } // Creates a new optional iterator. @@ -57,8 +58,8 @@ func (it *Optional) Reset() { it.lastCheck = false } -func (it *Optional) Close() { - it.subIt.Close() +func (it *Optional) Close() error { + return it.subIt.Close() } func (it *Optional) Tagger() *graph.Tagger { @@ -76,6 +77,10 @@ func (it *Optional) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } +func (it *Optional) Err() error { + return it.err +} + func (it *Optional) Result() graph.Value { return it.result } @@ -85,7 +90,11 @@ func (it *Optional) Result() graph.Value { // optional subbranch. func (it *Optional) NextPath() bool { if it.lastCheck { - return it.subIt.NextPath() + ok := it.subIt.NextPath() + if !ok { + it.err = it.subIt.Err() + } + return ok } return false } @@ -101,6 +110,7 @@ func (it *Optional) SubIterators() []graph.Iterator { func (it *Optional) Contains(val graph.Value) bool { checked := it.subIt.Contains(val) it.lastCheck = checked + it.err = it.subIt.Err() it.result = val return true } @@ -152,3 +162,5 @@ func (it *Optional) Stats() graph.IteratorStats { func (it *Optional) Size() (int64, bool) { return 0, true } + +var _ graph.Iterator = &Optional{} diff --git a/graph/iterator/or_iterator.go b/graph/iterator/or_iterator.go index 6c58238..25524e6 100644 --- a/graph/iterator/or_iterator.go +++ b/graph/iterator/or_iterator.go @@ -33,6 +33,7 @@ type Or struct { itCount int currentIterator int result graph.Value + err error } func NewOr() *Or { @@ -147,6 +148,11 @@ func (it *Or) Next() bool { return graph.NextLogOut(it, it.result, true) } + it.err = curIt.Err() + if it.err != nil { + return graph.NextLogOut(it, nil, false) + } + if it.isShortCircuiting && !first { break } @@ -159,12 +165,16 @@ func (it *Or) Next() bool { return graph.NextLogOut(it, nil, false) } +func (it *Or) Err() error { + return it.err +} + func (it *Or) Result() graph.Value { return it.result } // Checks a value against the iterators, in order. -func (it *Or) subItsContain(val graph.Value) bool { +func (it *Or) subItsContain(val graph.Value) (bool, error) { var subIsGood = false for i, sub := range it.internalIterators { subIsGood = sub.Contains(val) @@ -172,15 +182,23 @@ func (it *Or) subItsContain(val graph.Value) bool { it.currentIterator = i break } + + err := sub.Err() + if err != nil { + return false, err + } } - return subIsGood + return subIsGood, nil } // Check a value against the entire graph.iterator, in order. func (it *Or) Contains(val graph.Value) bool { graph.ContainsLogIn(it, val) - anyGood := it.subItsContain(val) - if !anyGood { + anyGood, err := it.subItsContain(val) + if err != nil { + it.err = err + return false + } else if !anyGood { return graph.ContainsLogOut(it, val, false) } it.result = val @@ -221,7 +239,12 @@ func (it *Or) Size() (int64, bool) { // shortcircuiting, only allow new results from the currently checked graph.iterator func (it *Or) NextPath() bool { if it.currentIterator != -1 { - return it.internalIterators[it.currentIterator].NextPath() + currIt := it.internalIterators[it.currentIterator] + ok := currIt.NextPath() + if !ok { + it.err = currIt.Err() + } + return ok } return false } @@ -231,12 +254,20 @@ func (it *Or) cleanUp() {} // Close this graph.iterator, and, by extension, close the subiterators. // Close should be idempotent, and it follows that if it's subiterators -// follow this contract, the And follows the contract. -func (it *Or) Close() { +// follow this contract, the Or follows the contract. It closes all +// subiterators it can, but returns the first error it encounters. +func (it *Or) Close() error { it.cleanUp() + + var err error for _, sub := range it.internalIterators { - sub.Close() + _err := sub.Close() + if _err != nil && err == nil { + err = _err + } } + + return err } func (it *Or) Optimize() (graph.Iterator, bool) { @@ -289,3 +320,5 @@ func (it *Or) Stats() graph.IteratorStats { // Register this as an "or" graph.iterator. func (it *Or) Type() graph.Type { return graph.Or } + +var _ graph.Nexter = &Or{} diff --git a/graph/iterator/or_iterator_test.go b/graph/iterator/or_iterator_test.go index 27cdc7d..7c4f2ef 100644 --- a/graph/iterator/or_iterator_test.go +++ b/graph/iterator/or_iterator_test.go @@ -15,6 +15,7 @@ package iterator import ( + "errors" "reflect" "testing" @@ -148,3 +149,46 @@ func TestShortCircuitingOrBasics(t *testing.T) { t.Errorf("Failed to iterate optimized Or correctly, got:%v expect:%v", got, expect) } } + +func TestOrIteratorErr(t *testing.T) { + wantErr := errors.New("unique") + orErr := newTestIterator(false, wantErr) + + fix1 := NewFixed(Identity) + fix1.Add(1) + + or := NewOr() + or.AddSubIterator(fix1) + or.AddSubIterator(orErr) + or.AddSubIterator(NewInt64(1, 5)) + + if !or.Next() { + t.Errorf("Failed to iterate Or correctly") + } + if got := or.Result(); got != 1 { + t.Errorf("Failed to iterate Or correctly, got:%v expect:1", got) + } + + if or.Next() != false { + t.Errorf("Or iterator did not pass through underlying 'false'") + } + if or.Err() != wantErr { + t.Errorf("Or iterator did not pass through underlying Err") + } +} + +func TestShortCircuitOrIteratorErr(t *testing.T) { + wantErr := errors.New("unique") + orErr := newTestIterator(false, wantErr) + + or := NewOr() + or.AddSubIterator(orErr) + or.AddSubIterator(NewInt64(1, 5)) + + if or.Next() != false { + t.Errorf("Or iterator did not pass through underlying 'false'") + } + if or.Err() != wantErr { + t.Errorf("Or iterator did not pass through underlying Err") + } +} diff --git a/graph/iterator/value_comparison_iterator.go b/graph/iterator/value_comparison_iterator.go index 627b853..0351d60 100644 --- a/graph/iterator/value_comparison_iterator.go +++ b/graph/iterator/value_comparison_iterator.go @@ -27,7 +27,6 @@ package iterator // In MQL terms, this is the [{"age>=": 21}] concept. import ( - "log" "strconv" "github.com/google/cayley/graph" @@ -51,6 +50,7 @@ type Comparison struct { val interface{} qs graph.QuadStore result graph.Value + err error } func NewComparison(sub graph.Iterator, op Operator, val interface{}, qs graph.QuadStore) *Comparison { @@ -91,8 +91,8 @@ func (it *Comparison) doComparison(val graph.Value) bool { } } -func (it *Comparison) Close() { - it.subIt.Close() +func (it *Comparison) Close() error { + return it.subIt.Close() } func RunIntOp(a int64, op Operator, b int64) bool { @@ -106,8 +106,7 @@ func RunIntOp(a int64, op Operator, b int64) bool { case compareGTE: return a >= b default: - log.Fatal("Unknown operator type") - return false + panic("Unknown operator type") } } @@ -133,9 +132,14 @@ func (it *Comparison) Next() bool { return true } } + it.err = it.subIt.Err() return false } +func (it *Comparison) Err() error { + return it.err +} + // DEPRECATED func (it *Comparison) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) @@ -149,6 +153,7 @@ func (it *Comparison) NextPath() bool { for { hasNext := it.subIt.NextPath() if !hasNext { + it.err = it.subIt.Err() return false } if it.doComparison(it.subIt.Result()) { @@ -168,7 +173,11 @@ func (it *Comparison) Contains(val graph.Value) bool { if !it.doComparison(val) { return false } - return it.subIt.Contains(val) + ok := it.subIt.Contains(val) + if !ok { + it.err = it.subIt.Err() + } + return ok } // If we failed the check, then the subiterator should not contribute to the result @@ -218,3 +227,5 @@ func (it *Comparison) Stats() graph.IteratorStats { func (it *Comparison) Size() (int64, bool) { return 0, true } + +var _ graph.Nexter = &Comparison{} diff --git a/graph/iterator/value_comparison_iterator_test.go b/graph/iterator/value_comparison_iterator_test.go index b8a6cc8..ccb8c54 100644 --- a/graph/iterator/value_comparison_iterator_test.go +++ b/graph/iterator/value_comparison_iterator_test.go @@ -15,6 +15,7 @@ package iterator import ( + "errors" "reflect" "testing" @@ -118,3 +119,17 @@ func TestVCIContains(t *testing.T) { } } } + +func TestComparisonIteratorErr(t *testing.T) { + wantErr := errors.New("unique") + errIt := newTestIterator(false, wantErr) + + vc := NewComparison(errIt, compareLT, int64(2), simpleStore) + + if vc.Next() != false { + t.Errorf("Comparison iterator did not pass through initial 'false'") + } + if vc.Err() != wantErr { + t.Errorf("Comparison iterator did not pass through underlying Err") + } +} diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index 515133f..e0d1bdc 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -119,6 +119,10 @@ func (it *AllIterator) Next() bool { return true } +func (it *AllIterator) Err() error { + return it.iter.Error() +} + func (it *AllIterator) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } @@ -141,11 +145,12 @@ func (it *AllIterator) Contains(v graph.Value) bool { return true } -func (it *AllIterator) Close() { +func (it *AllIterator) Close() error { if it.open { it.iter.Release() it.open = false } + return nil } func (it *AllIterator) Size() (int64, bool) { @@ -183,3 +188,5 @@ func (it *AllIterator) Stats() graph.IteratorStats { Size: s, } } + +var _ graph.Nexter = &AllIterator{} diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index 6ccd3fe..83321ad 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -109,11 +109,12 @@ func (it *Iterator) Clone() graph.Iterator { return out } -func (it *Iterator) Close() { +func (it *Iterator) Close() error { if it.open { it.iter.Release() it.open = false } + return nil } func (it *Iterator) isLiveValue(val []byte) bool { @@ -154,6 +155,10 @@ func (it *Iterator) Next() bool { return false } +func (it *Iterator) Err() error { + return it.iter.Error() +} + func (it *Iterator) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } @@ -283,3 +288,5 @@ func (it *Iterator) Stats() graph.IteratorStats { Size: s, } } + +var _ graph.Nexter = &Iterator{} diff --git a/graph/leveldb/quadstore.go b/graph/leveldb/quadstore.go index 799688d..73cf386 100644 --- a/graph/leveldb/quadstore.go +++ b/graph/leveldb/quadstore.go @@ -90,7 +90,10 @@ func newQuadStore(path string, options graph.Options) (graph.QuadStore, error) { var err error qs.path = path cacheSize := DefaultCacheSize - if val, ok := options.IntKey("cache_size_mb"); ok { + val, ok, err := options.IntKey("cache_size_mb") + if err != nil { + return nil, err + } else if ok { cacheSize = val } qs.dbOpts = &opt.Options{ @@ -99,7 +102,10 @@ func newQuadStore(path string, options graph.Options) (graph.QuadStore, error) { qs.dbOpts.ErrorIfMissing = true writeBufferSize := DefaultWriteBufferSize - if val, ok := options.IntKey("writeBufferSize"); ok { + val, ok, err = options.IntKey("writeBufferSize") + if err != nil { + return nil, err + } else if ok { writeBufferSize = val } qs.dbOpts.WriteBuffer = writeBufferSize * opt.MiB diff --git a/graph/memstore/all_iterator.go b/graph/memstore/all_iterator.go index 2d78ead..abfc105 100644 --- a/graph/memstore/all_iterator.go +++ b/graph/memstore/all_iterator.go @@ -52,6 +52,10 @@ func (it *nodesAllIterator) Next() bool { return true } +func (it *nodesAllIterator) Err() error { + return nil +} + func newQuadsAllIterator(qs *QuadStore) *quadsAllIterator { var out quadsAllIterator out.Int64 = *iterator.NewInt64(1, qs.nextQuadID-1) @@ -69,3 +73,6 @@ func (it *quadsAllIterator) Next() bool { } return out } + +var _ graph.Nexter = &nodesAllIterator{} +var _ graph.Nexter = &quadsAllIterator{} diff --git a/graph/memstore/iterator.go b/graph/memstore/iterator.go index d79e6c8..b39b4bf 100644 --- a/graph/memstore/iterator.go +++ b/graph/memstore/iterator.go @@ -15,6 +15,7 @@ package memstore import ( + "io" "math" "github.com/google/cayley/graph" @@ -30,6 +31,7 @@ type Iterator struct { iter *b.Enumerator data string result graph.Value + err error } func cmp(a, b int64) int { @@ -104,7 +106,9 @@ func (it *Iterator) Clone() graph.Iterator { return m } -func (it *Iterator) Close() {} +func (it *Iterator) Close() error { + return nil +} func (it *Iterator) checkValid(index int64) bool { return it.qs.log[index].DeletedBy == 0 @@ -118,6 +122,9 @@ func (it *Iterator) Next() bool { } result, _, err := it.iter.Next() if err != nil { + if err != io.EOF { + it.err = err + } return graph.NextLogOut(it, nil, false) } if !it.checkValid(result) { @@ -127,6 +134,10 @@ func (it *Iterator) Next() bool { return graph.NextLogOut(it, it.result, true) } +func (it *Iterator) Err() error { + return it.err +} + func (it *Iterator) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } @@ -191,3 +202,5 @@ func (it *Iterator) Stats() graph.IteratorStats { Size: int64(it.tree.Len()), } } + +var _ graph.Nexter = &Iterator{} diff --git a/graph/mongo/iterator.go b/graph/mongo/iterator.go index 876dff8..6409901 100644 --- a/graph/mongo/iterator.go +++ b/graph/mongo/iterator.go @@ -39,6 +39,7 @@ type Iterator struct { constraint bson.M collection string result graph.Value + err error } func NewIterator(qs *QuadStore, collection string, d quad.Direction, val graph.Value) *Iterator { @@ -98,8 +99,8 @@ func (it *Iterator) Reset() { } -func (it *Iterator) Close() { - it.iter.Close() +func (it *Iterator) Close() error { + return it.iter.Close() } func (it *Iterator) Tagger() *graph.Tagger { @@ -137,6 +138,7 @@ func (it *Iterator) Next() bool { if !found { err := it.iter.Err() if err != nil { + it.err = err glog.Errorln("Error Nexting Iterator: ", err) } return false @@ -148,6 +150,10 @@ func (it *Iterator) Next() bool { return true } +func (it *Iterator) Err() error { + return it.err +} + func (it *Iterator) ResultTree() *graph.ResultTree { return graph.NewResultTree(it.Result()) } @@ -230,3 +236,5 @@ func (it *Iterator) Stats() graph.IteratorStats { Size: size, } } + +var _ graph.Nexter = &Iterator{} diff --git a/graph/mongo/quadstore.go b/graph/mongo/quadstore.go index ce05efe..2a74be9 100644 --- a/graph/mongo/quadstore.go +++ b/graph/mongo/quadstore.go @@ -57,7 +57,10 @@ func createNewMongoGraph(addr string, options graph.Options) error { } conn.SetSafe(&mgo.Safe{}) dbName := DefaultDBName - if val, ok := options.StringKey("database_name"); ok { + val, ok, err := options.StringKey("database_name") + if err != nil { + return err + } else if ok { dbName = val } db := conn.DB(dbName) @@ -94,7 +97,10 @@ func newQuadStore(addr string, options graph.Options) (graph.QuadStore, error) { } conn.SetSafe(&mgo.Safe{}) dbName := DefaultDBName - if val, ok := options.StringKey("database_name"); ok { + val, ok, err := options.StringKey("database_name") + if err != nil { + return nil, err + } else if ok { dbName = val } qs.db = conn.DB(dbName) diff --git a/graph/primarykey.go b/graph/primarykey.go index 1df1c51..5332a8f 100644 --- a/graph/primarykey.go +++ b/graph/primarykey.go @@ -84,8 +84,9 @@ func (p *PrimaryKey) Int() int64 { case sequential: return p.sequentialID case unique: - glog.Fatal("UUID cannot be cast to an int64") - return -1 + msg := "UUID cannot be converted to an int64" + glog.Errorln(msg) + panic(msg) } return -1 } diff --git a/graph/quadstore.go b/graph/quadstore.go index e9a1a4f..8a89885 100644 --- a/graph/quadstore.go +++ b/graph/quadstore.go @@ -23,8 +23,8 @@ package graph import ( "errors" + "fmt" - "github.com/barakmich/glog" "github.com/google/cayley/quad" ) @@ -103,40 +103,40 @@ type QuadStore interface { type Options map[string]interface{} -func (d Options) IntKey(key string) (int, bool) { +func (d Options) IntKey(key string) (int, bool, error) { if val, ok := d[key]; ok { switch vv := val.(type) { case float64: - return int(vv), true + return int(vv), true, nil default: - glog.Fatalln("Invalid", key, "parameter type from config.") + return 0, false, fmt.Errorf("Invalid %s parameter type from config: %T", key, val) } } - return 0, false + return 0, false, nil } -func (d Options) StringKey(key string) (string, bool) { +func (d Options) StringKey(key string) (string, bool, error) { if val, ok := d[key]; ok { switch vv := val.(type) { case string: - return vv, true + return vv, true, nil default: - glog.Fatalln("Invalid", key, "parameter type from config.") + return "", false, fmt.Errorf("Invalid %s parameter type from config: %T", key, val) } } - return "", false + return "", false, nil } -func (d Options) BoolKey(key string) (bool, bool) { +func (d Options) BoolKey(key string) (bool, bool, error) { if val, ok := d[key]; ok { switch vv := val.(type) { case bool: - return vv, true + return vv, true, nil default: - glog.Fatalln("Invalid", key, "parameter type from config.") + return false, false, fmt.Errorf("Invalid %s parameter type from config: %T", key, val) } } - return false, false + return false, false, nil } var ErrCannotBulkLoad = errors.New("quadstore: cannot bulk load") diff --git a/query/mql/build_iterator.go b/query/mql/build_iterator.go index d1469d7..8d81717 100644 --- a/query/mql/build_iterator.go +++ b/query/mql/build_iterator.go @@ -17,7 +17,6 @@ package mql import ( "errors" "fmt" - "log" "math" "strings" @@ -93,7 +92,7 @@ func (q *Query) buildIteratorTreeInternal(query interface{}, path Path) (it grap it = q.buildResultIterator(path) optional = true default: - log.Fatal("Unknown JSON type?", query) + err = fmt.Errorf("Unknown JSON type: %T") } if err != nil { return nil, false, err diff --git a/writer/single.go b/writer/single.go index 53d46fa..c62788b 100644 --- a/writer/single.go +++ b/writer/single.go @@ -32,18 +32,28 @@ type Single struct { } func NewSingleReplication(qs graph.QuadStore, opts graph.Options) (graph.QuadWriter, error) { - var ignoreMissing, ignoreDuplicate bool + var ( + ignoreMissing bool + ignoreDuplicate bool + err error + ) if *graph.IgnoreMissing { ignoreMissing = true } else { - ignoreMissing, _ = opts.BoolKey("ignore_missing") + ignoreMissing, _, err = opts.BoolKey("ignore_missing") + if err != nil { + return nil, err + } } if *graph.IgnoreDup { ignoreDuplicate = true } else { - ignoreDuplicate, _ = opts.BoolKey("ignore_duplicate") + ignoreDuplicate, _, err = opts.BoolKey("ignore_duplicate") + if err != nil { + return nil, err + } } return &Single{