Skip to content

Commit

Permalink
Expose (private) entryBase object in entry interface
Browse files Browse the repository at this point in the history 
This lets us access all the relevant fields without having to define
them as methods in the entry interface.

Signed-off-by: Enis Inan <[email protected]>
  • Loading branch information
@ekinanp
ekinanp committed Jan 13, 2020
1 parent d6090c7 commit 6580cd6
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 147 deletions.
28 changes: 14 additions & 14 deletions plugin/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {
// Including the entry's ID allows plugin authors to use any Cached* methods defined on the
// children after their creation. This is necessary when the child's Cached* methods are used
// to calculate its attributes. Note that the child's ID is set in cachedOp.
entries, err := p.List(context.WithValue(ctx, parentID, p.id()))
entries, err := p.List(context.WithValue(ctx, parentID, p.eb().id))
if err != nil {
return nil, err
}
Expand All @@ -192,16 +192,16 @@ func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {

if duplicateEntry, ok := searchedEntries.mp[cname]; ok {
return nil, DuplicateCNameErr{
ParentID: p.id(),
FirstChildName: duplicateEntry.name(),
FirstChildSlashReplacer: duplicateEntry.slashReplacer(),
SecondChildName: entry.name(),
SecondChildSlashReplacer: entry.slashReplacer(),
ParentID: p.eb().id,
FirstChildName: duplicateEntry.eb().name,
FirstChildSlashReplacer: duplicateEntry.eb().slashReplacer,
SecondChildName: entry.eb().name,
SecondChildSlashReplacer: entry.eb().slashReplacer,
CName: cname,
}
}

if entry.isInaccessible() {
if entry.eb().isInaccessible {
// Skip entries that are expected to be inaccessible.
continue
}
Expand All @@ -210,7 +210,7 @@ func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {

// Ensure ID is set on all entries so that we can use it for caching later in places
// where the context doesn't include the parent's ID.
setChildID(p.id(), entry)
setChildID(p.eb().id, entry)

passAlongWrappedTypes(p, entry)
}
Expand Down Expand Up @@ -254,7 +254,7 @@ func cachedRead(ctx context.Context, e Entry) (entryContent, error) {
panic("attempting to retrieve the content of a non-readable entry")
}
content := newBlockReadableEntryContent(readFunc)
if attr := e.attributes(); attr.HasSize() {
if attr := e.eb().attributes; attr.HasSize() {
content.sz = attr.Size()
}
return content, nil
Expand Down Expand Up @@ -288,7 +288,7 @@ func cachedMetadata(ctx context.Context, e Entry) (JSONObject, error) {
// Common helper for CachedList, CachedOpen and CachedMetadata
func cachedDefaultOp(ctx context.Context, opCode defaultOpCode, entry Entry, op opFunc) (interface{}, error) {
opName := defaultOpCodeToNameMap[opCode]
ttl := entry.getTTLOf(opCode)
ttl := entry.eb().ttl[opCode]

return cachedOp(ctx, opName, entry, ttl, op)
}
Expand All @@ -307,19 +307,19 @@ func cachedOp(ctx context.Context, opName string, entry Entry, ttl time.Duration
return op()
}

if entry.id() == "" {
if entry.eb().id == "" {
// Try to set the ID based on parent ID
if obj := ctx.Value(parentID); obj != nil {
setChildID(obj.(string), entry)
} else {
panic(fmt.Sprintf("Cached op %v on %v had no cache ID and context did not include parent ID", opName, entry.name()))
panic(fmt.Sprintf("Cached op %v on %v had no cache ID and context did not include parent ID", opName, entry.eb().name))
}
}

return cache.GetOrUpdate(opName, entry.id(), ttl, false, op)
return cache.GetOrUpdate(opName, entry.eb().id, ttl, false, op)
}

func setChildID(parentID string, child Entry) {
id := strings.TrimRight(parentID, "/") + "/" + CName(child)
child.setID(id)
child.eb().id = id
}
16 changes: 8 additions & 8 deletions plugin/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,12 @@ func (suite *CacheTestSuite) TestCachedOp() {
opTTL := 5 * time.Second
op := func() (interface{}, error) { return "result", nil }
generateValueMatcher := suite.makeGenerateValueMatcher("result")
suite.cache.On("GetOrUpdate", opName, entry.id(), opTTL, false, mock.MatchedBy(generateValueMatcher)).Return("result", nil).Once()
suite.cache.On("GetOrUpdate", opName, entry.eb().id, opTTL, false, mock.MatchedBy(generateValueMatcher)).Return("result", nil).Once()
v, err := CachedOp(context.Background(), opName, entry, opTTL, op)
if suite.NoError(err) {
suite.Equal("result", v)
}
suite.cache.AssertCalled(suite.T(), "GetOrUpdate", opName, entry.id(), opTTL, false, mock.MatchedBy(generateValueMatcher))
suite.cache.AssertCalled(suite.T(), "GetOrUpdate", opName, entry.eb().id, opTTL, false, mock.MatchedBy(generateValueMatcher))
}

func (suite *CacheTestSuite) TestDuplicateCNameErr() {
Expand Down Expand Up @@ -290,12 +290,12 @@ func (suite *CacheTestSuite) testCachedDefaultOp(
entry.SetTTLOf(op, opTTL)
entry.On(opName, mock.Anything).Return(opValue, nil)
generateValueMatcher := suite.makeGenerateValueMatcher(mungedOpValue)
suite.cache.On("GetOrUpdate", opName, entry.id(), opTTL, false, mock.MatchedBy(generateValueMatcher)).Return(mungedOpValue, nil).Once()
suite.cache.On("GetOrUpdate", opName, entry.eb().id, opTTL, false, mock.MatchedBy(generateValueMatcher)).Return(mungedOpValue, nil).Once()
v, err = cachedDefaultOp(ctx, entry)
if suite.NoError(err) {
suite.Equal(mungedOpValue, v)
}
suite.cache.AssertCalled(suite.T(), "GetOrUpdate", opName, entry.id(), opTTL, false, mock.MatchedBy(generateValueMatcher))
suite.cache.AssertCalled(suite.T(), "GetOrUpdate", opName, entry.eb().id, opTTL, false, mock.MatchedBy(generateValueMatcher))
}

func toMap(children []Entry) map[string]Entry {
Expand Down Expand Up @@ -360,8 +360,8 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
children, err := cachedList(ctx, entry)
if suite.NoError(err) {
if suite.Equal(toMap(mockChildren), children.mp) {
suite.Equal("/foo#child1", children.mp["foo#child1"].id())
suite.Equal("/child2", children.mp["child2"].id())
suite.Equal("/foo#child1", children.mp["foo#child1"].eb().id)
suite.Equal("/child2", children.mp["child2"].eb().id)
}
}

Expand All @@ -373,8 +373,8 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
children, err = cachedList(ctx, entry)
if suite.NoError(err) {
if suite.Equal(toMap(mockChildren), children.mp) {
suite.Equal("/parent/foo#child1", children.mp["foo#child1"].id())
suite.Equal("/parent/child2", children.mp["child2"].id())
suite.Equal("/parent/foo#child1", children.mp["foo#child1"].eb().id)
suite.Equal("/parent/child2", children.mp["child2"].eb().id)
}
}
}
Expand Down
81 changes: 22 additions & 59 deletions plugin/entryBase.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ to do something like
SetMeta(meta)
*/
type EntryBase struct {
entryName string
attr EntryAttributes
slashReplacerCh rune
// washID represents the entry's wash ID. It is set in CachedList.
washID string
ttl [3]time.Duration
wrappedTypesMap SchemaMap
prefetched bool
inaccessible bool
name string
attributes EntryAttributes
slashReplacer rune
id string
ttl [3]time.Duration
wrappedTypes SchemaMap
isPrefetched bool
isInaccessible bool
}

// NewEntry creates a new entry
Expand All @@ -55,8 +54,8 @@ func NewEntry(name string) EntryBase {
}

e := EntryBase{
entryName: name,
slashReplacerCh: '#',
name: name,
slashReplacer: '#',
}
for op := range e.ttl {
e.SetTTLOf(defaultOpCode(op), 15*time.Second)
Expand All @@ -72,44 +71,12 @@ func (e *EntryBase) Metadata(ctx context.Context) (JSONObject, error) {
// Disable Metadata's caching in case the plugin author forgot to do this
e.DisableCachingFor(MetadataOp)

attr := e.attributes()
attr := e.attributes
return attr.Meta(), nil
}

func (e *EntryBase) name() string {
return e.entryName
}

func (e *EntryBase) attributes() EntryAttributes {
return e.attr
}

func (e *EntryBase) slashReplacer() rune {
return e.slashReplacerCh
}

func (e *EntryBase) id() string {
return e.washID
}

func (e *EntryBase) setID(id string) {
e.washID = id
}

func (e *EntryBase) getTTLOf(op defaultOpCode) time.Duration {
return e.ttl[op]
}

func (e *EntryBase) wrappedTypes() SchemaMap {
return e.wrappedTypesMap
}

func (e *EntryBase) setWrappedTypes(wrappedTypes SchemaMap) {
e.wrappedTypesMap = wrappedTypes
}

func (e *EntryBase) isPrefetched() bool {
return e.prefetched
func (e *EntryBase) eb() *EntryBase {
return e
}

// OTHER METHODS USED TO FACILITATE PLUGIN DEVELOPMENT
Expand All @@ -119,37 +86,33 @@ func (e *EntryBase) isPrefetched() bool {
// plugin.NewEntry. You should use e.Name() when making
// the appropriate API calls within your plugin.
func (e *EntryBase) Name() string {
return e.name()
return e.name
}

// String returns a unique identifier for the entry suitable for logging and error messages.
func (e *EntryBase) String() string {
return e.id()
return e.id
}

// Attributes returns a pointer to the entry's attributes. Use it
// to individually set the entry's attributes
func (e *EntryBase) Attributes() *EntryAttributes {
return &e.attr
return &e.attributes
}

// SetAttributes sets the entry's attributes. Use it to set
// the entry's attributes in a single operation, which is useful
// when you've already pre-computed them.
func (e *EntryBase) SetAttributes(attr EntryAttributes) *EntryBase {
e.attr = attr
e.attributes = attr
return e
}

func (e *EntryBase) isInaccessible() bool {
return e.inaccessible
}

// MarkInaccessible sets the inaccessible attribute and logs a message about why the entry is
// inaccessible.
func (e *EntryBase) MarkInaccessible(ctx context.Context, err error) {
activity.Record(ctx, "Omitting %v: %v", e.id(), err)
e.inaccessible = true
activity.Record(ctx, "Omitting %v: %v", e.id, err)
e.isInaccessible = true
}

// Prefetched marks the entry as a prefetched entry. A prefetched entry
Expand All @@ -158,7 +121,7 @@ func (e *EntryBase) MarkInaccessible(ctx context.Context, err error) {
// files are good examples of prefetched entries (see the volume
// package for more details).
func (e *EntryBase) Prefetched() *EntryBase {
e.prefetched = true
e.isPrefetched = true
return e
}

Expand All @@ -172,7 +135,7 @@ func (e *EntryBase) SetSlashReplacer(char rune) *EntryBase {
panic("e.SetSlashReplacer called with '/'")
}

e.slashReplacerCh = char
e.slashReplacer = char
return e
}

Expand Down Expand Up @@ -204,7 +167,7 @@ func (e *EntryBase) SetTestID(id string) {
panic("SetTestID can be only be called by the tests")
}

e.setID(id)
e.id = id
}

func notRunningTests() bool {
Expand Down
9 changes: 3 additions & 6 deletions plugin/entryBase_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type EntryBaseTestSuite struct {
}

func (suite *EntryBaseTestSuite) assertOpTTL(e EntryBase, op defaultOpCode, opName string, expectedTTL time.Duration) {
actualTTL := e.getTTLOf(op)
actualTTL := e.ttl[op]
suite.Equal(
expectedTTL,
actualTTL,
Expand All @@ -35,16 +35,13 @@ func (suite *EntryBaseTestSuite) TestNewEntry() {
e := NewEntry("foo")

e.SetAttributes(initialAttr)
suite.Equal(initialAttr, e.attr)
suite.Equal(initialAttr, e.attributes)

suite.Equal("foo", e.Name())
suite.assertOpTTL(e, ListOp, "List", 15*time.Second)
suite.assertOpTTL(e, ReadOp, "Read", 15*time.Second)
suite.assertOpTTL(e, MetadataOp, "Metadata", 15*time.Second)

e.setID("/foo")
suite.Equal("/foo", e.id())

e.SetTTLOf(ListOp, 40*time.Second)
suite.assertOpTTL(e, ListOp, "List", 40*time.Second)

Expand Down Expand Up @@ -75,7 +72,7 @@ func (suite *EntryBaseTestSuite) TestSetSlashReplacer() {
)

e.SetSlashReplacer(':')
suite.Equal(e.slashReplacer(), ':')
suite.Equal(e.slashReplacer, ':')
}

func TestEntryBase(t *testing.T) {
Expand Down
12 changes: 6 additions & 6 deletions plugin/entrySchema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func schema(e Entry) (*EntrySchema, error) {
for _, root := range t.pluginRoots {
childSchema, err := Schema(root)
if err != nil {
return nil, fmt.Errorf("failed to retrieve the %v plugin's schema: %v", root.name(), err)
return nil, fmt.Errorf("failed to retrieve the %v plugin's schema: %v", root.eb().name, err)
}
if childSchema == nil {
// The plugin doesn't have a schema, which means it's an external plugin.
Expand Down Expand Up @@ -251,7 +251,7 @@ func (s *EntrySchema) fill(graph *linkedhashmap.Map) {
}
// The ID here is meaningless. We only set it so that TypeID can get the
// plugin name
child.entry.setID(s.entry.id())
child.entry.eb().id = s.entry.eb().id
childTypeID := TypeID(child.entry)
s.entrySchema.Children = append(s.Children, childTypeID)
if _, ok := graph.Get(childTypeID); ok {
Expand All @@ -274,15 +274,15 @@ func passAlongWrappedTypes(p Parent, child Entry) {
if root, ok := child.(HasWrappedTypes); ok {
wrappedTypes = root.WrappedTypes()
} else {
wrappedTypes = p.wrappedTypes()
wrappedTypes = p.eb().wrappedTypes
}
child.setWrappedTypes(wrappedTypes)
child.eb().wrappedTypes = wrappedTypes
}

// Helper that wraps the common code shared by the SetMeta*Schema methods
func (s *EntrySchema) schemaOf(obj interface{}) (*JSONSchema, error) {
typeMappings := make(map[reflect.Type]*jsonschema.Type)
for t, s := range s.entry.wrappedTypes() {
for t, s := range s.entry.eb().wrappedTypes {
typeMappings[reflect.TypeOf(t)] = s.Type
}
r := jsonschema.Reflector{
Expand Down Expand Up @@ -314,7 +314,7 @@ func pluginName(e Entry) string {
// via something like "Schema(registry) => TypeID(Root)", where
// CachedList(registry) was not yet called. This can happen if, for
// example, the user starts the Wash shell and runs `stree`.
trimmedID := strings.Trim(e.id(), "/")
trimmedID := strings.Trim(e.eb().id, "/")
if trimmedID == "" {
switch e.(type) {
case Root:
Expand Down
Loading

0 comments on commit 6580cd6

Please sign in to comment.