Code review fixes (#8)

* Fix README and other comments

* Rename methods

* ADd comment

README.md

@@ -1,7 +1,7 @@
 # s3-batch-object-store
 
 `s3-batch-object-store` is a Go module that allows for batch uploading of objects to a single S3 file and retrieving 
-each object separately using the AWS S3 API, fetching only the bytes for that specific object
+each object separately using the AWS S3 API, fetching only the bytes for that specific object.
 
 ## Features
 
@@ -73,13 +73,21 @@ func main() {
 		}
 	}
 
+	// You can check the file properties to decide when to upload a file:
+	fmt.Printf("File is %s old, has %d objects, and is %d bytes long\n", file.Age(), file.Count(), file.Size())
+	// File is 42.375µs old, has 3 objects, and is 93 bytes long
+
 	// Upload the objects
-	err = client.UploadToS3(ctx, file, true)
+	err = client.UploadFile(ctx, file, true)
 	if err != nil {
 		panic("failed to upload object: " + err.Error())
 	}
 
 	// At this point the file.Indexes() can be stored to be used later to retrieve the objects.
+	fmt.Printf("File indexes:\n")
+	for id, index := range file.Indexes() {
+		fmt.Printf("objectID: %v, index: %+v\n", id, index)
+	}
 
 	// Retrieve an object
 	indexes := file.Indexes()
@@ -92,8 +100,6 @@ func main() {
 	// Contents of object2:
 	// This is the content of object2.
 }
