From bdbda54923c61db3e1d5d804408d8617e20b01ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Mat=C3=ADas=20Gomez?= Date: Thu, 14 Nov 2024 10:28:11 -0300 Subject: [PATCH] Return ObjectIndex on file.Append() call (#59) * Return ObjectIndex on file.Append() call * Add AppendAndReturnIndex to avoid breaking change --- download_test.go | 3 ++- temp_file.go | 24 ++++++++++++------ temp_file_test.go | 63 +++++++++++++++++++++++++++++++++++++++++------ upload_test.go | 6 +++-- 4 files changed, 78 insertions(+), 18 deletions(-) diff --git a/download_test.go b/download_test.go index 5dc383d..fc1afe8 100644 --- a/download_test.go +++ b/download_test.go @@ -110,7 +110,8 @@ func TestClient_Fetch(t *testing.T) { for _, fixture := range []objUploadFixture{fixture1, fixture2, fixture3} { b, err := marshalAndCompress(fixture.obj) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(file.Append(fixture.obj.ID, b)).ToNot(HaveOccurred()) + _, err = file.AppendAndReturnIndex(fixture.obj.ID, b) + g.Expect(err).ToNot(HaveOccurred()) } err = c.UploadFile(ctx, file, true) diff --git a/temp_file.go b/temp_file.go index 1f719be..17bd533 100644 --- a/temp_file.go +++ b/temp_file.go @@ -56,14 +56,21 @@ func NewTempFile[K comparable](tags map[string]string) (*TempFile[K], error) { }, nil } -// Append will take an id, and the slice of bytes of the Object, and append it to the temp file. -// This will also store the associated ObjectIndex information for this slice of bytes, -// telling where the object is located in this file (file, offset, length) +// Append is the same as AppendAndReturnIndex but doesn't return an index. This method could be deleted, but +// it is kept for backwards compatibility. +func (f *TempFile[K]) Append(id K, bytes []byte) error { + _, err := f.AppendAndReturnIndex(id, bytes) + return err +} + +// AppendAndReturnIndex will take an id, and the slice of bytes of the Object, and append it to the temp file. +// This will also return the associated ObjectIndex information for this slice of bytes, which tells +// where the object is located in this file (file, offset, length) // This method is not thread safe, if you expect to make concurrent calls to Append, you should protect it. // If you provide the same id twice, the second call will overwrite the first one, but the file will still grow in size. -func (f *TempFile[K]) Append(id K, bytes []byte) error { +func (f *TempFile[K]) AppendAndReturnIndex(id K, bytes []byte) (ObjectIndex, error) { if f.readonly { - return fmt.Errorf("file %s is readonly", f.fileName) + return ObjectIndex{}, fmt.Errorf("file %s is readonly", f.fileName) } length := uint64(len(bytes)) @@ -71,21 +78,22 @@ func (f *TempFile[K]) Append(id K, bytes []byte) error { // Append to file bytesWritten, err := f.file.Write(bytes) if err != nil { - return fmt.Errorf("failed to write %d bytes (%d written) to file %s: %w", length, bytesWritten, f.file.Name(), err) + return ObjectIndex{}, fmt.Errorf("failed to write %d bytes (%d written) to file %s: %w", length, bytesWritten, f.file.Name(), err) } // Add index - f.indexes[id] = ObjectIndex{ + index := ObjectIndex{ File: f.fileName, Offset: f.offset, Length: length, } + f.indexes[id] = index // Increment counters/metrics f.count++ f.offset += length - return nil + return index, nil } // Name returns the fileName diff --git a/temp_file_test.go b/temp_file_test.go index ebd5a4b..eea98db 100644 --- a/temp_file_test.go +++ b/temp_file_test.go @@ -18,6 +18,38 @@ var testTags = map[string]string{ "retention-days": "14", } +func TestFile_Append(t *testing.T) { + g := NewGomegaWithT(t) + + c := client[string]{} + + file, err := c.NewTempFile(testTags) + g.Expect(err).ToNot(HaveOccurred()) + defer func() { _ = file.Close() }() + + obj1 := &TestObject{ID: "4", Value: "contents"} + compressed, err := marshalAndCompress(obj1) + g.Expect(err).ToNot(HaveOccurred()) + + err = file.Append(obj1.ID, compressed) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(file.Name()).To(Equal(file.fileName)) + g.Expect(file.Indexes()[obj1.ID].Offset).To(Equal(uint64(0))) + g.Expect(file.Indexes()[obj1.ID].Length).To(BeNumerically(">", 0)) + + // Add another object, using AppendAndReturnIndex + obj2 := &TestObject{ID: "5", Value: "contents"} + compressed, err = marshalAndCompress(obj2) + g.Expect(err).ToNot(HaveOccurred()) + + index, err := file.AppendAndReturnIndex(obj2.ID, compressed) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(file.Name()).To(Equal(file.fileName)) + g.Expect(file.Indexes()[obj2.ID]).To(Equal(index)) + g.Expect(file.Indexes()[obj2.ID].Offset).To(Equal(file.Indexes()[obj1.ID].Length)) + g.Expect(file.Indexes()[obj2.ID].Length).To(BeNumerically(">", 0)) +} + func TestFile_WriteError(t *testing.T) { g := NewGomegaWithT(t) @@ -31,18 +63,26 @@ func TestFile_WriteError(t *testing.T) { compressed, err := marshalAndCompress(obj) g.Expect(err).ToNot(HaveOccurred()) - err = file.Append(obj.ID, compressed) + index, err := file.AppendAndReturnIndex(obj.ID, compressed) g.Expect(err).ToNot(HaveOccurred()) g.Expect(file.Name()).To(Equal(file.fileName)) + g.Expect(file.Indexes()[obj.ID]).To(Equal(index)) g.Expect(file.Indexes()[obj.ID].Offset).To(Equal(uint64(0))) g.Expect(file.Indexes()[obj.ID].Length).To(BeNumerically(">", 0)) // If file is closed, it won't be able to write more: g.Expect(file.file.Close()).ToNot(HaveOccurred()) - err = file.Append(obj.ID, compressed) + // Try to append a new object + obj = &TestObject{ID: "5", Value: "contents"} + compressed, err = marshalAndCompress(obj) + g.Expect(err).ToNot(HaveOccurred()) + + index, err = file.AppendAndReturnIndex(obj.ID, compressed) fileName := file.file.Name() g.Expect(err).To(MatchError(fmt.Sprintf("failed to write %d bytes (0 written) to file %s: write %s: file already closed", len(compressed), fileName, fileName))) + g.Expect(index).To(Equal(ObjectIndex{})) + g.Expect(file.Indexes()[obj.ID]).To(Equal(index)) } func TestFile_ReadOnly(t *testing.T) { @@ -59,17 +99,25 @@ func TestFile_ReadOnly(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) // Store one object, then ask for the readonly file and try to store one more object - err = file.Append(obj.ID, compressed) + index, err := file.AppendAndReturnIndex(obj.ID, compressed) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(file.Indexes()[obj.ID].Offset).To(Equal(uint64(0))) - g.Expect(file.Indexes()[obj.ID].Length).To(BeNumerically(">", 0)) + g.Expect(file.Indexes()[obj.ID]).To(Equal(index)) + g.Expect(index.Offset).To(Equal(uint64(0))) + g.Expect(index.Length).To(BeNumerically(">", 0)) roFile, err := file.readOnly() g.Expect(roFile).ToNot(BeNil()) g.Expect(err).To(BeNil()) - err = file.Append(obj.ID, compressed) + // Append a new object + obj = &TestObject{ID: "5", Value: "contents"} + compressed, err = marshalAndCompress(obj) + g.Expect(err).ToNot(HaveOccurred()) + + index, err = file.AppendAndReturnIndex(obj.ID, compressed) g.Expect(err).To(MatchError(fmt.Sprintf("file %s is readonly", file.fileName))) + g.Expect(index).To(Equal(ObjectIndex{})) + g.Expect(file.Indexes()[obj.ID]).To(Equal(index)) } func TestFile_ReadOnlyError(t *testing.T) { @@ -85,8 +133,9 @@ func TestFile_ReadOnlyError(t *testing.T) { compressed, err := marshalAndCompress(obj) g.Expect(err).ToNot(HaveOccurred()) - err = file.Append(obj.ID, compressed) + index, err := file.AppendAndReturnIndex(obj.ID, compressed) g.Expect(err).ToNot(HaveOccurred()) + g.Expect(file.Indexes()[obj.ID]).To(Equal(index)) g.Expect(file.Indexes()[obj.ID].Offset).To(Equal(uint64(0))) g.Expect(file.Indexes()[obj.ID].Length).To(BeNumerically(">", 0)) diff --git a/upload_test.go b/upload_test.go index 6f61d1b..e4a73b5 100644 --- a/upload_test.go +++ b/upload_test.go @@ -168,7 +168,8 @@ func TestClient_UploadFile(t *testing.T) { for _, objs := range test.objs { compressed, err := marshalAndCompress(objs) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(file.Append(objs.ID, compressed)).ToNot(HaveOccurred()) + _, err = file.AppendAndReturnIndex(objs.ID, compressed) + g.Expect(err).ToNot(HaveOccurred()) } g.Expect(file.Count()).To(Equal(uint(len(test.objs)))) g.Expect(file.Age()).To(BeNumerically(">=", uint64(0))) @@ -270,7 +271,8 @@ func TestClient_DeleteFile(t *testing.T) { for _, obj := range test.objs { compressed, err := marshalAndCompress(obj) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(file.Append(obj.ID, compressed)).ToNot(HaveOccurred()) + _, err = file.AppendAndReturnIndex(obj.ID, compressed) + g.Expect(err).ToNot(HaveOccurred()) } g.Expect(file.Count()).To(Equal(uint(len(test.objs)))) g.Expect(file.Age()).To(BeNumerically(">=", uint64(0)))