-
Notifications
You must be signed in to change notification settings - Fork 534
global storage interface refactor #112
Changes from 4 commits
d156d7f
cbad394
dfe21c0
98bba04
6d421ea
b494f4e
7f2f4ff
2e2b5e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,41 +11,46 @@ var ( | |
ErrNotImplemented = errors.New("method not-implemented") | ||
) | ||
|
||
// ObjectStorage generic storage of objects | ||
type ObjectStorage interface { | ||
// ObjectStorer generic storage of objects | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change the comment to include the word persist, as the other storers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as we agreed, persistent is not a property intrinsic to a ObjectStorer |
||
type ObjectStorer interface { | ||
// NewObject returns a new Object, the real type of the object can be a | ||
// custom implementation or the defaul one, MemoryObject | ||
NewObject() Object | ||
// Set save an object into the storage, the object shuld be create with | ||
// the NewObject, method, and file if the type is not supported. | ||
Set(Object) (Hash, error) | ||
// Get an object by hash with the given ObjectType. Implementors should | ||
// return (nil, ErrObjectNotFound) if an object doesn't exist with both the | ||
// given hash and object type. | ||
// SetObject save an object into the storage, the object shuld be create | ||
// with the NewObject, method, and file if the type is not supported. | ||
SetObject(Object) (Hash, error) | ||
// GetObject an object by hash with the given ObjectType. Implementors | ||
// should return (nil, ErrObjectNotFound) if an object doesn't exist with | ||
// both the given hash and object type. | ||
// | ||
// Valid ObjectType values are CommitObject, BlobObject, TagObject, | ||
// TreeObject and AnyObject. | ||
// | ||
// If AnyObject is given, the object must be looked up regardless of its type. | ||
Get(ObjectType, Hash) (Object, error) | ||
// Iter returns a custom ObjectIter over all the object on the storage. | ||
// TreeObject and AnyObject. If AnyObject is given, the object must be | ||
// looked up regardless of its type. | ||
GetObject(ObjectType, Hash) (Object, error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be called Object instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure |
||
// IterObjects returns a custom ObjectIter over all the object on the | ||
// storage. | ||
// | ||
// Valid ObjectType values are CommitObject, BlobObject, TagObject, | ||
Iter(ObjectType) (ObjectIter, error) | ||
IterObjects(ObjectType) (ObjectIter, error) | ||
} | ||
|
||
// ObjectStorerTx is a optional method for ObjectStorer, it enable transaction | ||
// base write and read operations in the storage | ||
type ObjectStorerTx interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are going too far with the naming of this types, I suggest naming this Tx and move it to an store/object package. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. having another package for a interface maybe is too much. Maybe just call the interface |
||
// Begin starts a transaction. | ||
Begin() TxObjectStorage | ||
Begin() TxObjectStorer | ||
} | ||
|
||
// ObjectStorageWrite is a optional method for ObjectStorage, it enable direct | ||
// write of packfile to the storage | ||
type ObjectStorageWrite interface { | ||
// Writer retuns a writer for writing a packfile to the Storage, this method | ||
// is optional, if not implemented the ObjectStorage should return a | ||
// ErrNotImplemented error. | ||
// PackfileWriter is a optional method for ObjectStorer, it enable direct write | ||
// of packfile to the storage | ||
type PackfileWriter interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interface and method with the same name: we need to rethink how to organize or code. |
||
// PackfileWriter retuns a writer for writing a packfile to the Storage, | ||
// this method is optional, if not implemented the ObjectStorer should | ||
// return a ErrNotImplemented error. | ||
// | ||
// If the implementation not implements Writer the objects should be written | ||
// using the Set method. | ||
Writer() (io.WriteCloser, error) | ||
PackfileWriter() (io.WriteCloser, error) | ||
} | ||
|
||
// ObjectIter is a generic closable interface for iterating over objects. | ||
|
@@ -55,20 +60,20 @@ type ObjectIter interface { | |
Close() | ||
} | ||
|
||
// TxObjectStorage is an in-progress storage transaction. | ||
// A transaction must end with a call to Commit or Rollback. | ||
type TxObjectStorage interface { | ||
Set(Object) (Hash, error) | ||
Get(ObjectType, Hash) (Object, error) | ||
// TxObjectStorer is an in-progress storage transaction. A transaction must end | ||
// with a call to Commit or Rollback. | ||
type TxObjectStorer interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have an ObjectStorerTx interface and a TxObjectStorer interface. We must step back, think and organize the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also we must split all this concepts in their own files. |
||
SetObject(Object) (Hash, error) | ||
GetObject(ObjectType, Hash) (Object, error) | ||
Commit() error | ||
Rollback() error | ||
} | ||
|
||
// ReferenceStorage generic storage of references | ||
type ReferenceStorage interface { | ||
Set(*Reference) error | ||
Get(ReferenceName) (*Reference, error) | ||
Iter() (ReferenceIter, error) | ||
// ReferenceStorer generic storage of references | ||
type ReferenceStorer interface { | ||
SetReference(*Reference) error | ||
GetReference(ReferenceName) (*Reference, error) | ||
IterReferences() (ReferenceIter, error) | ||
} | ||
|
||
// ReferenceIter is a generic closable interface for iterating over references | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence makes no sense, it has no verb.
Also, is this comment accurate?, does the memory implementation persist data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done