Merge pull request #22 from ncode/juliano/deepen-discovery-input-surface

Deepen discovery input surface

Author: ncode

cache_test.go

@@ -3,6 +3,7 @@ package facts
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
@@ -111,6 +112,34 @@ func TestWithCache_servesFreshCachedValueOverResolver(t *testing.T) {
 	}
 }
 
+func TestWithCache_selectsQueriedFactsThroughEngineCachePath(t *testing.T) {
+	dir := redirectCacheDir(t)
+	conf := writeTTLConfig(t, "demo")
+	seedCacheFile(t, filepath.Join(dir, "demo"), map[string]any{"demo": map[string]any{"child": "from-cache"}})
+
+	eng, err := New(
+		WithCache(),
+		WithConfigFile(conf),
+		WithFact("demo", func(context.Context) (any, error) {
+			return map[string]any{"child": "fresh"}, nil
+		}),
+		WithFact("other", cachingResolver("fresh")),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	snap, err := eng.Discover(context.Background(), "demo.child")
+	if err != nil {
+		t.Fatalf("Discover() err = %v", err)
+	}
+	if got, err := snap.Value("demo.child"); err != nil || got != "from-cache" {
+		t.Fatalf("Value(demo.child) = %#v, %v, want queried cached value", got, err)
+	}
+	if _, err := snap.Value("other"); !errors.Is(err, ErrFactNotFound) {
+		t.Fatalf("Value(other) err = %v, want unqueried fact omitted from cached Snapshot", err)
+	}
+}
+
 // TestWithoutCache_ignoresExistingCache is the toggle control: the same seeded
 // cache that WithCache would serve is ignored when WithCache is absent, proving
 // the option — not some always-on path — is what enables caching.

engine_contract_test.go

@@ -130,6 +130,26 @@ func TestSnapshotTree_excludesLegacyAliasFacts(t *testing.T) {
 	}
 }
 
+func TestDiscoverQueriesSelectReturnedSnapshotFacts(t *testing.T) {
+	dir := t.TempDir()
+	writeTestFile(t, dir, "site.txt", "site_one=one\nsite_two=two\n")
+
+	eng, err := New(WithExternalDirs(dir))
+	if err != nil {
+		t.Fatal(err)
+	}
+	snap, err := eng.Discover(context.Background(), "site_one")
+	if err != nil {
+		t.Fatalf("Discover() err = %v", err)
+	}
+	if got, err := snap.Value("site_one"); err != nil || got != "one" {
+		t.Fatalf("Value(site_one) = %#v, %v, want selected fact", got, err)
+	}
+	if _, err := snap.Value("site_two"); !errors.Is(err, ErrFactNotFound) {
+		t.Fatalf("Value(site_two) err = %v, want unqueried fact omitted from selected Snapshot", err)
+	}
+}
+
 // Pins the surviving halves of TestAdd_resolvesProgrammaticCustomFactLazily,
 // TestValue_reusesResolvedProgrammaticCustomFact, and
 // TestValue_missingFactDoesNotResolveUnrelatedProgrammaticCustomFacts: each