-
-
 ```
 
 ## Contributing

client.go

@@ -9,27 +9,27 @@ import (
 
 // Client is the client used to store and fetch object to/from s3.
 // K represents the type of IDs for the objects that will be uploaded
-// To create a new file, first call NewTempFile, then append objects to it, and finally call UploadToS3.
+// To create a new file, first call NewTempFile, then append objects to it, and finally call UploadFile.
 // After the file is uploaded, you can save the object indexes to a database, and use them to fetch the objects later.
 // To fetch the contents of a single object, call Fetch with the ObjectIndex that you had stored.
 //
 //go:generate mockgen -source=./client.go -destination=./mock/client/mock_client.go -package=mocks3batchstore Client
 type Client[K comparable] interface {
-	// NewTempFile Creates a new file in a temp folder
+	// NewTempFile creates a new file in a temp folder.
 	// tags can be used to store information about this file in S3, like retention days
 	// The file itself is not thread safe, if you expect to make concurrent calls to Append, you should protect it.
-	// Once all the objects are appended, you can call UploadToS3 to upload the file to s3.
+	// Once all the objects are appended, you can call UploadFile to upload the file to s3.
 	NewTempFile(tags map[string]string) (*TempFile[K], error)
 
-	// UploadToS3 will take a TempFile that already has all the objects in it, and upload it to a s3 file,
+	// UploadFile will take a TempFile that already has all the objects in it, and upload it to a s3 file,
 	// in one single operation.
 	// withMetaFile indicates whether the metadata will be also uploaded to the file.MetaFileKey() location,
 	// with the index information for each object, or not.
-	UploadToS3(ctx context.Context, file *TempFile[K], withMetaFile bool) error
+	UploadFile(ctx context.Context, file *TempFile[K], withMetaFile bool) error
 
-	// DeleteFromS3 allows to try to delete any files that may have been uploaded to s3 based on the provided file.
-	// This is provided in case of any error when calling UploadToS3, callers have the possibility to clean up the files.
-	DeleteFromS3(ctx context.Context, file *TempFile[K]) error
+	// DeleteFile allows to try to delete any files that may have been uploaded to s3 based on the provided file.
+	// This is provided in case of any error when calling UploadFile, callers have the possibility to clean up the files.
+	DeleteFile(ctx context.Context, file *TempFile[K]) error
 
 	// Fetch downloads the payload from s3 given the ObjectIndex, fetching only the needed bytes, and returning
 	// the payload as a byte array.

download_test.go

@@ -113,7 +113,7 @@ func TestClient_Fetch(t *testing.T) {
 		g.Expect(file.Append(fixture.obj.ID, b)).ToNot(HaveOccurred())
 	}
 
-	err = c.UploadToS3(ctx, file, true)
+	err = c.UploadFile(ctx, file, true)
 	g.Expect(err).To(BeNil())
 	g.Expect(len(file.indexes)).To(Equal(len(expectedIndexes)))
 	for id, index := range expectedIndexes {

temp_file.go

@@ -24,7 +24,7 @@ type TempFile[K comparable] struct {
 	tags      map[string]string
 
 	readonly bool
-	count    int    // How many items are currently saved in the file
+	count    uint   // How many items are currently saved in the file
 	offset   uint64 // The current offset in the file
 	indexes  map[K]ObjectIndex
 }
@@ -60,13 +60,14 @@ func NewTempFile[K comparable](tags map[string]string) (*TempFile[K], error) {
 // 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)
 // 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 {
-	length := uint64(len(bytes))
-
 	if f.readonly {
 		return fmt.Errorf("file %s is readonly", f.fileName)
 	}
 
+	length := uint64(len(bytes))
+
 	// Append to file
 	bytesWritten, err := f.file.Write(bytes)
 	if err != nil {
@@ -97,13 +98,13 @@ func (f *TempFile[K]) Tags() map[string]string {
 	return f.tags
 }
 
-// Age returns the duration since this file has been started
+// Age returns the duration since this file was created
 func (f *TempFile[K]) Age() time.Duration {
 	return time.Since(f.createdOn)
 }
 
 // Count returns the number of items stored in this file
-func (f *TempFile[K]) Count() int {
+func (f *TempFile[K]) Count() uint {
 	return f.count
 }
 

upload.go

@@ -11,7 +11,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 )
 
-func (c *client[K]) UploadToS3(ctx context.Context, file *TempFile[K], withMetaFile bool) error {
+func (c *client[K]) UploadFile(ctx context.Context, file *TempFile[K], withMetaFile bool) error {
 	body, err := file.readOnly()
 	if err != nil {
 		return fmt.Errorf("failed to get the readonly file: %w", err)
@@ -49,7 +49,7 @@ func (c *client[K]) UploadToS3(ctx context.Context, file *TempFile[K], withMetaF
 	return nil
 }
 
-func (c *client[K]) DeleteFromS3(ctx context.Context, file *TempFile[K]) error {
+func (c *client[K]) DeleteFile(ctx context.Context, file *TempFile[K]) error {
 	metafileKey := file.MetaFileKey()
 	_, err := c.s3Client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
 		Bucket: &c.s3Bucket,
@@ -66,11 +66,11 @@ func (c *client[K]) DeleteFromS3(ctx context.Context, file *TempFile[K]) error {
 	return nil
 }
 
+// serializeTags converts the tags to url encoded string.
 func serializeTags(tags map[string]string) string {
 	params := url.Values{}
 	for k, v := range tags {
 		params.Add(k, v)
 	}
-	encoded := params.Encode()
-	return encoded
+	return params.Encode()
 }

upload_test.go

@@ -16,7 +16,7 @@ import (
 	"go.uber.org/mock/gomock"
 )
 
-func TestClient_UploadToS3(t *testing.T) {
+func TestClient_UploadFile(t *testing.T) {
 	g := NewGomegaWithT(t)
 	ctx := context.Background()
 
@@ -170,12 +170,12 @@ func TestClient_UploadToS3(t *testing.T) {
 				g.Expect(err).ToNot(HaveOccurred())
 				g.Expect(file.Append(objs.ID, compressed)).ToNot(HaveOccurred())
 			}
-			g.Expect(file.Count()).To(Equal(len(test.objs)))
+			g.Expect(file.Count()).To(Equal(uint(len(test.objs))))
 			g.Expect(file.Age()).To(BeNumerically(">=", uint64(0)))
 			g.Expect(file.Size()).To(BeNumerically(">=", uint64(0)))
 
 			test.configureMocks(g, file, s3Mock)
-			err = c.UploadToS3(ctx, file, test.withMetaFile)
+			err = c.UploadFile(ctx, file, test.withMetaFile)
 			if test.err == nil {
 				g.Expect(err).ToNot(HaveOccurred())
 			} else {
@@ -185,7 +185,7 @@ func TestClient_UploadToS3(t *testing.T) {
 	}
 }
 
-func TestClient_DeleteFromS3(t *testing.T) {
+func TestClient_DeleteFile(t *testing.T) {
 	g := NewGomegaWithT(t)
 
 	ctx := context.Background()
@@ -272,12 +272,12 @@ func TestClient_DeleteFromS3(t *testing.T) {
 				g.Expect(err).ToNot(HaveOccurred())
 				g.Expect(file.Append(obj.ID, compressed)).ToNot(HaveOccurred())
 			}
-			g.Expect(file.Count()).To(Equal(len(test.objs)))
+			g.Expect(file.Count()).To(Equal(uint(len(test.objs))))
 			g.Expect(file.Age()).To(BeNumerically(">=", uint64(0)))
 			g.Expect(file.Size()).To(BeNumerically(">=", uint64(0)))
 
 			test.configureMocks(g, ctrl, file, s3Mock)
-			err = c.DeleteFromS3(ctx, file)
+			err = c.DeleteFile(ctx, file)
 			if test.err == nil {
 				g.Expect(err).ToNot(HaveOccurred())
 			} else {