From c5f671ce185cb85512d98f9a0ea0258954dd68af Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Fri, 3 Oct 2025 15:42:36 +0530 Subject: [PATCH 1/8] added len method --- v2/arangodb/collection_documents_create.go | 2 + .../collection_documents_create_impl.go | 22 ++++ v2/arangodb/collection_documents_delete.go | 3 + .../collection_documents_delete_impl.go | 29 ++++- v2/arangodb/collection_documents_read.go | 2 + v2/arangodb/collection_documents_read_impl.go | 17 ++- v2/arangodb/collection_documents_replace.go | 2 + .../collection_documents_replace_impl.go | 35 ++++- v2/arangodb/collection_documents_update.go | 2 + .../collection_documents_update_impl.go | 35 ++++- v2/arangodb/shared/read_all.go | 97 ++++++++++++++ ...atabase_collection_doc_create_code_test.go | 57 +++++++- .../database_collection_doc_delete_test.go | 3 +- .../database_collection_operations_test.go | 122 ++++++++++++++++++ 14 files changed, 415 insertions(+), 13 deletions(-) create mode 100644 v2/arangodb/shared/read_all.go diff --git a/v2/arangodb/collection_documents_create.go b/v2/arangodb/collection_documents_create.go index 53d4c914..4e81f2c2 100644 --- a/v2/arangodb/collection_documents_create.go +++ b/v2/arangodb/collection_documents_create.go @@ -71,7 +71,9 @@ type CollectionDocumentCreate interface { } type CollectionDocumentCreateResponseReader interface { + shared.ReadAllReadable[CollectionDocumentCreateResponse] Read() (CollectionDocumentCreateResponse, error) + Len() int } type CollectionDocumentCreateResponse struct { diff --git a/v2/arangodb/collection_documents_create_impl.go b/v2/arangodb/collection_documents_create_impl.go index b8117243..a2874185 100644 --- a/v2/arangodb/collection_documents_create_impl.go +++ b/v2/arangodb/collection_documents_create_impl.go @@ -133,6 +133,7 @@ func newCollectionDocumentCreateResponseReader(array *connection.Array, options c.response.New = newUnmarshalInto(c.options.NewObject) } + c.ReadAllReader = shared.ReadAllReader[CollectionDocumentCreateResponse, *collectionDocumentCreateResponseReader]{Reader: c} return c } @@ -147,6 +148,12 @@ type collectionDocumentCreateResponseReader struct { Old *UnmarshalInto `json:"old,omitempty"` New *UnmarshalInto `json:"new,omitempty"` } + shared.ReadAllReader[CollectionDocumentCreateResponse, *collectionDocumentCreateResponseReader] + + // Cache for len() method + cachedResults []CollectionDocumentCreateResponse + cachedErrors []error + cached bool } func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreateResponse, error) { @@ -171,9 +178,24 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat return CollectionDocumentCreateResponse{}, err } + // Update meta with the unmarshaled data + meta.DocumentMeta = *c.response.DocumentMeta + meta.ResponseStruct = *c.response.ResponseStruct + meta.Old = c.response.Old + meta.New = c.response.New + if meta.Error != nil && *meta.Error { return meta, meta.AsArangoError() } return meta, nil } + +// Len returns the number of items in the response +func (c *collectionDocumentCreateResponseReader) Len() int { + if !c.cached { + c.cachedResults, c.cachedErrors = c.ReadAll() + c.cached = true + } + return len(c.cachedResults) +} diff --git a/v2/arangodb/collection_documents_delete.go b/v2/arangodb/collection_documents_delete.go index 28641243..4a4e241c 100644 --- a/v2/arangodb/collection_documents_delete.go +++ b/v2/arangodb/collection_documents_delete.go @@ -63,7 +63,9 @@ type CollectionDocumentDeleteResponse struct { } type CollectionDocumentDeleteResponseReader interface { + shared.ReadAllIntoReadable[CollectionDocumentDeleteResponse] Read(i interface{}) (CollectionDocumentDeleteResponse, error) + Len() int } type CollectionDocumentDeleteOptions struct { @@ -82,6 +84,7 @@ type CollectionDocumentDeleteOptions struct { WithWaitForSync *bool // Return additionally the complete previous revision of the changed document + // Should be a pointer to an object OldObject interface{} // If set to true, an empty object is returned as response if the document operation succeeds. diff --git a/v2/arangodb/collection_documents_delete_impl.go b/v2/arangodb/collection_documents_delete_impl.go index 9a12837e..54248833 100644 --- a/v2/arangodb/collection_documents_delete_impl.go +++ b/v2/arangodb/collection_documents_delete_impl.go @@ -24,6 +24,7 @@ import ( "context" "io" "net/http" + "reflect" "github.com/pkg/errors" @@ -42,6 +43,7 @@ var _ CollectionDocumentDelete = &collectionDocumentDelete{} type collectionDocumentDelete struct { collection *collection + shared.ReadAllIntoReader[CollectionDocumentDeleteResponse, *collectionDocumentDeleteResponseReader] } func (c collectionDocumentDelete) DeleteDocument(ctx context.Context, key string) (CollectionDocumentDeleteResponse, error) { @@ -103,6 +105,7 @@ func (c collectionDocumentDelete) DeleteDocumentsWithOptions(ctx context.Context func newCollectionDocumentDeleteResponseReader(array *connection.Array, options *CollectionDocumentDeleteOptions) *collectionDocumentDeleteResponseReader { c := &collectionDocumentDeleteResponseReader{array: array, options: options} + c.ReadAllIntoReader = shared.ReadAllIntoReader[CollectionDocumentDeleteResponse, *collectionDocumentDeleteResponseReader]{Reader: c} return c } @@ -111,6 +114,11 @@ var _ CollectionDocumentDeleteResponseReader = &collectionDocumentDeleteResponse type collectionDocumentDeleteResponseReader struct { array *connection.Array options *CollectionDocumentDeleteOptions + shared.ReadAllIntoReader[CollectionDocumentDeleteResponse, *collectionDocumentDeleteResponseReader] + // Cache for len() method + cachedResults []CollectionDocumentDeleteResponse + cachedErrors []error + cached bool } func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (CollectionDocumentDeleteResponse, error) { @@ -146,11 +154,30 @@ func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (Collection } if c.options != nil && c.options.OldObject != nil { - meta.Old = c.options.OldObject + // Create a new instance for each document to avoid reusing the same pointer + oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() + meta.Old = reflect.New(oldObjectType).Interface() + + // Extract old data into the new instance if err := response.Object.Object.Extract("old").Inject(meta.Old); err != nil { return CollectionDocumentDeleteResponse{}, err } + + // Copy data from the new instance to the original OldObject for backward compatibility + oldValue := reflect.ValueOf(meta.Old).Elem() + originalValue := reflect.ValueOf(c.options.OldObject).Elem() + originalValue.Set(oldValue) } return meta, nil } + +// Len returns the number of items in the response +func (c *collectionDocumentDeleteResponseReader) Len() int { + if !c.cached { + var dummySlice []interface{} + c.cachedResults, c.cachedErrors = c.ReadAll(&dummySlice) + c.cached = true + } + return len(c.cachedResults) +} diff --git a/v2/arangodb/collection_documents_read.go b/v2/arangodb/collection_documents_read.go index 32c49d83..f46d40f8 100644 --- a/v2/arangodb/collection_documents_read.go +++ b/v2/arangodb/collection_documents_read.go @@ -59,6 +59,8 @@ type CollectionDocumentRead interface { type CollectionDocumentReadResponseReader interface { Read(i interface{}) (CollectionDocumentReadResponse, error) + shared.ReadAllIntoReadable[CollectionDocumentReadResponse] + Len() int } type CollectionDocumentReadResponse struct { diff --git a/v2/arangodb/collection_documents_read_impl.go b/v2/arangodb/collection_documents_read_impl.go index 6a30bfb8..8bc680cc 100644 --- a/v2/arangodb/collection_documents_read_impl.go +++ b/v2/arangodb/collection_documents_read_impl.go @@ -97,7 +97,7 @@ func (c collectionDocumentRead) ReadDocumentWithOptions(ctx context.Context, key func newCollectionDocumentReadResponseReader(array *connection.Array, options *CollectionDocumentReadOptions) *collectionDocumentReadResponseReader { c := &collectionDocumentReadResponseReader{array: array, options: options} - + c.ReadAllIntoReader = shared.ReadAllIntoReader[CollectionDocumentReadResponse, *collectionDocumentReadResponseReader]{Reader: c} return c } @@ -106,6 +106,11 @@ var _ CollectionDocumentReadResponseReader = &collectionDocumentReadResponseRead type collectionDocumentReadResponseReader struct { array *connection.Array options *CollectionDocumentReadOptions + shared.ReadAllIntoReader[CollectionDocumentReadResponse, *collectionDocumentReadResponseReader] + // Cache for len() method + cachedResults []CollectionDocumentReadResponse + cachedErrors []error + cached bool } func (c *collectionDocumentReadResponseReader) Read(i interface{}) (CollectionDocumentReadResponse, error) { @@ -142,3 +147,13 @@ func (c *collectionDocumentReadResponseReader) Read(i interface{}) (CollectionDo return meta, nil } + +// Len returns the number of items in the response +func (c *collectionDocumentReadResponseReader) Len() int { + if !c.cached { + var dummySlice []interface{} + c.cachedResults, c.cachedErrors = c.ReadAll(&dummySlice) + c.cached = true + } + return len(c.cachedResults) +} diff --git a/v2/arangodb/collection_documents_replace.go b/v2/arangodb/collection_documents_replace.go index 1e62b789..ed6a7269 100644 --- a/v2/arangodb/collection_documents_replace.go +++ b/v2/arangodb/collection_documents_replace.go @@ -61,7 +61,9 @@ type CollectionDocumentReplace interface { } type CollectionDocumentReplaceResponseReader interface { + shared.ReadAllReadable[CollectionDocumentReplaceResponse] Read() (CollectionDocumentReplaceResponse, error) + Len() int } type CollectionDocumentReplaceResponse struct { diff --git a/v2/arangodb/collection_documents_replace_impl.go b/v2/arangodb/collection_documents_replace_impl.go index 9da6ac36..9c138a40 100644 --- a/v2/arangodb/collection_documents_replace_impl.go +++ b/v2/arangodb/collection_documents_replace_impl.go @@ -24,6 +24,7 @@ import ( "context" "io" "net/http" + "reflect" "github.com/pkg/errors" @@ -132,7 +133,7 @@ func newCollectionDocumentReplaceResponseReader(array *connection.Array, options c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } - + c.ReadAllReader = shared.ReadAllReader[CollectionDocumentReplaceResponse, *collectionDocumentReplaceResponseReader]{Reader: c} return c } @@ -147,6 +148,12 @@ type collectionDocumentReplaceResponseReader struct { Old *UnmarshalInto `json:"old,omitempty"` New *UnmarshalInto `json:"new,omitempty"` } + shared.ReadAllReader[CollectionDocumentReplaceResponse, *collectionDocumentReplaceResponseReader] + + // Cache for len() method + cachedResults []CollectionDocumentReplaceResponse + cachedErrors []error + cached bool } func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentReplaceResponse, error) { @@ -157,8 +164,15 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl var meta CollectionDocumentReplaceResponse if c.options != nil { - meta.Old = c.options.OldObject - meta.New = c.options.NewObject + // Create new instances for each document to avoid reusing the same pointers + if c.options.OldObject != nil { + oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() + meta.Old = reflect.New(oldObjectType).Interface() + } + if c.options.NewObject != nil { + newObjectType := reflect.TypeOf(c.options.NewObject).Elem() + meta.New = reflect.New(newObjectType).Interface() + } } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev @@ -171,9 +185,24 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl return CollectionDocumentReplaceResponse{}, err } + // Update meta with the unmarshaled data + meta.DocumentMetaWithOldRev = *c.response.DocumentMetaWithOldRev + meta.ResponseStruct = *c.response.ResponseStruct + meta.Old = c.response.Old + meta.New = c.response.New + if meta.Error != nil && *meta.Error { return meta, meta.AsArangoError() } return meta, nil } + +// Len returns the number of items in the response +func (c *collectionDocumentReplaceResponseReader) Len() int { + if !c.cached { + c.cachedResults, c.cachedErrors = c.ReadAll() + c.cached = true + } + return len(c.cachedResults) +} diff --git a/v2/arangodb/collection_documents_update.go b/v2/arangodb/collection_documents_update.go index adffcf6b..d565b420 100644 --- a/v2/arangodb/collection_documents_update.go +++ b/v2/arangodb/collection_documents_update.go @@ -62,7 +62,9 @@ type CollectionDocumentUpdate interface { } type CollectionDocumentUpdateResponseReader interface { + shared.ReadAllReadable[CollectionDocumentUpdateResponse] Read() (CollectionDocumentUpdateResponse, error) + Len() int } type CollectionDocumentUpdateResponse struct { diff --git a/v2/arangodb/collection_documents_update_impl.go b/v2/arangodb/collection_documents_update_impl.go index 068d1b39..23f57104 100644 --- a/v2/arangodb/collection_documents_update_impl.go +++ b/v2/arangodb/collection_documents_update_impl.go @@ -24,6 +24,7 @@ import ( "context" "io" "net/http" + "reflect" "github.com/pkg/errors" @@ -132,7 +133,7 @@ func newCollectionDocumentUpdateResponseReader(array *connection.Array, options c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } - + c.ReadAllReader = shared.ReadAllReader[CollectionDocumentUpdateResponse, *collectionDocumentUpdateResponseReader]{Reader: c} return c } @@ -147,6 +148,12 @@ type collectionDocumentUpdateResponseReader struct { Old *UnmarshalInto `json:"old,omitempty"` New *UnmarshalInto `json:"new,omitempty"` } + shared.ReadAllReader[CollectionDocumentUpdateResponse, *collectionDocumentUpdateResponseReader] + + // Cache for len() method + cachedResults []CollectionDocumentUpdateResponse + cachedErrors []error + cached bool } func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdateResponse, error) { @@ -157,8 +164,15 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat var meta CollectionDocumentUpdateResponse if c.options != nil { - meta.Old = c.options.OldObject - meta.New = c.options.NewObject + // Create new instances for each document to avoid reusing the same pointers + if c.options.OldObject != nil { + oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() + meta.Old = reflect.New(oldObjectType).Interface() + } + if c.options.NewObject != nil { + newObjectType := reflect.TypeOf(c.options.NewObject).Elem() + meta.New = reflect.New(newObjectType).Interface() + } } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev @@ -171,9 +185,24 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat return CollectionDocumentUpdateResponse{}, err } + // Update meta with the unmarshaled data + meta.DocumentMetaWithOldRev = *c.response.DocumentMetaWithOldRev + meta.ResponseStruct = *c.response.ResponseStruct + meta.Old = c.response.Old + meta.New = c.response.New + if meta.Error != nil && *meta.Error { return meta, meta.AsArangoError() } return meta, nil } + +// Len returns the number of items in the response +func (c *collectionDocumentUpdateResponseReader) Len() int { + if !c.cached { + c.cachedResults, c.cachedErrors = c.ReadAll() + c.cached = true + } + return len(c.cachedResults) +} diff --git a/v2/arangodb/shared/read_all.go b/v2/arangodb/shared/read_all.go new file mode 100644 index 00000000..777a37e6 --- /dev/null +++ b/v2/arangodb/shared/read_all.go @@ -0,0 +1,97 @@ +// DISCLAIMER +// +// # Copyright 2020-2025 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany + +package shared + +import ( + "errors" + "reflect" +) + +type readReader[T any] interface { + Read() (T, error) +} + +type ReadAllReadable[T any] interface { + ReadAll() ([]T, []error) +} + +type ReadAllReader[T any, R readReader[T]] struct { + Reader R +} + +func (r ReadAllReader[T, R]) ReadAll() ([]T, []error) { + var docs []T + var errs []error + for { + doc, e := r.Reader.Read() + if errors.Is(e, NoMoreDocumentsError{}) { + break + } + errs = append(errs, e) + docs = append(docs, doc) + } + return docs, errs +} + +type readReaderInto[T any] interface { + Read(i interface{}) (T, error) +} + +type ReadAllIntoReadable[T any] interface { + ReadAll(i interface{}) ([]T, []error) +} + +type ReadAllIntoReader[T any, R readReaderInto[T]] struct { + Reader R +} + +func (r ReadAllIntoReader[T, R]) ReadAll(i interface{}) ([]T, []error) { + + iVal := reflect.ValueOf(i) + if iVal.Kind() != reflect.Ptr || iVal.Elem().Kind() != reflect.Slice { + panic("i must be a pointer to a slice") + } + + eVal := iVal.Elem() + eType := eVal.Type().Elem() + + var docs []T + var errs []error + + for { + res := reflect.New(eType) + doc, e := r.Reader.Read(res.Interface()) + if errors.Is(e, NoMoreDocumentsError{}) { + break + } + + iDocVal := reflect.ValueOf(doc) + if iDocVal.Kind() == reflect.Ptr { + iDocVal = iDocVal.Elem() + } + docCopy := reflect.New(iDocVal.Type()).Elem() + docCopy.Set(iDocVal) + + errs = append(errs, e) + docs = append(docs, docCopy.Interface().(T)) + eVal = reflect.Append(eVal, res.Elem()) + } + iVal.Elem().Set(eVal) + return docs, errs +} diff --git a/v2/tests/database_collection_doc_create_code_test.go b/v2/tests/database_collection_doc_create_code_test.go index a59181ca..19d97fc1 100644 --- a/v2/tests/database_collection_doc_create_code_test.go +++ b/v2/tests/database_collection_doc_create_code_test.go @@ -24,11 +24,10 @@ import ( "context" "testing" - "github.com/arangodb/go-driver/v2/arangodb/shared" - "github.com/stretchr/testify/require" "github.com/arangodb/go-driver/v2/arangodb" + "github.com/arangodb/go-driver/v2/arangodb/shared" ) type DocWithCode struct { @@ -38,6 +37,7 @@ type DocWithCode struct { func Test_DatabaseCollectionDocCreateCode(t *testing.T) { Wrap(t, func(t *testing.T, client arangodb.Client) { + // COMMENTED OUT FOR DEBUGGING ONLY WithDatabase(t, client, nil, func(db arangodb.Database) { WithCollectionV2(t, db, nil, func(col arangodb.Collection) { withContextT(t, defaultTestTimeout, func(ctx context.Context, tb testing.TB) { @@ -85,7 +85,7 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { meta, err := docs.Read(&z) require.NoError(t, err) - require.EqualValues(t, "test", meta.Key) + require.Equal(t, "test", meta.Key) _, err = docs.Read(&z) require.Error(t, err) @@ -93,7 +93,7 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { meta, err = docs.Read(&z) require.NoError(t, err) - require.EqualValues(t, "test2", meta.Key) + require.Equal(t, "test2", meta.Key) _, err = docs.Read(&z) require.Error(t, err) @@ -101,5 +101,54 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { }) }) }) + + WithDatabase(t, client, nil, func(db arangodb.Database) { + WithCollectionV2(t, db, nil, func(col arangodb.Collection) { + withContextT(t, defaultTestTimeout, func(ctx context.Context, tb testing.TB) { + doc1 := DocWithCode{ + Key: "test", + Code: "code1", + } + doc2 := DocWithCode{ + Key: "test2", + Code: "code2", + } + readerCrt, err := col.CreateDocuments(ctx, []any{doc1, doc2}) + require.NoError(t, err) + metaCrt, errs := readerCrt.ReadAll() + require.Equal(t, 2, len(metaCrt)) // Verify we got 2 results + require.ElementsMatch(t, []any{doc1.Key, doc2.Key}, []any{metaCrt[0].Key, metaCrt[1].Key}) + require.ElementsMatch(t, []any{nil, nil}, errs) + + var docRedRead []DocWithCode + + readeRed, err := col.ReadDocuments(ctx, []string{ + "test", "test2", "nonexistent", + }) + require.NoError(t, err) + + metaRed, errs := readeRed.ReadAll(&docRedRead) + + require.ElementsMatch(t, []any{doc1.Key, doc2.Key}, []any{metaRed[0].Key, metaRed[1].Key}) + require.ElementsMatch(t, []any{nil, nil, shared.ErrArangoDocumentNotFound}, []any{errs[0], errs[1], errs[2].(shared.ArangoError).ErrorNum}) + + var docOldObject DocWithCode + var docDelRead []DocWithCode + + readerDel, err := col.DeleteDocumentsWithOptions(ctx, []string{ + "test", "test2", "nonexistent", + }, &arangodb.CollectionDocumentDeleteOptions{OldObject: &docOldObject}) + require.NoError(t, err) + metaDel, errs := readerDel.ReadAll(&docDelRead) + + require.ElementsMatch(t, []any{doc1.Key, doc2.Key, ""}, []any{metaDel[0].Key, metaDel[1].Key, metaDel[2].Key}) + require.ElementsMatch(t, []any{nil, nil, shared.ErrArangoDocumentNotFound}, []any{errs[0], errs[1], errs[2].(shared.ArangoError).ErrorNum}) + + // Now this should work correctly with separate Old objects + require.ElementsMatch(t, []any{doc1.Code, doc2.Code}, []any{metaDel[0].Old.(*DocWithCode).Code, metaDel[1].Old.(*DocWithCode).Code}) + + }) + }) + }) }) } diff --git a/v2/tests/database_collection_doc_delete_test.go b/v2/tests/database_collection_doc_delete_test.go index 2d149f80..b171fb5f 100644 --- a/v2/tests/database_collection_doc_delete_test.go +++ b/v2/tests/database_collection_doc_delete_test.go @@ -151,7 +151,8 @@ func Test_DatabaseCollectionDocDeleteSimple(t *testing.T) { } require.NoError(t, err, meta) require.Equal(t, keys[i], meta.Key) - require.Equal(t, keys[i], oldDoc.Key) + require.NotNil(t, meta.Old) + require.Equal(t, keys[i], meta.Old.(*document).Key) } }) }) diff --git a/v2/tests/database_collection_operations_test.go b/v2/tests/database_collection_operations_test.go index 0291e2f2..b32d1a55 100644 --- a/v2/tests/database_collection_operations_test.go +++ b/v2/tests/database_collection_operations_test.go @@ -270,6 +270,7 @@ func Test_DatabaseCollectionOperations(t *testing.T) { require.NoError(t, err) r, err := col.ReadDocuments(ctx, docsIds) + require.NoError(t, err) nd := docs @@ -473,6 +474,127 @@ func Test_DatabaseCollectionOperations(t *testing.T) { require.Len(t, nd, 0) }) + + t.Run("Replace", func(t *testing.T) { + // Create some documents to replace + replaceDocs := newDocs(5) + for i := 0; i < 5; i++ { + replaceDocs[i].Fields = GenerateUUID("replace-test") + } + + // Create the documents first + _, err := col.CreateDocuments(ctx, replaceDocs) + require.NoError(t, err) + + // Now replace them + for i := 0; i < 5; i++ { + replaceDocs[i].Fields = GenerateUUID("replaced-test") + } + + var oldDoc document + var newDoc document + + _, err = col.ReplaceDocumentsWithOptions(ctx, replaceDocs, &arangodb.CollectionDocumentReplaceOptions{ + OldObject: &oldDoc, + NewObject: &newDoc, + }) + require.NoError(t, err) + }) + }) + }) + }) + }) +} + +func Test_DatabaseCollectionBulkOperations(t *testing.T) { + Wrap(t, func(t *testing.T, client arangodb.Client) { + WithDatabase(t, client, nil, func(db arangodb.Database) { + WithCollectionV2(t, db, nil, func(col arangodb.Collection) { + withContextT(t, defaultTestTimeout, func(ctx context.Context, tb testing.TB) { + size := 10 + docs := newDocs(size) + + for i := 0; i < size; i++ { + docs[i].Fields = GenerateUUID("test-doc-bulk") + } + + docsIds := docs.asBasic().getKeys() + + t.Run("Create_Bulk", func(t *testing.T) { + createReader, err := col.CreateDocuments(ctx, docs) + require.NoError(t, err) + + // Test bulk create operation + createResults, createErrs := createReader.ReadAll() + require.Equal(t, size, len(createResults)) + require.Equal(t, size, len(createErrs)) + for _, err := range createErrs { + require.NoError(t, err) + } + }) + + t.Run("Read_Bulk", func(t *testing.T) { + readReader, err := col.ReadDocuments(ctx, docsIds) + require.NoError(t, err) + + // Test bulk read operation + var readResults []document + readResponses, readErrs := readReader.ReadAll(&readResults) + require.Equal(t, size, len(readResponses)) + require.Equal(t, size, len(readErrs)) + require.Equal(t, size, len(readResults)) + for _, err := range readErrs { + require.NoError(t, err) + } + }) + + t.Run("Update_Bulk", func(t *testing.T) { + // Update the documents + for i := 0; i < size; i++ { + docs[i].Fields = GenerateUUID("updated-test-doc") + } + + var oldDoc document + var newDoc document + + updateReader, err := col.UpdateDocumentsWithOptions(ctx, docs, &arangodb.CollectionDocumentUpdateOptions{ + OldObject: &oldDoc, + NewObject: &newDoc, + }) + require.NoError(t, err) + + // Test bulk update operation + updateResults, updateErrs := updateReader.ReadAll() + require.Equal(t, size, len(updateResults)) + require.Equal(t, size, len(updateErrs)) + for _, err := range updateErrs { + require.NoError(t, err) + } + }) + + t.Run("Replace_Bulk", func(t *testing.T) { + // Replace the documents + for i := 0; i < size; i++ { + docs[i].Fields = GenerateUUID("replaced-test-doc") + } + + var oldDoc document + var newDoc document + + replaceReader, err := col.ReplaceDocumentsWithOptions(ctx, docs, &arangodb.CollectionDocumentReplaceOptions{ + OldObject: &oldDoc, + NewObject: &newDoc, + }) + require.NoError(t, err) + + // Test bulk replace operation + replaceResults, replaceErrs := replaceReader.ReadAll() + require.Equal(t, size, len(replaceResults)) + require.Equal(t, size, len(replaceErrs)) + for _, err := range replaceErrs { + require.NoError(t, err) + } + }) }) }) }) From 78505d20233fb8f234969b7b5cc83cdb8d360a3f Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Tue, 28 Oct 2025 13:41:35 +0530 Subject: [PATCH 2/8] modified read benchmark test and codebase related to len method --- test/benchmarks_test.go | 29 +++++++------------ .../collection_documents_create_impl.go | 19 ++++++++++-- .../collection_documents_delete_impl.go | 21 ++++++++++++-- v2/arangodb/collection_documents_read_impl.go | 21 ++++++++++++-- .../collection_documents_replace_impl.go | 19 ++++++++++-- .../collection_documents_update_impl.go | 19 ++++++++++-- v2/tests/benchmarks_test.go | 16 +++++----- 7 files changed, 109 insertions(+), 35 deletions(-) diff --git a/test/benchmarks_test.go b/test/benchmarks_test.go index 85c16595..6045b641 100644 --- a/test/benchmarks_test.go +++ b/test/benchmarks_test.go @@ -87,7 +87,7 @@ func BenchmarkV1BulkInsert100KDocs(b *testing.B) { } func bulkRead(b *testing.B, docSize int) { - db, col := setup(b) + _, col := setup(b) // ----------------------------- // Prepare and insert documents @@ -110,31 +110,24 @@ func bulkRead(b *testing.B, docSize int) { require.NoError(b, err) // ----------------------------------------- - // Sub-benchmark 1: Read entire collection + // Sub-benchmark 1: Read entire collection using ReadDocuments // ----------------------------------------- b.Run("ReadAllDocsOnce", func(b *testing.B) { - query := fmt.Sprintf("FOR d IN %s RETURN d", col.Name()) + // Prepare keys for reading + keys := make([]string, docSize) + for j := 0; j < docSize; j++ { + keys[j] = fmt.Sprintf("doc_%d", j) + } b.ResetTimer() for i := 0; i < b.N; i++ { - cursor, err := db.Query(ctx, query, nil) + readDocs := make([]TestDoc, docSize) + _, _, err := col.ReadDocuments(ctx, keys, readDocs) require.NoError(b, err) - count := 0 - for { - var doc TestDoc - _, err := cursor.ReadDocument(ctx, &doc) - if driver.IsNoMoreDocuments(err) { - break - } - require.NoError(b, err) - count++ - } - // require.Equal(b, docSize, count, "expected to read all documents") - _ = cursor.Close() // sanity check - if count != docSize { - b.Fatalf("expected to read %d docs, got %d", docSize, count) + if len(readDocs) != docSize { + b.Fatalf("expected to read %d docs, got %d", docSize, len(readDocs)) } } }) diff --git a/v2/arangodb/collection_documents_create_impl.go b/v2/arangodb/collection_documents_create_impl.go index a2874185..a9a91610 100644 --- a/v2/arangodb/collection_documents_create_impl.go +++ b/v2/arangodb/collection_documents_create_impl.go @@ -150,13 +150,26 @@ type collectionDocumentCreateResponseReader struct { } shared.ReadAllReader[CollectionDocumentCreateResponse, *collectionDocumentCreateResponseReader] - // Cache for len() method + // Cache for len() method - allows Read() to work after Len() is called cachedResults []CollectionDocumentCreateResponse cachedErrors []error cached bool + readIndex int // Track position in cache for Read() after Len() } func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreateResponse, error) { + // If Len() was called, serve from cache + if c.cached { + if c.readIndex >= len(c.cachedResults) { + return CollectionDocumentCreateResponse{}, shared.NoMoreDocumentsError{} + } + result := c.cachedResults[c.readIndex] + err := c.cachedErrors[c.readIndex] + c.readIndex++ + return result, err + } + + // Normal streaming read if !c.array.More() { return CollectionDocumentCreateResponse{}, shared.NoMoreDocumentsError{} } @@ -191,11 +204,13 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat return meta, nil } -// Len returns the number of items in the response +// Len returns the number of items in the response. +// After calling Len(), you can still use Read() to iterate through items. func (c *collectionDocumentCreateResponseReader) Len() int { if !c.cached { c.cachedResults, c.cachedErrors = c.ReadAll() c.cached = true + c.readIndex = 0 // Reset read position to allow Read() after Len() } return len(c.cachedResults) } diff --git a/v2/arangodb/collection_documents_delete_impl.go b/v2/arangodb/collection_documents_delete_impl.go index 54248833..182aad82 100644 --- a/v2/arangodb/collection_documents_delete_impl.go +++ b/v2/arangodb/collection_documents_delete_impl.go @@ -115,13 +115,27 @@ type collectionDocumentDeleteResponseReader struct { array *connection.Array options *CollectionDocumentDeleteOptions shared.ReadAllIntoReader[CollectionDocumentDeleteResponse, *collectionDocumentDeleteResponseReader] - // Cache for len() method + // Cache for len() method - allows Read() to work after Len() is called cachedResults []CollectionDocumentDeleteResponse cachedErrors []error cached bool + readIndex int // Track position in cache for Read() after Len() } func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (CollectionDocumentDeleteResponse, error) { + // If Len() was called, serve from cache + // Note: When serving from cache, the 'i' parameter is not populated with document data + if c.cached { + if c.readIndex >= len(c.cachedResults) { + return CollectionDocumentDeleteResponse{}, shared.NoMoreDocumentsError{} + } + result := c.cachedResults[c.readIndex] + err := c.cachedErrors[c.readIndex] + c.readIndex++ + return result, err + } + + // Normal streaming read if !c.array.More() { return CollectionDocumentDeleteResponse{}, shared.NoMoreDocumentsError{} } @@ -172,12 +186,15 @@ func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (Collection return meta, nil } -// Len returns the number of items in the response +// Len returns the number of items in the response. +// After calling Len(), you can still use Read() to iterate through items. +// Note: When Read() serves from cache, the document data parameter is not populated. func (c *collectionDocumentDeleteResponseReader) Len() int { if !c.cached { var dummySlice []interface{} c.cachedResults, c.cachedErrors = c.ReadAll(&dummySlice) c.cached = true + c.readIndex = 0 // Reset read position to allow Read() after Len() } return len(c.cachedResults) } diff --git a/v2/arangodb/collection_documents_read_impl.go b/v2/arangodb/collection_documents_read_impl.go index 8bc680cc..5c99cdd7 100644 --- a/v2/arangodb/collection_documents_read_impl.go +++ b/v2/arangodb/collection_documents_read_impl.go @@ -107,13 +107,27 @@ type collectionDocumentReadResponseReader struct { array *connection.Array options *CollectionDocumentReadOptions shared.ReadAllIntoReader[CollectionDocumentReadResponse, *collectionDocumentReadResponseReader] - // Cache for len() method + // Cache for len() method - allows Read() to work after Len() is called cachedResults []CollectionDocumentReadResponse cachedErrors []error cached bool + readIndex int // Track position in cache for Read() after Len() } func (c *collectionDocumentReadResponseReader) Read(i interface{}) (CollectionDocumentReadResponse, error) { + // If Len() was called, serve from cache + // Note: When serving from cache, the 'i' parameter is not populated with document data + if c.cached { + if c.readIndex >= len(c.cachedResults) { + return CollectionDocumentReadResponse{}, shared.NoMoreDocumentsError{} + } + result := c.cachedResults[c.readIndex] + err := c.cachedErrors[c.readIndex] + c.readIndex++ + return result, err + } + + // Normal streaming read if !c.array.More() { return CollectionDocumentReadResponse{}, shared.NoMoreDocumentsError{} } @@ -148,12 +162,15 @@ func (c *collectionDocumentReadResponseReader) Read(i interface{}) (CollectionDo return meta, nil } -// Len returns the number of items in the response +// Len returns the number of items in the response. +// After calling Len(), you can still use Read() to iterate through items. +// Note: When Read() serves from cache, the document data parameter is not populated. func (c *collectionDocumentReadResponseReader) Len() int { if !c.cached { var dummySlice []interface{} c.cachedResults, c.cachedErrors = c.ReadAll(&dummySlice) c.cached = true + c.readIndex = 0 // Reset read position to allow Read() after Len() } return len(c.cachedResults) } diff --git a/v2/arangodb/collection_documents_replace_impl.go b/v2/arangodb/collection_documents_replace_impl.go index 9c138a40..194f0dd3 100644 --- a/v2/arangodb/collection_documents_replace_impl.go +++ b/v2/arangodb/collection_documents_replace_impl.go @@ -150,13 +150,26 @@ type collectionDocumentReplaceResponseReader struct { } shared.ReadAllReader[CollectionDocumentReplaceResponse, *collectionDocumentReplaceResponseReader] - // Cache for len() method + // Cache for len() method - allows Read() to work after Len() is called cachedResults []CollectionDocumentReplaceResponse cachedErrors []error cached bool + readIndex int // Track position in cache for Read() after Len() } func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentReplaceResponse, error) { + // If Len() was called, serve from cache + if c.cached { + if c.readIndex >= len(c.cachedResults) { + return CollectionDocumentReplaceResponse{}, shared.NoMoreDocumentsError{} + } + result := c.cachedResults[c.readIndex] + err := c.cachedErrors[c.readIndex] + c.readIndex++ + return result, err + } + + // Normal streaming read if !c.array.More() { return CollectionDocumentReplaceResponse{}, shared.NoMoreDocumentsError{} } @@ -198,11 +211,13 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl return meta, nil } -// Len returns the number of items in the response +// Len returns the number of items in the response. +// After calling Len(), you can still use Read() to iterate through items. func (c *collectionDocumentReplaceResponseReader) Len() int { if !c.cached { c.cachedResults, c.cachedErrors = c.ReadAll() c.cached = true + c.readIndex = 0 // Reset read position to allow Read() after Len() } return len(c.cachedResults) } diff --git a/v2/arangodb/collection_documents_update_impl.go b/v2/arangodb/collection_documents_update_impl.go index 23f57104..b6a4bb9b 100644 --- a/v2/arangodb/collection_documents_update_impl.go +++ b/v2/arangodb/collection_documents_update_impl.go @@ -150,13 +150,26 @@ type collectionDocumentUpdateResponseReader struct { } shared.ReadAllReader[CollectionDocumentUpdateResponse, *collectionDocumentUpdateResponseReader] - // Cache for len() method + // Cache for len() method - allows Read() to work after Len() is called cachedResults []CollectionDocumentUpdateResponse cachedErrors []error cached bool + readIndex int // Track position in cache for Read() after Len() } func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdateResponse, error) { + // If Len() was called, serve from cache + if c.cached { + if c.readIndex >= len(c.cachedResults) { + return CollectionDocumentUpdateResponse{}, shared.NoMoreDocumentsError{} + } + result := c.cachedResults[c.readIndex] + err := c.cachedErrors[c.readIndex] + c.readIndex++ + return result, err + } + + // Normal streaming read if !c.array.More() { return CollectionDocumentUpdateResponse{}, shared.NoMoreDocumentsError{} } @@ -198,11 +211,13 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat return meta, nil } -// Len returns the number of items in the response +// Len returns the number of items in the response. +// After calling Len(), you can still use Read() to iterate through items. func (c *collectionDocumentUpdateResponseReader) Len() int { if !c.cached { c.cachedResults, c.cachedErrors = c.ReadAll() c.cached = true + c.readIndex = 0 // Reset read position to allow Read() after Len() } return len(c.cachedResults) } diff --git a/v2/tests/benchmarks_test.go b/v2/tests/benchmarks_test.go index 1716f2d4..39616e48 100644 --- a/v2/tests/benchmarks_test.go +++ b/v2/tests/benchmarks_test.go @@ -209,7 +209,7 @@ func BenchmarkV2BulkInsert100KDocs(b *testing.B) { } func bulkRead(b *testing.B, docSize int) { - db, col := setup(b) + _, col := setup(b) // ----------------------------- // Prepare and insert documents @@ -237,28 +237,30 @@ func bulkRead(b *testing.B, docSize int) { require.NoError(b, err) // ----------------------------------------- - // Sub-benchmark 1: Read entire collection + // Sub-benchmark 1: Read entire collection using ReadDocuments // ----------------------------------------- b.Run("ReadAllDocsOnce", func(b *testing.B) { - query := fmt.Sprintf("FOR d IN %s RETURN d", col.Name()) + // Prepare keys for reading + keys := make([]string, docSize) + for j := 0; j < docSize; j++ { + keys[j] = fmt.Sprintf("doc_%d", j) + } b.ResetTimer() for i := 0; i < b.N; i++ { - cursor, err := db.Query(ctx, query, nil) + resp, err := col.ReadDocuments(ctx, keys) require.NoError(b, err) count := 0 for { var doc TestDoc - _, err := cursor.ReadDocument(ctx, &doc) + _, err := resp.Read(&doc) if shared.IsNoMoreDocuments(err) { break } require.NoError(b, err) count++ } - // require.Equal(b, docSize, count, "expected to read all documents") - _ = cursor.Close() // sanity check if count != docSize { b.Fatalf("expected to read %d docs, got %d", docSize, count) From 2add54eb14486815d471c2fc41d2c366d28ccf3b Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Tue, 28 Oct 2025 14:07:56 +0530 Subject: [PATCH 3/8] add note in CHANGELOG file --- v2/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/v2/CHANGELOG.md b/v2/CHANGELOG.md index 03cf8344..e580b982 100644 --- a/v2/CHANGELOG.md +++ b/v2/CHANGELOG.md @@ -7,6 +7,7 @@ - Disabled V8 related testcases in V1 and V2 - Added new ConsolidationPolicy attributes to support updated configuration options for ArangoSearch Views properties and Inverted Indexes - Add Vector index feature +- Add len() for Read methods ## [2.1.6](https://github.com/arangodb/go-driver/tree/v2.1.6) (2025-11-06) - Add missing endpoints from replication From 45ab135cbbd5b37ed94d2c260a60d8664e1ba8f5 Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Wed, 29 Oct 2025 12:47:41 +0530 Subject: [PATCH 4/8] addressed cursor comments --- v2/arangodb/collection_documents_create_impl.go | 17 +++++++++-------- .../collection_documents_replace_impl.go | 15 ++++----------- v2/arangodb/collection_documents_update_impl.go | 15 ++++----------- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/v2/arangodb/collection_documents_create_impl.go b/v2/arangodb/collection_documents_create_impl.go index a9a91610..b4589b09 100644 --- a/v2/arangodb/collection_documents_create_impl.go +++ b/v2/arangodb/collection_documents_create_impl.go @@ -24,6 +24,7 @@ import ( "context" "io" "net/http" + "reflect" "github.com/pkg/errors" @@ -177,12 +178,18 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat var meta CollectionDocumentCreateResponse if c.options != nil { - meta.Old = c.options.OldObject - meta.New = c.options.NewObject + if c.options.OldObject != nil { + meta.Old = reflect.New(reflect.TypeOf(c.options.OldObject).Elem()).Interface() + } + if c.options.NewObject != nil { + meta.New = reflect.New(reflect.TypeOf(c.options.NewObject).Elem()).Interface() + } } c.response.DocumentMeta = &meta.DocumentMeta c.response.ResponseStruct = &meta.ResponseStruct + c.response.Old = newUnmarshalInto(meta.Old) + c.response.New = newUnmarshalInto(meta.New) if err := c.array.Unmarshal(&c.response); err != nil { if err == io.EOF { @@ -191,12 +198,6 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat return CollectionDocumentCreateResponse{}, err } - // Update meta with the unmarshaled data - meta.DocumentMeta = *c.response.DocumentMeta - meta.ResponseStruct = *c.response.ResponseStruct - meta.Old = c.response.Old - meta.New = c.response.New - if meta.Error != nil && *meta.Error { return meta, meta.AsArangoError() } diff --git a/v2/arangodb/collection_documents_replace_impl.go b/v2/arangodb/collection_documents_replace_impl.go index 194f0dd3..f694f5f8 100644 --- a/v2/arangodb/collection_documents_replace_impl.go +++ b/v2/arangodb/collection_documents_replace_impl.go @@ -177,19 +177,18 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl var meta CollectionDocumentReplaceResponse if c.options != nil { - // Create new instances for each document to avoid reusing the same pointers if c.options.OldObject != nil { - oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() - meta.Old = reflect.New(oldObjectType).Interface() + meta.Old = reflect.New(reflect.TypeOf(c.options.OldObject).Elem()).Interface() } if c.options.NewObject != nil { - newObjectType := reflect.TypeOf(c.options.NewObject).Elem() - meta.New = reflect.New(newObjectType).Interface() + meta.New = reflect.New(reflect.TypeOf(c.options.NewObject).Elem()).Interface() } } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev c.response.ResponseStruct = &meta.ResponseStruct + c.response.Old = newUnmarshalInto(meta.Old) + c.response.New = newUnmarshalInto(meta.New) if err := c.array.Unmarshal(&c.response); err != nil { if err == io.EOF { @@ -198,12 +197,6 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl return CollectionDocumentReplaceResponse{}, err } - // Update meta with the unmarshaled data - meta.DocumentMetaWithOldRev = *c.response.DocumentMetaWithOldRev - meta.ResponseStruct = *c.response.ResponseStruct - meta.Old = c.response.Old - meta.New = c.response.New - if meta.Error != nil && *meta.Error { return meta, meta.AsArangoError() } diff --git a/v2/arangodb/collection_documents_update_impl.go b/v2/arangodb/collection_documents_update_impl.go index b6a4bb9b..1f069e71 100644 --- a/v2/arangodb/collection_documents_update_impl.go +++ b/v2/arangodb/collection_documents_update_impl.go @@ -177,19 +177,18 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat var meta CollectionDocumentUpdateResponse if c.options != nil { - // Create new instances for each document to avoid reusing the same pointers if c.options.OldObject != nil { - oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() - meta.Old = reflect.New(oldObjectType).Interface() + meta.Old = reflect.New(reflect.TypeOf(c.options.OldObject).Elem()).Interface() } if c.options.NewObject != nil { - newObjectType := reflect.TypeOf(c.options.NewObject).Elem() - meta.New = reflect.New(newObjectType).Interface() + meta.New = reflect.New(reflect.TypeOf(c.options.NewObject).Elem()).Interface() } } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev c.response.ResponseStruct = &meta.ResponseStruct + c.response.Old = newUnmarshalInto(meta.Old) + c.response.New = newUnmarshalInto(meta.New) if err := c.array.Unmarshal(&c.response); err != nil { if err == io.EOF { @@ -198,12 +197,6 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat return CollectionDocumentUpdateResponse{}, err } - // Update meta with the unmarshaled data - meta.DocumentMetaWithOldRev = *c.response.DocumentMetaWithOldRev - meta.ResponseStruct = *c.response.ResponseStruct - meta.Old = c.response.Old - meta.New = c.response.New - if meta.Error != nil && *meta.Error { return meta, meta.AsArangoError() } From 4d4dc5e3282f02d6545ec105990eb7c46f9f7498 Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Wed, 29 Oct 2025 13:52:02 +0530 Subject: [PATCH 5/8] code changes --- .../collection_documents_create_impl.go | 40 +++++++++++++++---- .../collection_documents_replace_impl.go | 40 +++++++++++++++---- .../collection_documents_update_impl.go | 40 +++++++++++++++---- 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/v2/arangodb/collection_documents_create_impl.go b/v2/arangodb/collection_documents_create_impl.go index b4589b09..c34f629f 100644 --- a/v2/arangodb/collection_documents_create_impl.go +++ b/v2/arangodb/collection_documents_create_impl.go @@ -130,6 +130,14 @@ func newCollectionDocumentCreateResponseReader(array *connection.Array, options c := &collectionDocumentCreateResponseReader{array: array, options: options} if c.options != nil { + // Cache reflection types once during initialization for performance + if c.options.OldObject != nil { + c.oldType = reflect.TypeOf(c.options.OldObject).Elem() + } + if c.options.NewObject != nil { + c.newType = reflect.TypeOf(c.options.NewObject).Elem() + } + c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } @@ -156,6 +164,10 @@ type collectionDocumentCreateResponseReader struct { cachedErrors []error cached bool readIndex int // Track position in cache for Read() after Len() + + // Performance: Cache reflection types to avoid repeated lookups + oldType reflect.Type + newType reflect.Type } func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreateResponse, error) { @@ -177,13 +189,13 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat var meta CollectionDocumentCreateResponse - if c.options != nil { - if c.options.OldObject != nil { - meta.Old = reflect.New(reflect.TypeOf(c.options.OldObject).Elem()).Interface() - } - if c.options.NewObject != nil { - meta.New = reflect.New(reflect.TypeOf(c.options.NewObject).Elem()).Interface() - } + // Create new instances for each document to avoid pointer reuse + // Use cached types for performance + if c.oldType != nil { + meta.Old = reflect.New(c.oldType).Interface() + } + if c.newType != nil { + meta.New = reflect.New(c.newType).Interface() } c.response.DocumentMeta = &meta.DocumentMeta @@ -202,6 +214,20 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat return meta, meta.AsArangoError() } + // Copy data from the new instances back to the original option objects for backward compatibility + if c.options != nil { + if c.options.OldObject != nil && meta.Old != nil { + oldValue := reflect.ValueOf(meta.Old).Elem() + originalValue := reflect.ValueOf(c.options.OldObject).Elem() + originalValue.Set(oldValue) + } + if c.options.NewObject != nil && meta.New != nil { + newValue := reflect.ValueOf(meta.New).Elem() + originalValue := reflect.ValueOf(c.options.NewObject).Elem() + originalValue.Set(newValue) + } + } + return meta, nil } diff --git a/v2/arangodb/collection_documents_replace_impl.go b/v2/arangodb/collection_documents_replace_impl.go index f694f5f8..9937b297 100644 --- a/v2/arangodb/collection_documents_replace_impl.go +++ b/v2/arangodb/collection_documents_replace_impl.go @@ -130,6 +130,14 @@ func newCollectionDocumentReplaceResponseReader(array *connection.Array, options c := &collectionDocumentReplaceResponseReader{array: array, options: options} if c.options != nil { + // Cache reflection types once during initialization for performance + if c.options.OldObject != nil { + c.oldType = reflect.TypeOf(c.options.OldObject).Elem() + } + if c.options.NewObject != nil { + c.newType = reflect.TypeOf(c.options.NewObject).Elem() + } + c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } @@ -155,6 +163,10 @@ type collectionDocumentReplaceResponseReader struct { cachedErrors []error cached bool readIndex int // Track position in cache for Read() after Len() + + // Performance: Cache reflection types to avoid repeated lookups + oldType reflect.Type + newType reflect.Type } func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentReplaceResponse, error) { @@ -176,13 +188,13 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl var meta CollectionDocumentReplaceResponse - if c.options != nil { - if c.options.OldObject != nil { - meta.Old = reflect.New(reflect.TypeOf(c.options.OldObject).Elem()).Interface() - } - if c.options.NewObject != nil { - meta.New = reflect.New(reflect.TypeOf(c.options.NewObject).Elem()).Interface() - } + // Create new instances for each document to avoid pointer reuse + // Use cached types for performance + if c.oldType != nil { + meta.Old = reflect.New(c.oldType).Interface() + } + if c.newType != nil { + meta.New = reflect.New(c.newType).Interface() } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev @@ -201,6 +213,20 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl return meta, meta.AsArangoError() } + // Copy data from the new instances back to the original option objects for backward compatibility + if c.options != nil { + if c.options.OldObject != nil && meta.Old != nil { + oldValue := reflect.ValueOf(meta.Old).Elem() + originalValue := reflect.ValueOf(c.options.OldObject).Elem() + originalValue.Set(oldValue) + } + if c.options.NewObject != nil && meta.New != nil { + newValue := reflect.ValueOf(meta.New).Elem() + originalValue := reflect.ValueOf(c.options.NewObject).Elem() + originalValue.Set(newValue) + } + } + return meta, nil } diff --git a/v2/arangodb/collection_documents_update_impl.go b/v2/arangodb/collection_documents_update_impl.go index 1f069e71..44ace99d 100644 --- a/v2/arangodb/collection_documents_update_impl.go +++ b/v2/arangodb/collection_documents_update_impl.go @@ -130,6 +130,14 @@ func newCollectionDocumentUpdateResponseReader(array *connection.Array, options c := &collectionDocumentUpdateResponseReader{array: array, options: options} if c.options != nil { + // Cache reflection types once during initialization for performance + if c.options.OldObject != nil { + c.oldType = reflect.TypeOf(c.options.OldObject).Elem() + } + if c.options.NewObject != nil { + c.newType = reflect.TypeOf(c.options.NewObject).Elem() + } + c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } @@ -155,6 +163,10 @@ type collectionDocumentUpdateResponseReader struct { cachedErrors []error cached bool readIndex int // Track position in cache for Read() after Len() + + // Performance: Cache reflection types to avoid repeated lookups + oldType reflect.Type + newType reflect.Type } func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdateResponse, error) { @@ -176,13 +188,13 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat var meta CollectionDocumentUpdateResponse - if c.options != nil { - if c.options.OldObject != nil { - meta.Old = reflect.New(reflect.TypeOf(c.options.OldObject).Elem()).Interface() - } - if c.options.NewObject != nil { - meta.New = reflect.New(reflect.TypeOf(c.options.NewObject).Elem()).Interface() - } + // Create new instances for each document to avoid pointer reuse + // Use cached types for performance + if c.oldType != nil { + meta.Old = reflect.New(c.oldType).Interface() + } + if c.newType != nil { + meta.New = reflect.New(c.newType).Interface() } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev @@ -201,6 +213,20 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat return meta, meta.AsArangoError() } + // Copy data from the new instances back to the original option objects for backward compatibility + if c.options != nil { + if c.options.OldObject != nil && meta.Old != nil { + oldValue := reflect.ValueOf(meta.Old).Elem() + originalValue := reflect.ValueOf(c.options.OldObject).Elem() + originalValue.Set(oldValue) + } + if c.options.NewObject != nil && meta.New != nil { + newValue := reflect.ValueOf(meta.New).Elem() + originalValue := reflect.ValueOf(c.options.NewObject).Elem() + originalValue.Set(newValue) + } + } + return meta, nil } From d40feb92ae574df0ede0bf607441ccd666526591 Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Wed, 24 Dec 2025 17:35:26 +0530 Subject: [PATCH 6/8] addressed cursor comments --- .../collection_documents_create_impl.go | 80 ++++++++---------- .../collection_documents_delete_impl.go | 55 ++++++------- v2/arangodb/collection_documents_read_impl.go | 55 ++++++------- .../collection_documents_replace_impl.go | 79 ++++++++---------- .../collection_documents_update_impl.go | 81 ++++++++----------- ...atabase_collection_doc_create_code_test.go | 12 ++- .../database_collection_doc_create_test.go | 12 +++ .../database_collection_doc_delete_test.go | 6 ++ v2/tests/database_collection_doc_read_test.go | 2 + .../database_collection_doc_update_test.go | 8 ++ .../database_collection_operations_test.go | 8 ++ v2/tests/database_query_test.go | 8 ++ 12 files changed, 202 insertions(+), 204 deletions(-) diff --git a/v2/arangodb/collection_documents_create_impl.go b/v2/arangodb/collection_documents_create_impl.go index c34f629f..e0a9cfcc 100644 --- a/v2/arangodb/collection_documents_create_impl.go +++ b/v2/arangodb/collection_documents_create_impl.go @@ -25,6 +25,7 @@ import ( "io" "net/http" "reflect" + "sync" "github.com/pkg/errors" @@ -50,6 +51,13 @@ func (c collectionDocumentCreate) CreateDocumentsWithOptions(ctx context.Context return nil, errors.Errorf("Input documents should be list") } + // Get document count from input (same as v1 approach) + documentsVal := reflect.ValueOf(documents) + if documentsVal.Kind() == reflect.Ptr { + documentsVal = documentsVal.Elem() + } + documentCount := documentsVal.Len() + url := c.collection.url("document") req, err := c.collection.connection().NewRequest(http.MethodPost, url) @@ -74,7 +82,7 @@ func (c collectionDocumentCreate) CreateDocumentsWithOptions(ctx context.Context case http.StatusCreated: fallthrough case http.StatusAccepted: - return newCollectionDocumentCreateResponseReader(&arr, opts), nil + return newCollectionDocumentCreateResponseReader(&arr, opts, documentCount), nil default: return nil, shared.NewResponseStruct().AsArangoErrorWithCode(code) } @@ -126,18 +134,14 @@ func (c collectionDocumentCreate) CreateDocument(ctx context.Context, document i return c.CreateDocumentWithOptions(ctx, document, nil) } -func newCollectionDocumentCreateResponseReader(array *connection.Array, options *CollectionDocumentCreateOptions) *collectionDocumentCreateResponseReader { - c := &collectionDocumentCreateResponseReader{array: array, options: options} +func newCollectionDocumentCreateResponseReader(array *connection.Array, options *CollectionDocumentCreateOptions, documentCount int) *collectionDocumentCreateResponseReader { + c := &collectionDocumentCreateResponseReader{ + array: array, + options: options, + documentCount: documentCount, + } if c.options != nil { - // Cache reflection types once during initialization for performance - if c.options.OldObject != nil { - c.oldType = reflect.TypeOf(c.options.OldObject).Elem() - } - if c.options.NewObject != nil { - c.newType = reflect.TypeOf(c.options.NewObject).Elem() - } - c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } @@ -149,40 +153,23 @@ func newCollectionDocumentCreateResponseReader(array *connection.Array, options var _ CollectionDocumentCreateResponseReader = &collectionDocumentCreateResponseReader{} type collectionDocumentCreateResponseReader struct { - array *connection.Array - options *CollectionDocumentCreateOptions - response struct { + array *connection.Array + options *CollectionDocumentCreateOptions + documentCount int // Store input document count for Len() without caching + response struct { *DocumentMeta *shared.ResponseStruct `json:",inline"` Old *UnmarshalInto `json:"old,omitempty"` New *UnmarshalInto `json:"new,omitempty"` } shared.ReadAllReader[CollectionDocumentCreateResponse, *collectionDocumentCreateResponseReader] - - // Cache for len() method - allows Read() to work after Len() is called - cachedResults []CollectionDocumentCreateResponse - cachedErrors []error - cached bool - readIndex int // Track position in cache for Read() after Len() - - // Performance: Cache reflection types to avoid repeated lookups - oldType reflect.Type - newType reflect.Type + mu sync.Mutex } func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreateResponse, error) { - // If Len() was called, serve from cache - if c.cached { - if c.readIndex >= len(c.cachedResults) { - return CollectionDocumentCreateResponse{}, shared.NoMoreDocumentsError{} - } - result := c.cachedResults[c.readIndex] - err := c.cachedErrors[c.readIndex] - c.readIndex++ - return result, err - } + c.mu.Lock() + defer c.mu.Unlock() - // Normal streaming read if !c.array.More() { return CollectionDocumentCreateResponse{}, shared.NoMoreDocumentsError{} } @@ -190,12 +177,15 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat var meta CollectionDocumentCreateResponse // Create new instances for each document to avoid pointer reuse - // Use cached types for performance - if c.oldType != nil { - meta.Old = reflect.New(c.oldType).Interface() - } - if c.newType != nil { - meta.New = reflect.New(c.newType).Interface() + if c.options != nil { + if c.options.OldObject != nil { + oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() + meta.Old = reflect.New(oldObjectType).Interface() + } + if c.options.NewObject != nil { + newObjectType := reflect.TypeOf(c.options.NewObject).Elem() + meta.New = reflect.New(newObjectType).Interface() + } } c.response.DocumentMeta = &meta.DocumentMeta @@ -232,12 +222,8 @@ func (c *collectionDocumentCreateResponseReader) Read() (CollectionDocumentCreat } // Len returns the number of items in the response. +// Returns the input document count immediately without reading/caching (same as v1 behavior). // After calling Len(), you can still use Read() to iterate through items. func (c *collectionDocumentCreateResponseReader) Len() int { - if !c.cached { - c.cachedResults, c.cachedErrors = c.ReadAll() - c.cached = true - c.readIndex = 0 // Reset read position to allow Read() after Len() - } - return len(c.cachedResults) + return c.documentCount } diff --git a/v2/arangodb/collection_documents_delete_impl.go b/v2/arangodb/collection_documents_delete_impl.go index 182aad82..0554b303 100644 --- a/v2/arangodb/collection_documents_delete_impl.go +++ b/v2/arangodb/collection_documents_delete_impl.go @@ -25,6 +25,7 @@ import ( "io" "net/http" "reflect" + "sync" "github.com/pkg/errors" @@ -80,6 +81,13 @@ func (c collectionDocumentDelete) DeleteDocumentsWithOptions(ctx context.Context return nil, errors.Errorf("Input documents should be list") } + // Get document count from input (same as v1 approach) + documentsVal := reflect.ValueOf(documents) + if documentsVal.Kind() == reflect.Ptr { + documentsVal = documentsVal.Elem() + } + documentCount := documentsVal.Len() + url := c.collection.url("document") req, err := c.collection.connection().NewRequest(http.MethodDelete, url) @@ -99,11 +107,15 @@ func (c collectionDocumentDelete) DeleteDocumentsWithOptions(ctx context.Context if err != nil { return nil, err } - return newCollectionDocumentDeleteResponseReader(&arr, opts), nil + return newCollectionDocumentDeleteResponseReader(&arr, opts, documentCount), nil } -func newCollectionDocumentDeleteResponseReader(array *connection.Array, options *CollectionDocumentDeleteOptions) *collectionDocumentDeleteResponseReader { - c := &collectionDocumentDeleteResponseReader{array: array, options: options} +func newCollectionDocumentDeleteResponseReader(array *connection.Array, options *CollectionDocumentDeleteOptions, documentCount int) *collectionDocumentDeleteResponseReader { + c := &collectionDocumentDeleteResponseReader{ + array: array, + options: options, + documentCount: documentCount, + } c.ReadAllIntoReader = shared.ReadAllIntoReader[CollectionDocumentDeleteResponse, *collectionDocumentDeleteResponseReader]{Reader: c} return c @@ -112,30 +124,17 @@ func newCollectionDocumentDeleteResponseReader(array *connection.Array, options var _ CollectionDocumentDeleteResponseReader = &collectionDocumentDeleteResponseReader{} type collectionDocumentDeleteResponseReader struct { - array *connection.Array - options *CollectionDocumentDeleteOptions + array *connection.Array + options *CollectionDocumentDeleteOptions + documentCount int // Store input document count for Len() without caching shared.ReadAllIntoReader[CollectionDocumentDeleteResponse, *collectionDocumentDeleteResponseReader] - // Cache for len() method - allows Read() to work after Len() is called - cachedResults []CollectionDocumentDeleteResponse - cachedErrors []error - cached bool - readIndex int // Track position in cache for Read() after Len() + mu sync.Mutex } func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (CollectionDocumentDeleteResponse, error) { - // If Len() was called, serve from cache - // Note: When serving from cache, the 'i' parameter is not populated with document data - if c.cached { - if c.readIndex >= len(c.cachedResults) { - return CollectionDocumentDeleteResponse{}, shared.NoMoreDocumentsError{} - } - result := c.cachedResults[c.readIndex] - err := c.cachedErrors[c.readIndex] - c.readIndex++ - return result, err - } + c.mu.Lock() + defer c.mu.Unlock() - // Normal streaming read if !c.array.More() { return CollectionDocumentDeleteResponse{}, shared.NoMoreDocumentsError{} } @@ -168,7 +167,7 @@ func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (Collection } if c.options != nil && c.options.OldObject != nil { - // Create a new instance for each document to avoid reusing the same pointer + // Create a new instance for each document to avoid pointer reuse oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() meta.Old = reflect.New(oldObjectType).Interface() @@ -187,14 +186,8 @@ func (c *collectionDocumentDeleteResponseReader) Read(i interface{}) (Collection } // Len returns the number of items in the response. +// Returns the input document count immediately without reading/caching (same as v1 behavior). // After calling Len(), you can still use Read() to iterate through items. -// Note: When Read() serves from cache, the document data parameter is not populated. func (c *collectionDocumentDeleteResponseReader) Len() int { - if !c.cached { - var dummySlice []interface{} - c.cachedResults, c.cachedErrors = c.ReadAll(&dummySlice) - c.cached = true - c.readIndex = 0 // Reset read position to allow Read() after Len() - } - return len(c.cachedResults) + return c.documentCount } diff --git a/v2/arangodb/collection_documents_read_impl.go b/v2/arangodb/collection_documents_read_impl.go index 5c99cdd7..7c0e357f 100644 --- a/v2/arangodb/collection_documents_read_impl.go +++ b/v2/arangodb/collection_documents_read_impl.go @@ -24,6 +24,8 @@ import ( "context" "io" "net/http" + "reflect" + "sync" "github.com/arangodb/go-driver/v2/arangodb/shared" "github.com/arangodb/go-driver/v2/connection" @@ -42,6 +44,13 @@ type collectionDocumentRead struct { } func (c collectionDocumentRead) ReadDocumentsWithOptions(ctx context.Context, documents interface{}, opts *CollectionDocumentReadOptions) (CollectionDocumentReadResponseReader, error) { + // Get document count from input (same as v1 approach) + documentsVal := reflect.ValueOf(documents) + if documentsVal.Kind() == reflect.Ptr { + documentsVal = documentsVal.Elem() + } + documentCount := documentsVal.Len() + url := c.collection.url("document") req, err := c.collection.connection().NewRequest(http.MethodPut, url) @@ -60,10 +69,11 @@ func (c collectionDocumentRead) ReadDocumentsWithOptions(ctx context.Context, do if _, err := c.collection.connection().Do(ctx, req, &arr, http.StatusOK); err != nil { return nil, err } - return newCollectionDocumentReadResponseReader(&arr, opts), nil + return newCollectionDocumentReadResponseReader(&arr, opts, documentCount), nil } func (c collectionDocumentRead) ReadDocuments(ctx context.Context, keys []string) (CollectionDocumentReadResponseReader, error) { + // ReadDocumentsWithOptions will calculate documentCount from keys using reflection return c.ReadDocumentsWithOptions(ctx, keys, nil) } @@ -95,8 +105,12 @@ func (c collectionDocumentRead) ReadDocumentWithOptions(ctx context.Context, key } } -func newCollectionDocumentReadResponseReader(array *connection.Array, options *CollectionDocumentReadOptions) *collectionDocumentReadResponseReader { - c := &collectionDocumentReadResponseReader{array: array, options: options} +func newCollectionDocumentReadResponseReader(array *connection.Array, options *CollectionDocumentReadOptions, documentCount int) *collectionDocumentReadResponseReader { + c := &collectionDocumentReadResponseReader{ + array: array, + options: options, + documentCount: documentCount, + } c.ReadAllIntoReader = shared.ReadAllIntoReader[CollectionDocumentReadResponse, *collectionDocumentReadResponseReader]{Reader: c} return c } @@ -104,29 +118,16 @@ func newCollectionDocumentReadResponseReader(array *connection.Array, options *C var _ CollectionDocumentReadResponseReader = &collectionDocumentReadResponseReader{} type collectionDocumentReadResponseReader struct { - array *connection.Array - options *CollectionDocumentReadOptions + array *connection.Array + options *CollectionDocumentReadOptions + documentCount int // Store input document count for Len() without caching shared.ReadAllIntoReader[CollectionDocumentReadResponse, *collectionDocumentReadResponseReader] - // Cache for len() method - allows Read() to work after Len() is called - cachedResults []CollectionDocumentReadResponse - cachedErrors []error - cached bool - readIndex int // Track position in cache for Read() after Len() + mu sync.Mutex } func (c *collectionDocumentReadResponseReader) Read(i interface{}) (CollectionDocumentReadResponse, error) { - // If Len() was called, serve from cache - // Note: When serving from cache, the 'i' parameter is not populated with document data - if c.cached { - if c.readIndex >= len(c.cachedResults) { - return CollectionDocumentReadResponse{}, shared.NoMoreDocumentsError{} - } - result := c.cachedResults[c.readIndex] - err := c.cachedErrors[c.readIndex] - c.readIndex++ - return result, err - } - + c.mu.Lock() + defer c.mu.Unlock() // Normal streaming read if !c.array.More() { return CollectionDocumentReadResponse{}, shared.NoMoreDocumentsError{} @@ -163,14 +164,8 @@ func (c *collectionDocumentReadResponseReader) Read(i interface{}) (CollectionDo } // Len returns the number of items in the response. +// Returns the input document count immediately without reading/caching (same as v1 behavior). // After calling Len(), you can still use Read() to iterate through items. -// Note: When Read() serves from cache, the document data parameter is not populated. func (c *collectionDocumentReadResponseReader) Len() int { - if !c.cached { - var dummySlice []interface{} - c.cachedResults, c.cachedErrors = c.ReadAll(&dummySlice) - c.cached = true - c.readIndex = 0 // Reset read position to allow Read() after Len() - } - return len(c.cachedResults) + return c.documentCount } diff --git a/v2/arangodb/collection_documents_replace_impl.go b/v2/arangodb/collection_documents_replace_impl.go index 9937b297..a7c24ba0 100644 --- a/v2/arangodb/collection_documents_replace_impl.go +++ b/v2/arangodb/collection_documents_replace_impl.go @@ -25,6 +25,7 @@ import ( "io" "net/http" "reflect" + "sync" "github.com/pkg/errors" @@ -96,6 +97,13 @@ func (c collectionDocumentReplace) ReplaceDocumentsWithOptions(ctx context.Conte return nil, errors.Errorf("Input documents should be list") } + // Get document count from input (same as v1 approach) + documentsVal := reflect.ValueOf(documents) + if documentsVal.Kind() == reflect.Ptr { + documentsVal = documentsVal.Elem() + } + documentCount := documentsVal.Len() + url := c.collection.url("document") req, err := c.collection.connection().NewRequest(http.MethodPut, url) @@ -120,24 +128,20 @@ func (c collectionDocumentReplace) ReplaceDocumentsWithOptions(ctx context.Conte case http.StatusCreated: fallthrough case http.StatusAccepted: - return newCollectionDocumentReplaceResponseReader(&arr, opts), nil + return newCollectionDocumentReplaceResponseReader(&arr, opts, documentCount), nil default: return nil, shared.NewResponseStruct().AsArangoErrorWithCode(code) } } -func newCollectionDocumentReplaceResponseReader(array *connection.Array, options *CollectionDocumentReplaceOptions) *collectionDocumentReplaceResponseReader { - c := &collectionDocumentReplaceResponseReader{array: array, options: options} +func newCollectionDocumentReplaceResponseReader(array *connection.Array, options *CollectionDocumentReplaceOptions, documentCount int) *collectionDocumentReplaceResponseReader { + c := &collectionDocumentReplaceResponseReader{ + array: array, + options: options, + documentCount: documentCount, + } if c.options != nil { - // Cache reflection types once during initialization for performance - if c.options.OldObject != nil { - c.oldType = reflect.TypeOf(c.options.OldObject).Elem() - } - if c.options.NewObject != nil { - c.newType = reflect.TypeOf(c.options.NewObject).Elem() - } - c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } @@ -148,9 +152,10 @@ func newCollectionDocumentReplaceResponseReader(array *connection.Array, options var _ CollectionDocumentReplaceResponseReader = &collectionDocumentReplaceResponseReader{} type collectionDocumentReplaceResponseReader struct { - array *connection.Array - options *CollectionDocumentReplaceOptions - response struct { + array *connection.Array + options *CollectionDocumentReplaceOptions + documentCount int // Store input document count for Len() without caching + response struct { *DocumentMetaWithOldRev *shared.ResponseStruct `json:",inline"` Old *UnmarshalInto `json:"old,omitempty"` @@ -158,30 +163,13 @@ type collectionDocumentReplaceResponseReader struct { } shared.ReadAllReader[CollectionDocumentReplaceResponse, *collectionDocumentReplaceResponseReader] - // Cache for len() method - allows Read() to work after Len() is called - cachedResults []CollectionDocumentReplaceResponse - cachedErrors []error - cached bool - readIndex int // Track position in cache for Read() after Len() - - // Performance: Cache reflection types to avoid repeated lookups - oldType reflect.Type - newType reflect.Type + mu sync.Mutex } func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentReplaceResponse, error) { - // If Len() was called, serve from cache - if c.cached { - if c.readIndex >= len(c.cachedResults) { - return CollectionDocumentReplaceResponse{}, shared.NoMoreDocumentsError{} - } - result := c.cachedResults[c.readIndex] - err := c.cachedErrors[c.readIndex] - c.readIndex++ - return result, err - } + c.mu.Lock() + defer c.mu.Unlock() - // Normal streaming read if !c.array.More() { return CollectionDocumentReplaceResponse{}, shared.NoMoreDocumentsError{} } @@ -189,12 +177,15 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl var meta CollectionDocumentReplaceResponse // Create new instances for each document to avoid pointer reuse - // Use cached types for performance - if c.oldType != nil { - meta.Old = reflect.New(c.oldType).Interface() - } - if c.newType != nil { - meta.New = reflect.New(c.newType).Interface() + if c.options != nil { + if c.options.OldObject != nil { + oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() + meta.Old = reflect.New(oldObjectType).Interface() + } + if c.options.NewObject != nil { + newObjectType := reflect.TypeOf(c.options.NewObject).Elem() + meta.New = reflect.New(newObjectType).Interface() + } } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev @@ -231,12 +222,8 @@ func (c *collectionDocumentReplaceResponseReader) Read() (CollectionDocumentRepl } // Len returns the number of items in the response. +// Returns the input document count immediately without reading/caching (same as v1 behavior). // After calling Len(), you can still use Read() to iterate through items. func (c *collectionDocumentReplaceResponseReader) Len() int { - if !c.cached { - c.cachedResults, c.cachedErrors = c.ReadAll() - c.cached = true - c.readIndex = 0 // Reset read position to allow Read() after Len() - } - return len(c.cachedResults) + return c.documentCount } diff --git a/v2/arangodb/collection_documents_update_impl.go b/v2/arangodb/collection_documents_update_impl.go index 44ace99d..e1143d7d 100644 --- a/v2/arangodb/collection_documents_update_impl.go +++ b/v2/arangodb/collection_documents_update_impl.go @@ -25,6 +25,7 @@ import ( "io" "net/http" "reflect" + "sync" "github.com/pkg/errors" @@ -96,6 +97,13 @@ func (c collectionDocumentUpdate) UpdateDocumentsWithOptions(ctx context.Context return nil, errors.Errorf("Input documents should be list") } + // Get document count from input (same as v1 approach) + documentsVal := reflect.ValueOf(documents) + if documentsVal.Kind() == reflect.Ptr { + documentsVal = documentsVal.Elem() + } + documentCount := documentsVal.Len() + url := c.collection.url("document") req, err := c.collection.connection().NewRequest(http.MethodPatch, url) @@ -120,27 +128,24 @@ func (c collectionDocumentUpdate) UpdateDocumentsWithOptions(ctx context.Context case http.StatusCreated: fallthrough case http.StatusAccepted: - return newCollectionDocumentUpdateResponseReader(&arr, opts), nil + return newCollectionDocumentUpdateResponseReader(&arr, opts, documentCount), nil default: return nil, shared.NewResponseStruct().AsArangoErrorWithCode(code) } } -func newCollectionDocumentUpdateResponseReader(array *connection.Array, options *CollectionDocumentUpdateOptions) *collectionDocumentUpdateResponseReader { - c := &collectionDocumentUpdateResponseReader{array: array, options: options} +func newCollectionDocumentUpdateResponseReader(array *connection.Array, options *CollectionDocumentUpdateOptions, documentCount int) *collectionDocumentUpdateResponseReader { + c := &collectionDocumentUpdateResponseReader{ + array: array, + options: options, + documentCount: documentCount, + } if c.options != nil { - // Cache reflection types once during initialization for performance - if c.options.OldObject != nil { - c.oldType = reflect.TypeOf(c.options.OldObject).Elem() - } - if c.options.NewObject != nil { - c.newType = reflect.TypeOf(c.options.NewObject).Elem() - } - c.response.Old = newUnmarshalInto(c.options.OldObject) c.response.New = newUnmarshalInto(c.options.NewObject) } + c.ReadAllReader = shared.ReadAllReader[CollectionDocumentUpdateResponse, *collectionDocumentUpdateResponseReader]{Reader: c} return c } @@ -148,9 +153,10 @@ func newCollectionDocumentUpdateResponseReader(array *connection.Array, options var _ CollectionDocumentUpdateResponseReader = &collectionDocumentUpdateResponseReader{} type collectionDocumentUpdateResponseReader struct { - array *connection.Array - options *CollectionDocumentUpdateOptions - response struct { + array *connection.Array + options *CollectionDocumentUpdateOptions + documentCount int // Store input document count for Len() without caching + response struct { *DocumentMetaWithOldRev *shared.ResponseStruct `json:",inline"` Old *UnmarshalInto `json:"old,omitempty"` @@ -158,30 +164,12 @@ type collectionDocumentUpdateResponseReader struct { } shared.ReadAllReader[CollectionDocumentUpdateResponse, *collectionDocumentUpdateResponseReader] - // Cache for len() method - allows Read() to work after Len() is called - cachedResults []CollectionDocumentUpdateResponse - cachedErrors []error - cached bool - readIndex int // Track position in cache for Read() after Len() - - // Performance: Cache reflection types to avoid repeated lookups - oldType reflect.Type - newType reflect.Type + mu sync.Mutex } func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdateResponse, error) { - // If Len() was called, serve from cache - if c.cached { - if c.readIndex >= len(c.cachedResults) { - return CollectionDocumentUpdateResponse{}, shared.NoMoreDocumentsError{} - } - result := c.cachedResults[c.readIndex] - err := c.cachedErrors[c.readIndex] - c.readIndex++ - return result, err - } - - // Normal streaming read + c.mu.Lock() + defer c.mu.Unlock() if !c.array.More() { return CollectionDocumentUpdateResponse{}, shared.NoMoreDocumentsError{} } @@ -189,12 +177,15 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat var meta CollectionDocumentUpdateResponse // Create new instances for each document to avoid pointer reuse - // Use cached types for performance - if c.oldType != nil { - meta.Old = reflect.New(c.oldType).Interface() - } - if c.newType != nil { - meta.New = reflect.New(c.newType).Interface() + if c.options != nil { + if c.options.OldObject != nil { + oldObjectType := reflect.TypeOf(c.options.OldObject).Elem() + meta.Old = reflect.New(oldObjectType).Interface() + } + if c.options.NewObject != nil { + newObjectType := reflect.TypeOf(c.options.NewObject).Elem() + meta.New = reflect.New(newObjectType).Interface() + } } c.response.DocumentMetaWithOldRev = &meta.DocumentMetaWithOldRev @@ -231,12 +222,8 @@ func (c *collectionDocumentUpdateResponseReader) Read() (CollectionDocumentUpdat } // Len returns the number of items in the response. +// Returns the input document count immediately without reading/caching (same as v1 behavior). // After calling Len(), you can still use Read() to iterate through items. func (c *collectionDocumentUpdateResponseReader) Len() int { - if !c.cached { - c.cachedResults, c.cachedErrors = c.ReadAll() - c.cached = true - c.readIndex = 0 // Reset read position to allow Read() after Len() - } - return len(c.cachedResults) + return c.documentCount } diff --git a/v2/tests/database_collection_doc_create_code_test.go b/v2/tests/database_collection_doc_create_code_test.go index 19d97fc1..f86bc210 100644 --- a/v2/tests/database_collection_doc_create_code_test.go +++ b/v2/tests/database_collection_doc_create_code_test.go @@ -28,6 +28,7 @@ import ( "github.com/arangodb/go-driver/v2/arangodb" "github.com/arangodb/go-driver/v2/arangodb/shared" + "github.com/arangodb/go-driver/v2/utils" ) type DocWithCode struct { @@ -69,10 +70,11 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { Key: "test2", } - _, err := col.CreateDocuments(ctx, []any{ + readerCreate, err := col.CreateDocuments(ctx, []any{ doc, doc2, }) require.NoError(t, err) + require.Equal(t, 2, readerCreate.Len(), "CreateDocuments should return a reader with 2 documents") docs, err := col.ReadDocuments(ctx, []string{ "test", @@ -80,6 +82,7 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { "test2", }) require.NoError(t, err) + require.Equal(t, 3, docs.Len(), "ReadDocuments should return a reader with 3 documents") var z DocWithCode @@ -126,9 +129,8 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { "test", "test2", "nonexistent", }) require.NoError(t, err) - + require.Equal(t, 3, readeRed.Len(), "ReadDocuments should return a reader with 3 documents") metaRed, errs := readeRed.ReadAll(&docRedRead) - require.ElementsMatch(t, []any{doc1.Key, doc2.Key}, []any{metaRed[0].Key, metaRed[1].Key}) require.ElementsMatch(t, []any{nil, nil, shared.ErrArangoDocumentNotFound}, []any{errs[0], errs[1], errs[2].(shared.ArangoError).ErrorNum}) @@ -140,6 +142,7 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { }, &arangodb.CollectionDocumentDeleteOptions{OldObject: &docOldObject}) require.NoError(t, err) metaDel, errs := readerDel.ReadAll(&docDelRead) + require.Equal(t, 3, readerDel.Len(), "ReadAll() should return 3 results matching number of delete attempts") require.ElementsMatch(t, []any{doc1.Key, doc2.Key, ""}, []any{metaDel[0].Key, metaDel[1].Key, metaDel[2].Key}) require.ElementsMatch(t, []any{nil, nil, shared.ErrArangoDocumentNotFound}, []any{errs[0], errs[1], errs[2].(shared.ArangoError).ErrorNum}) @@ -150,5 +153,8 @@ func Test_DatabaseCollectionDocCreateCode(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) + } diff --git a/v2/tests/database_collection_doc_create_test.go b/v2/tests/database_collection_doc_create_test.go index 9eaa181e..e555bd9d 100644 --- a/v2/tests/database_collection_doc_create_test.go +++ b/v2/tests/database_collection_doc_create_test.go @@ -126,6 +126,8 @@ func Test_DatabaseCollectionDocCreateOverwrite(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -197,6 +199,8 @@ func Test_DatabaseCollectionDocCreateKeepNull(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -278,6 +282,8 @@ func Test_DatabaseCollectionDocCreateMergeObjects(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -301,6 +307,8 @@ func Test_DatabaseCollectionDocCreateSilent(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -332,6 +340,8 @@ func Test_DatabaseCollectionDocCreateWaitForSync(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -400,5 +410,7 @@ func Test_DatabaseCollectionDocCreateReplaceWithVersionAttribute(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } diff --git a/v2/tests/database_collection_doc_delete_test.go b/v2/tests/database_collection_doc_delete_test.go index b171fb5f..a35cce40 100644 --- a/v2/tests/database_collection_doc_delete_test.go +++ b/v2/tests/database_collection_doc_delete_test.go @@ -158,6 +158,8 @@ func Test_DatabaseCollectionDocDeleteSimple(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -192,6 +194,8 @@ func Test_DatabaseCollectionDocDeleteIfMatch(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -287,6 +291,8 @@ func Test_DatabaseCollectionDocDeleteIgnoreRevs(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } diff --git a/v2/tests/database_collection_doc_read_test.go b/v2/tests/database_collection_doc_read_test.go index 1f9c2436..08103f6d 100644 --- a/v2/tests/database_collection_doc_read_test.go +++ b/v2/tests/database_collection_doc_read_test.go @@ -157,6 +157,8 @@ func Test_DatabaseCollectionDocReadIgnoreRevs(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(true), }) } diff --git a/v2/tests/database_collection_doc_update_test.go b/v2/tests/database_collection_doc_update_test.go index 94ad5aa0..32e1ae6c 100644 --- a/v2/tests/database_collection_doc_update_test.go +++ b/v2/tests/database_collection_doc_update_test.go @@ -73,6 +73,8 @@ func Test_DatabaseCollectionDocUpdateIfMatch(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -123,6 +125,8 @@ func Test_DatabaseCollectionDocUpdateIgnoreRevs(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -217,6 +221,8 @@ func Test_DatabaseCollectionDocUpdateKeepNull(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -294,6 +300,8 @@ func Test_DatabaseCollectionDocUpdateMergeObjects(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } diff --git a/v2/tests/database_collection_operations_test.go b/v2/tests/database_collection_operations_test.go index b32d1a55..2fd29d8a 100644 --- a/v2/tests/database_collection_operations_test.go +++ b/v2/tests/database_collection_operations_test.go @@ -247,6 +247,8 @@ func Test_WithQueryOptimizerRules(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -503,6 +505,8 @@ func Test_DatabaseCollectionOperations(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -598,6 +602,8 @@ func Test_DatabaseCollectionBulkOperations(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -628,6 +634,8 @@ func Test_DatabaseCollectionTruncate(t *testing.T) { }) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } diff --git a/v2/tests/database_query_test.go b/v2/tests/database_query_test.go index 42c89a32..575a39b5 100644 --- a/v2/tests/database_query_test.go +++ b/v2/tests/database_query_test.go @@ -812,6 +812,8 @@ func Test_GetQueryPlanCache(t *testing.T) { } }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -944,6 +946,8 @@ func Test_ClearQueryPlanCache(t *testing.T) { require.NoError(t, err) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -1111,6 +1115,8 @@ func Test_GetQueryEntriesCache(t *testing.T) { } }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } @@ -1242,6 +1248,8 @@ func Test_ClearQueryCache(t *testing.T) { require.NoError(t, err) }) }) + }, WrapOptions{ + Parallel: utils.NewType(false), }) } From f5c2f6d52c0f3a69494dcdb78c6121c1936cfb8d Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Wed, 24 Dec 2025 18:04:23 +0530 Subject: [PATCH 7/8] reverted benchmark chages in v1 --- test/benchmarks_test.go | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/test/benchmarks_test.go b/test/benchmarks_test.go index 6045b641..85c16595 100644 --- a/test/benchmarks_test.go +++ b/test/benchmarks_test.go @@ -87,7 +87,7 @@ func BenchmarkV1BulkInsert100KDocs(b *testing.B) { } func bulkRead(b *testing.B, docSize int) { - _, col := setup(b) + db, col := setup(b) // ----------------------------- // Prepare and insert documents @@ -110,24 +110,31 @@ func bulkRead(b *testing.B, docSize int) { require.NoError(b, err) // ----------------------------------------- - // Sub-benchmark 1: Read entire collection using ReadDocuments + // Sub-benchmark 1: Read entire collection // ----------------------------------------- b.Run("ReadAllDocsOnce", func(b *testing.B) { - // Prepare keys for reading - keys := make([]string, docSize) - for j := 0; j < docSize; j++ { - keys[j] = fmt.Sprintf("doc_%d", j) - } + query := fmt.Sprintf("FOR d IN %s RETURN d", col.Name()) b.ResetTimer() for i := 0; i < b.N; i++ { - readDocs := make([]TestDoc, docSize) - _, _, err := col.ReadDocuments(ctx, keys, readDocs) + cursor, err := db.Query(ctx, query, nil) require.NoError(b, err) + count := 0 + for { + var doc TestDoc + _, err := cursor.ReadDocument(ctx, &doc) + if driver.IsNoMoreDocuments(err) { + break + } + require.NoError(b, err) + count++ + } + // require.Equal(b, docSize, count, "expected to read all documents") + _ = cursor.Close() // sanity check - if len(readDocs) != docSize { - b.Fatalf("expected to read %d docs, got %d", docSize, len(readDocs)) + if count != docSize { + b.Fatalf("expected to read %d docs, got %d", docSize, count) } } }) From a9953cbc7e93d2dd76a07808a73db333a5b58329 Mon Sep 17 00:00:00 2001 From: bluepal-prasanthi-moparthi Date: Wed, 24 Dec 2025 18:08:17 +0530 Subject: [PATCH 8/8] Revert "reverted benchmark chages in v1" This reverts commit f5c2f6d52c0f3a69494dcdb78c6121c1936cfb8d. --- test/benchmarks_test.go | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/test/benchmarks_test.go b/test/benchmarks_test.go index 85c16595..6045b641 100644 --- a/test/benchmarks_test.go +++ b/test/benchmarks_test.go @@ -87,7 +87,7 @@ func BenchmarkV1BulkInsert100KDocs(b *testing.B) { } func bulkRead(b *testing.B, docSize int) { - db, col := setup(b) + _, col := setup(b) // ----------------------------- // Prepare and insert documents @@ -110,31 +110,24 @@ func bulkRead(b *testing.B, docSize int) { require.NoError(b, err) // ----------------------------------------- - // Sub-benchmark 1: Read entire collection + // Sub-benchmark 1: Read entire collection using ReadDocuments // ----------------------------------------- b.Run("ReadAllDocsOnce", func(b *testing.B) { - query := fmt.Sprintf("FOR d IN %s RETURN d", col.Name()) + // Prepare keys for reading + keys := make([]string, docSize) + for j := 0; j < docSize; j++ { + keys[j] = fmt.Sprintf("doc_%d", j) + } b.ResetTimer() for i := 0; i < b.N; i++ { - cursor, err := db.Query(ctx, query, nil) + readDocs := make([]TestDoc, docSize) + _, _, err := col.ReadDocuments(ctx, keys, readDocs) require.NoError(b, err) - count := 0 - for { - var doc TestDoc - _, err := cursor.ReadDocument(ctx, &doc) - if driver.IsNoMoreDocuments(err) { - break - } - require.NoError(b, err) - count++ - } - // require.Equal(b, docSize, count, "expected to read all documents") - _ = cursor.Close() // sanity check - if count != docSize { - b.Fatalf("expected to read %d docs, got %d", docSize, count) + if len(readDocs) != docSize { + b.Fatalf("expected to read %d docs, got %d", docSize, len(readDocs)) } } })