@@ -155,8 +175,8 @@ func TestWithFact_resolverRunsExactlyOncePerDiscover(t *testing.T) {
 		t.Fatalf("resolver ran %d times after one Discover with an unrelated query, want eager resolution exactly once", calls)
 	}
 	for range 2 {
-		if got, err := first.Value("counted_fact"); err != nil || got != 1 {
-			t.Fatalf("Value(counted_fact) = %#v, %v, want first-run value 1", got, err)
+		if _, err := first.Value("counted_fact"); !errors.Is(err, ErrFactNotFound) {
+			t.Fatalf("Value(counted_fact) err = %v, want unrelated fact omitted from queried Snapshot", err)
 		}
 	}
 	if calls != 1 {
@@ -173,8 +193,8 @@ func TestWithFact_resolverRunsExactlyOncePerDiscover(t *testing.T) {
 	if got, _ := second.Value("counted_fact"); got != 2 {
 		t.Fatalf("second Value(counted_fact) = %#v, want fresh value 2", got)
 	}
-	if got, _ := first.Value("counted_fact"); got != 1 {
-		t.Fatalf("first Value(counted_fact) = %#v after rediscovery, want immutable 1", got)
+	if _, err := first.Value("counted_fact"); !errors.Is(err, ErrFactNotFound) {
+		t.Fatalf("first Value(counted_fact) err = %v after rediscovery, want queried Snapshot unchanged", err)
 	}
 }
 

engine_test.go

@@ -119,6 +119,68 @@ func TestWithConfigFile_loadsConfiguredDirs(t *testing.T) {
 	}
 }
 
+func TestWithConfigFile_recomputesDiscoveryPolicyEachDiscover(t *testing.T) {
+	cacheDir := redirectCacheDir(t)
+	firstDir := t.TempDir()
+	writeTestFile(t, firstDir, "site.txt", "site_location=first\n")
+	writeTestFile(t, firstDir, "blocked.txt", "blocked_probe=blocked\n")
+	secondDir := t.TempDir()
+	writeTestFile(t, secondDir, "site.txt", "site_location=second\nblocked_probe=visible\n")
+	configDir := t.TempDir()
+	configPath := filepath.Join(configDir, "facter.conf")
+	writeTestFile(t, configDir, "facter.conf", `global : {
+  external-dir : [ "`+firstDir+`" ],
+}
+facts : {
+  blocklist : [ "blocked.txt" ],
+}
+`)
+
+	eng, err := New(
+		WithCache(),
+		WithConfigFile(configPath),
+		WithFact("cache_probe", func(context.Context) (any, error) { return "cached", nil }),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	first, err := eng.Discover(context.Background())
+	if err != nil {
+		t.Fatalf("first Discover() err = %v", err)
+	}
+	if got, err := first.Value("site_location"); err != nil || got != "first" {
+		t.Fatalf("first Value(site_location) = %#v, %v, want first", got, err)
+	}
+	if _, err := first.Value("blocked_probe"); !errors.Is(err, ErrFactNotFound) {
+		t.Fatalf("first Value(blocked_probe) err = %v, want ErrFactNotFound", err)
+	}
+	if _, err := os.Stat(filepath.Join(cacheDir, "cache_probe")); !os.IsNotExist(err) {
+		t.Fatalf("cache file after first Discover stat err = %v, want not exist", err)
+	}
+
+	writeTestFile(t, configDir, "facter.conf", `global : {
+  external-dir : [ "`+secondDir+`" ],
+}
+facts : {
+  ttls : [ { "cache_probe" : "30 days" } ],
+}
+`)
+	second, err := eng.Discover(context.Background())
+	if err != nil {
+		t.Fatalf("second Discover() err = %v", err)
+	}
+	if got, err := second.Value("site_location"); err != nil || got != "second" {
+		t.Fatalf("second Value(site_location) = %#v, %v, want second", got, err)
+	}
+	if got, err := second.Value("blocked_probe"); err != nil || got != "visible" {
+		t.Fatalf("second Value(blocked_probe) = %#v, %v, want visible", got, err)
+	}
+	data := readCacheFile(t, filepath.Join(cacheDir, "cache_probe"))
+	if data["cache_probe"] != "cached" {
+		t.Fatalf("cached cache_probe = %#v, want cached", data["cache_probe"])
+	}
+}
+
 func TestWithConfigFile_blocklistSuppressesFacts(t *testing.T) {
 	configPath := writeTestFile(t, t.TempDir(), "facter.conf", "facts : {\n  blocklist : [ \"ssh\" ],\n}\n")
 

internal/app/app.go

@@ -264,8 +264,10 @@ func runQuery(stdout, stderr io.Writer, args []string) error {
 	if configErr != nil {
 		return configErr
 	}
-	if len(externalDirs) == 0 {
-		externalDirs = configOptions.ExternalDirs
+	cliExternalDirs := externalDirs
+	discoveryExternalDirs := externalDirs
+	if len(discoveryExternalDirs) == 0 {
+		discoveryExternalDirs = configOptions.ExternalDirs
 	}
 	if configOptions.NoExternalFacts {
 		*noExternalFacts = true
@@ -276,16 +278,21 @@ func runQuery(stdout, stderr io.Writer, args []string) error {
 	if configOptions.Verbose {
 		*verbose = true
 	}
-	if *noExternalFacts && hasNonEmpty(externalDirs) {
+	if *noExternalFacts && hasNonEmpty(discoveryExternalDirs) {
 		return optionError(stdout, errors.New("--no-external-facts and --external-dir options conflict: please specify only one"))
 	}
 	if !*noExternalFacts {
-		externalDirs = effectiveExternalDirs(externalDirs)
+		discoveryExternalDirs = effectiveExternalDirs(discoveryExternalDirs)
+	}
+	blockedFactsForFastPath := map[string]bool{}
+	var blockedFacts map[string]bool
+	if *noBlock {
+		blockedFacts = map[string]bool{}
 	}
-	blockedFacts := map[string]bool{}
 	if !*noBlock {
-		blockedFacts = engine.BlocklistedFactsForFiltering(configOptions.Blocklist, configOptions.FactGroups)
+		blockedFactsForFastPath = engine.BlocklistedFactsForFiltering(configOptions.Blocklist, configOptions.FactGroups)
 	}
+	mergeDottedFacts := configOptions.ForceDotResolution || *forceDotResolution
 	logLevel := firstNonEmpty(flags.Lookup("log-level").Value.String(), flags.Lookup("l").Value.String(), configOptions.LogLevel)
 	if logLevel != "" && !cli.SupportedLogLevel(logLevel) {
 		return optionError(stdout, fmt.Errorf("unsupported log level %s", logLevel))
@@ -304,17 +311,25 @@ func runQuery(stdout, stderr io.Writer, args []string) error {
 		writeInfo(stderr, "executed with command line: "+strings.Join(args, " "), colorDiagnostics)
 		writeInfo(stderr, "resolving facts", colorDiagnostics)
 	}
-	if canUseVersionQueryFastPath(flags.Args(), externalDirs, blockedFacts, *noExternalFacts, *timing || *timingShort) {
+	if canUseVersionQueryFastPath(flags.Args(), discoveryExternalDirs, blockedFactsForFastPath, *noExternalFacts, *timing || *timingShort) {
 		return writeVersionQuery(stdout, *jsonOutput || *jsonOutputShort, *yamlOutput || *yamlOutputShort, *hoconOutput)
 	}
 	resolutionStart := time.Now()
 
 	eng, err := engine.NewEngine(engine.EngineConfig{
-		CLICompat:       true,
-		ExternalDirs:    externalDirs,
-		NoExternalFacts: *noExternalFacts,
-		BlockedFacts:    blockedFacts,
-		Logger:          logger,
+		CLICompat:              true,
+		SystemDefaults:         true,
+		ConfigFile:             configFile,
+		ConfigLoaded:           true,
+		Config:                 configOptions,
+		ExternalDirs:           cliExternalDirs,
+		UseCache:               !*noCache,
+		NoExternalFacts:        *noExternalFacts,
+		BlockedFacts:           blockedFacts,
+		DefaultExternalDirsSet: true,
+		DefaultExternalDirs:    defaultExternalFactDirs(),
+		IncludeTypedDotted:     mergeDottedFacts,
+		Logger:                 logger,
 	})
 	if err != nil {
 		return err
@@ -324,17 +339,6 @@ func runQuery(stdout, stderr io.Writer, args []string) error {
 		return err
 	}
 	facts := snapshot.Facts()
-	mergeDottedFacts := configOptions.ForceDotResolution || *forceDotResolution
-	projection := engine.NewProjection(facts, mergeDottedFacts)
-	facts = projection.Select(flags.Args())
-	if !*noCache {
-		cache := engine.NewFactCache(engine.DefaultCachePath(), configOptions.TTLs, configOptions.FactGroups, logger)
-		remaining, cached := cache.ResolveFacts(facts)
-		if err := cache.CacheFacts(remaining); err != nil {
-			return err
-		}
-		facts = append(remaining, cached...)
-	}
 	resolutionDuration := time.Since(resolutionStart).Seconds()
 	out, err := engine.BuildFormatter(engine.FormatOptions{
 		JSON:               *jsonOutput || *jsonOutputShort,

internal/app/app_test.go

@@ -18,6 +18,18 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
+func seedAppCacheFile(t *testing.T, path string, data map[string]any) {
+	t.Helper()
+	data["cache_format_version"] = 1
+	raw, err := json.Marshal(data)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(path, raw, 0o600); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestRun_version(t *testing.T) {
 	var stdout, stderr bytes.Buffer
 
@@ -535,6 +547,35 @@ func TestRun_configTTLsUseFreshCachedFact(t *testing.T) {
 	}
 }
 
+func TestRun_noCacheIgnoresFreshCachedFact(t *testing.T) {
+	dir := t.TempDir()
+	factPath := filepath.Join(dir, "site.txt")
+	if err := os.WriteFile(factPath, []byte("site_role=fresh\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	configPath := filepath.Join(t.TempDir(), "facter.conf")
+	conf := "facts : {\n  ttls : [ { \"site_role\" : \"1 hour\" } ],\n}\n"
+	if err := os.WriteFile(configPath, []byte(conf), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	cacheDir := t.TempDir()
+	oldDefaultCachePath := engine.DefaultCachePath
+	engine.DefaultCachePath = func() string { return cacheDir }
+	t.Cleanup(func() { engine.DefaultCachePath = oldDefaultCachePath })
+	seedAppCacheFile(t, filepath.Join(cacheDir, "site_role"), map[string]any{"site_role": "from-cache"})
+	var stdout, stderr bytes.Buffer
+
+	if err := Run(&stdout, &stderr, []string{"--no-cache", "--config", configPath, "--external-dir", dir, "site_role"}); err != nil {
+		t.Fatal(err)
+	}
+	if got, want := stdout.String(), "fresh\n"; got != want {
+		t.Fatalf("stdout = %q, want %q", got, want)
+	}
+	if stderr.Len() != 0 {
+		t.Fatalf("stderr = %q, want empty", stderr.String())
+	}
+}
+
 func TestRun_strictLogsMissingFactErrorWhenQueriedFactIsMissing(t *testing.T) {
 	var stdout, stderr bytes.Buffer