docs: describe cache interfaces in the comments themselves

Drop ticket and process notes so type and test comments state the adapter
split, who owns each capability, and what each test asserts. Build the
sensitive-path index test with sensitive.PathIndex after the rebase.

Signed-off-by: Markus Vahlenkamp <markus.vahlenkamp@nokia.com>

Author: steiler

pkg/cache/cache.go

@@ -21,39 +21,32 @@ import (
 	"github.com/sdcio/sdc-protos/tree_persist"
 )
 
-// IntentReader reads real Intents. Every backend implements this directly —
-// it's the one capability every Cache.Type genuinely has data for.
+// IntentReader reads real Intents. Every cache backend implements it.
 type IntentReader interface {
 	InstanceIntentsList(ctx context.Context, cacheInstanceName string) ([]string, error)
 	InstanceIntentGet(ctx context.Context, cacheName string, intentName string) (importer.IntentAdapter, error)
 	InstanceIntentExists(ctx context.Context, cacheName string, intentName string) (bool, error)
 	InstanceIntentGetAll(ctx context.Context, cacheName string, excludeIntentNames []string, intentChan chan<- importer.IntentAdapter, errChan chan<- error)
 }
 
-// IntentWriter persists real Intents. Backends that don't own real-Intent
-// writes (e.g. config-server, where kube-api owns them) compose a no-op
-// implementation instead of implementing this directly, so the type system
-// makes that lack of ownership visible.
+// IntentWriter persists real Intents. A backend that does not own those
+// writes (config-server: kube-api writes them) composes noopIntentWriter.
 type IntentWriter interface {
 	InstanceIntentModify(ctx context.Context, cacheName string, intent *tree_persist.Intent) error
 	InstanceIntentDelete(ctx context.Context, cacheName string, intentName string, IgnoreNonExisting bool) error
 }
 
-// RunningStore reads/writes "running": split out of the generic Intent
-// surface, independent of Cache.Type. LocalCache keeps its existing
-// disk-backed store for it (unchanged on-disk behavior); other backends may
-// back it however they see fit (e.g. purely in-memory), since it's never
-// config-server's data to begin with.
+// RunningStore reads and writes "running", the synced device-state intent.
+// It is independent of Cache.Type: LocalCache keeps it on disk; other
+// backends may keep it in memory.
 type RunningStore interface {
 	InstanceRunningGet(ctx context.Context, cacheName string) (importer.ImportConfigAdapter, error)
 	InstanceRunningModify(ctx context.Context, cacheName string, intent *tree_persist.Intent) error
 }
 
-// InstanceLifecycle creates/destroys/enumerates cache instances. Under
-// ConfigServerCache this only ever governs its in-memory running store — it
-// has no relationship to whether real-Intent data exists for a target under
-// that backend, unlike LocalCache where it governs the one shared disk store
-// underlying everything.
+// InstanceLifecycle creates, closes, and enumerates cache instances.
+// On a config-server backend this covers only the in-memory running store,
+// not whether real Intent data exists for a target.
 type InstanceLifecycle interface {
 	InstanceCreate(ctx context.Context, cacheInstanceName string) error
 	InstanceDelete(ctx context.Context, cacheInstanceName string) error
@@ -62,6 +55,8 @@ type InstanceLifecycle interface {
 	InstancesList(ctx context.Context) []string
 }
 
+// Client is a full cache backend: intent read and write, running, and
+// instance lifecycle.
 type Client interface {
 	IntentReader
 	IntentWriter

pkg/cache/cacheClientBound.go

@@ -21,34 +21,35 @@ import (
 	"github.com/sdcio/sdc-protos/tree_persist"
 )
 
-// BoundIntentReader is the bound-call equivalent of IntentReader.
+// BoundIntentReader is IntentReader scoped to one cache instance.
 type BoundIntentReader interface {
 	IntentsList(ctx context.Context) ([]string, error)
 	IntentGet(ctx context.Context, intentName string) (importer.IntentAdapter, error)
 	IntentExists(ctx context.Context, intentName string) (bool, error)
 	IntentGetAll(ctx context.Context, excludeIntentNames []string, intentChan chan<- importer.IntentAdapter, errChan chan<- error)
 }
 
-// BoundIntentWriter is the bound-call equivalent of IntentWriter.
+// BoundIntentWriter is IntentWriter scoped to one cache instance.
 type BoundIntentWriter interface {
 	IntentModify(ctx context.Context, intent *tree_persist.Intent) error
 	IntentDelete(ctx context.Context, intentName string, IgnoreNonExisting bool) error
 }
 
-// BoundRunningStore is the bound-call equivalent of RunningStore.
+// BoundRunningStore is RunningStore scoped to one cache instance.
 type BoundRunningStore interface {
 	RunningGet(ctx context.Context) (importer.ImportConfigAdapter, error)
 	RunningModify(ctx context.Context, intent *tree_persist.Intent) error
 }
 
-// BoundInstanceLifecycle is the bound-call equivalent of InstanceLifecycle.
+// BoundInstanceLifecycle is InstanceLifecycle scoped to one cache instance.
 type BoundInstanceLifecycle interface {
 	InstanceCreate(ctx context.Context) error
 	InstanceDelete(ctx context.Context) error
 	InstanceClose(ctx context.Context) error
 	InstanceExists(ctx context.Context) bool
 }
 
+// CacheClientBound is Client with the cache instance already selected.
 type CacheClientBound interface {
 	BoundIntentReader
 	BoundIntentWriter

pkg/cache/cache_test.go

@@ -14,10 +14,7 @@
 
 package cache
 
-// Compile-time assertions that the capability interfaces compose back into
-// Client with no change to its method surface, and that *LocalCache keeps
-// satisfying Client (and, by extension, all four capabilities) with zero
-// code changes.
+// *LocalCache satisfies Client and each capability.
 var (
 	_ Client = (*LocalCache)(nil)
 
@@ -27,9 +24,8 @@ var (
 	_ InstanceLifecycle = (*LocalCache)(nil)
 )
 
-// composedClient documents that Client is exactly the sum of the four
-// capabilities — nothing more, nothing less — from the perspective of any
-// caller assembling one from the parts.
+// composedClient is Client as the four capabilities, so a value assembled
+// from those parts still satisfies Client.
 type composedClient interface {
 	IntentReader
 	IntentWriter
@@ -40,9 +36,7 @@ type composedClient interface {
 var _ Client = composedClient(nil)
 var _ composedClient = Client(nil)
 
-// Same compile-time shape check for the bound side: CacheClientBoundImpl
-// already implements every method needed, so it should keep satisfying
-// CacheClientBound and each bound capability with zero code changes.
+// *CacheClientBoundImpl satisfies CacheClientBound and each bound capability.
 var (
 	_ CacheClientBound = (*CacheClientBoundImpl)(nil)
 

pkg/cache/local.go

@@ -49,11 +49,8 @@ type LocalCache struct {
 	*cache.Cache
 }
 
-// decodeIntent unmarshals the disk-backed store's raw bytes into a
-// *tree_persist.Intent and wraps it as an importer.IntentAdapter — the
-// shared decode step every InstanceIntentGet/InstanceIntentGetAll/
-// InstanceRunningGet call site needs. Running callers narrow to
-// ImportConfigAdapter at the RunningStore boundary.
+// decodeIntent unmarshals stored bytes into an IntentAdapter.
+// Running reads use the same bytes and narrow the result to ImportConfigAdapter.
 func (l *LocalCache) decodeIntent(b []byte) (importer.IntentAdapter, error) {
 	result := &tree_persist.Intent{}
 	if err := proto.Unmarshal(b, result); err != nil {
@@ -114,10 +111,7 @@ func (l *LocalCache) InstanceIntentModify(ctx context.Context, cacheName string,
 	return l.Cache.InstanceIntentModify(ctx, cacheName, intent.GetIntentName(), b)
 }
 
-// InstanceRunningGet and InstanceRunningModify are thin passthroughs to the same
-// disk-backed mechanism InstanceIntentGet/InstanceIntentModify already use, so the
-// local backend's on-disk behavior for "running" is unchanged by the split — it
-// simply becomes unreachable via the intent-name-keyed InstanceIntent* surface.
+// InstanceRunningGet loads "running" from the disk store under the running intent name.
 func (l *LocalCache) InstanceRunningGet(ctx context.Context, cacheName string) (importer.ImportConfigAdapter, error) {
 	b, err := l.Cache.InstanceIntentGet(ctx, cacheName, consts.RunningIntentName)
 	if err != nil {
@@ -126,6 +120,7 @@ func (l *LocalCache) InstanceRunningGet(ctx context.Context, cacheName string) (
 	return l.decodeIntent(b)
 }
 
+// InstanceRunningModify stores "running" on disk under the running intent name.
 func (l *LocalCache) InstanceRunningModify(ctx context.Context, cacheName string, intent *tree_persist.Intent) error {
 	b, err := proto.Marshal(intent)
 	if err != nil {

pkg/cache/local_test.go

@@ -24,9 +24,8 @@ func newTestLocalCache(t *testing.T) *LocalCache {
 	return lc
 }
 
-// TestLocalCache_InstanceIntentGetRoundTrip verifies InstanceIntentModify/InstanceIntentGet
-// round-trip a *tree_persist.Intent through the disk-backed store — a regression check
-// pinning current decode behavior before extracting it into a shared helper.
+// TestLocalCache_InstanceIntentGetRoundTrip: InstanceIntentModify then
+// InstanceIntentGet returns the stored intent name and priority.
 func TestLocalCache_InstanceIntentGetRoundTrip(t *testing.T) {
 	ctx := context.Background()
 	lc := newTestLocalCache(t)
@@ -61,12 +60,8 @@ func TestLocalCache_InstanceIntentGetRoundTrip(t *testing.T) {
 	}
 }
 
-// TestLocalCache_RunningRoundTrip verifies InstanceRunningModify/InstanceRunningGet
-// are thin passthroughs to the same disk-backed store InstanceIntentGet/Modify use,
-// so the local backend's on-disk behavior for "running" is unchanged by the split.
-// InstanceRunningGet returns an importer.ImportConfigAdapter (Intent reads return
-// IntentAdapter) rather than the raw *tree_persist.Intent, so the assertions go
-// through its accessors.
+// TestLocalCache_RunningRoundTrip: InstanceRunningModify then InstanceRunningGet
+// returns the running intent through ImportConfigAdapter.
 func TestLocalCache_RunningRoundTrip(t *testing.T) {
 	ctx := context.Background()
 	lc := newTestLocalCache(t)
@@ -101,10 +96,8 @@ func TestLocalCache_RunningRoundTrip(t *testing.T) {
 	}
 }
 
-// TestLocalCache_RunningExcludedFromIntentGetAll verifies that once callers ask
-// for real intents via InstanceIntentGetAll with running explicitly excluded (the
-// pattern every real call site now uses), "running" never shows up, even though
-// LocalCache still persists it in the same underlying disk-backed store.
+// TestLocalCache_RunningExcludedFromIntentGetAll: excluding the running intent
+// name keeps it out of InstanceIntentGetAll, even though it shares the disk store.
 func TestLocalCache_RunningExcludedFromIntentGetAll(t *testing.T) {
 	ctx := context.Background()
 	lc := newTestLocalCache(t)

pkg/cache/noop_intent_writer.go

@@ -20,14 +20,8 @@ import (
 	"github.com/sdcio/sdc-protos/tree_persist"
 )
 
-// noopIntentWriter is a generic IntentWriter for backends that don't own
-// real-Intent writes (e.g. config-server, where kube-api is the sole
-// writer). It is composed into a full Client at the one legitimate
-// type-switch point, Server.createCacheClient, rather than living as
-// no-op-shaped methods inside the read-only backend's own type — that keeps
-// the backend's type signature honest about what it can actually do, and
-// gives any future read-only backend a name to compose instead of writing
-// its own no-op boilerplate.
+// noopIntentWriter is an IntentWriter whose modify and delete always succeed.
+// Read-only backends compose it when another system owns real-Intent writes.
 type noopIntentWriter struct{}
 
 func (noopIntentWriter) InstanceIntentModify(ctx context.Context, cacheName string, intent *tree_persist.Intent) error {

pkg/cache/noop_intent_writer_test.go

@@ -23,10 +23,7 @@ import (
 
 var _ IntentWriter = noopIntentWriter{}
 
-// TestNoopIntentWriter_AlwaysNil verifies InstanceIntentModify/
-// InstanceIntentDelete return nil unconditionally, regardless of input —
-// there is no backend to fail against, so there is no error path to
-// exercise.
+// TestNoopIntentWriter_AlwaysNil: modify and delete succeed for any input.
 func TestNoopIntentWriter_AlwaysNil(t *testing.T) {
 	w := noopIntentWriter{}
 	ctx := context.Background()

pkg/config/datastore.go

@@ -279,7 +279,7 @@ func (c *CacheConfig) validateSetDefaults() error {
 			c.Dir = defaultCacheDir
 		}
 	case cacheTypeConfigServer:
-		// no connection settings yet; added by a follow-up ticket.
+		// Known type; CacheConfig has no defaults for it.
 	default:
 		return fmt.Errorf("unknown cache type: %q", c.Type)
 	}

pkg/datastore/datastore_rpc_test.go

@@ -7,17 +7,15 @@ import (
 	"github.com/sdcio/data-server/mocks/mockcacheclient"
 	"github.com/sdcio/data-server/pkg/tree/importer"
 	treeproto "github.com/sdcio/data-server/pkg/tree/importer/proto"
-	treetypes "github.com/sdcio/data-server/pkg/tree/types"
+	"github.com/sdcio/data-server/pkg/tree/sensitive"
 	sdcpb "github.com/sdcio/sdc-protos/sdcpb"
 	"github.com/sdcio/sdc-protos/tree_persist"
 	"go.uber.org/mock/gomock"
 )
 
-// TestPopulateSensitivePathIndex_NarrowIntentReader verifies
-// populateSensitivePathIndex loads sensitive_paths from every intent that
-// declares them, skipping intents with none. It is built against a
-// MockBoundIntentReader — not the full MockCacheClientBound — since
-// populateSensitivePathIndex only ever reads real Intents via forEachIntent.
+// TestPopulateSensitivePathIndex_NarrowIntentReader loads sensitive paths from
+// intents that declare them and skips intents with none. The reader is a
+// BoundIntentReader; the index only needs intent reads.
 func TestPopulateSensitivePathIndex_NarrowIntentReader(t *testing.T) {
 	ctx := context.Background()
 	ctrl := gomock.NewController(t)
@@ -39,7 +37,7 @@ func TestPopulateSensitivePathIndex_NarrowIntentReader(t *testing.T) {
 			close(errChan)
 		})
 
-	s := treetypes.NewSensitivePathIndex()
+	s := sensitive.NewPathIndex()
 	if err := populateSensitivePathIndex(ctx, s, reader); err != nil {
 		t.Fatalf("populateSensitivePathIndex() error = %v", err)
 	}

pkg/datastore/sensitive_path_union_test.go

@@ -236,7 +236,7 @@ func TestGetIntent_CrossIntentPathsRedact(t *testing.T) {
 			}
 
 			resp, err := ds.GetIntent(ctx, GetIntentOpts{
-				IntentName: "data-intent",
+				IntentName:                "data-intent",
 				NorthboundSensitivityOpts: NorthboundSensitivityOpts{IncludeSensitive: tt.exposeSensitive},
 			})
 			if err != nil {
@@ -320,7 +320,7 @@ func TestGetIntent_OwnSensitivePaths_Redacted(t *testing.T) {
 			}
 
 			resp, err := ds.GetIntent(ctx, GetIntentOpts{
-				IntentName: "data-intent",
+				IntentName:                "data-intent",
 				NorthboundSensitivityOpts: NorthboundSensitivityOpts{IncludeSensitive: tt.exposeSensitive},
 			})
 			if err != nil {