Fix rename of a folder not correctly identifying inner entities are r…

…enamed (#4649)

* Patch for folder renames

* PR comments

* Proposed further refactors to 4649 (#4654)

---------

Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>

runtime/compilers/rillv1/parser.go

@@ -15,11 +15,18 @@ import (
 )
 
 // Built-in parser limits
-var (
+const (
 	maxFiles    = 10000
 	maxFileSize = 1 << 17 // 128kb
 )
 
+// IgnoredPaths is a list of paths that are ignored by the parser.
+var IgnoredPaths = []string{
+	"/tmp",
+	"/.git",
+	"/node_modules",
+}
+
 // Resource parsed from code files.
 // One file may output multiple resources and multiple files may contribute config to one resource.
 type Resource struct {
@@ -240,12 +247,41 @@ func (p *Parser) Reparse(ctx context.Context, paths []string) (*Diff, error) {
 	return p.reparseExceptRillYAML(ctx, paths)
 }
 
-// IsIgnored returns true if the path will be ignored by Reparse.
-// It's useful for callers to avoid triggering a reparse when they know the path is not relevant.
+// IsIgnored returns true if the path (and any files in nested directories) should be ignored.
 func (p *Parser) IsIgnored(path string) bool {
+	for _, dir := range IgnoredPaths {
+		if path == dir {
+			return true
+		}
+		if strings.HasPrefix(path, dir) && path[len(dir)] == '/' {
+			return true
+		}
+	}
+	return false
+}
+
+// IsSkippable returns true if the path will be skipped by Reparse.
+// It's useful for callers to avoid triggering a reparse when they know the path is not relevant.
+func (p *Parser) IsSkippable(path string) bool {
 	return !pathIsYAML(path) && !pathIsSQL(path) && !pathIsDotEnv(path)
 }
 
+// TrackedPathsInDir returns the paths under the given directory that the parser currently has cached results for.
+func (p *Parser) TrackedPathsInDir(dir string) []string {
+	// Ensure dir has a trailing path separator
+	if dir != "" && dir[len(dir)-1] != '/' {
+		dir += "/"
+	}
+	// Find paths
+	var paths []string
+	for path := range p.resourcesForPath {
+		if strings.HasPrefix(path, dir) {
+			paths = append(paths, path)
+		}
+	}
+	return paths
+}
+
 // reload resets the parser's state and then parses the entire project.
 func (p *Parser) reload(ctx context.Context) error {
 	// Reset state
@@ -265,9 +301,16 @@ func (p *Parser) reload(ctx context.Context) error {
 		return fmt.Errorf("could not list project files: %w", err)
 	}
 
-	paths := make([]string, len(files))
-	for i, file := range files {
-		paths[i] = file.Path
+	// Build paths slice
+	paths := make([]string, 0, len(files))
+	for _, file := range files {
+		// Filter out ignored files
+		// TODO: Incorporate the ignore list directly into the ListRecursive call.
+		if p.IsIgnored(file.Path) {
+			continue
+		}
+
+		paths = append(paths, file.Path)
 	}
 
 	// Parse all files
@@ -326,7 +369,10 @@ func (p *Parser) reparseExceptRillYAML(ctx context.Context, paths []string) (*Di
 		}
 		seenPaths[path] = true
 
-		// Skip files that aren't SQL or YAML or .env file
+		// Skip ignores files and files that aren't SQL or YAML or .env file
+		if p.IsIgnored(path) {
+			continue
+		}
 		isSQL := pathIsSQL(path)
 		isYAML := pathIsYAML(path)
 		isDotEnv := pathIsDotEnv(path)
@@ -335,12 +381,16 @@ func (p *Parser) reparseExceptRillYAML(ctx context.Context, paths []string) (*Di
 		}
 
 		// If a file exists at path, add it to the parse list
-		_, err := p.Repo.Stat(ctx, path)
+		info, err := p.Repo.Stat(ctx, path)
 		if err == nil {
+			if info.IsDir {
+				continue
+			}
 			parsePaths = append(parsePaths, path)
 		} else if !os.IsNotExist(err) {
 			return nil, fmt.Errorf("unexpected file stat error: %w", err)
 		}
+		// NOTE: Continue even if the file has been deleted because it may have associated state we need to clear.
 
 		// Check if path is .env and clear it (so we can re-parse it)
 		if isDotEnv {

runtime/drivers/admin/repo.go

@@ -110,6 +110,7 @@ func (h *Handle) Stat(ctx context.Context, filePath string) (*drivers.RepoObject
 
 	return &drivers.RepoObjectStat{
 		LastUpdated: info.ModTime(),
+		IsDir:       info.IsDir(),
 	}, nil
 }
 

runtime/drivers/file/repo.go

@@ -91,6 +91,7 @@ func (c *connection) Stat(ctx context.Context, filePath string) (*drivers.RepoOb
 
 	return &drivers.RepoObjectStat{
 		LastUpdated: info.ModTime(),
+		IsDir:       info.IsDir(),
 	}, nil
 }
 

runtime/drivers/github/repo.go

@@ -120,6 +120,7 @@ func (c *connection) Stat(ctx context.Context, filePath string) (*drivers.RepoOb
 
 	return &drivers.RepoObjectStat{
 		LastUpdated: info.ModTime(),
+		IsDir:       info.IsDir(),
 	}, nil
 }
 

runtime/drivers/repo.go

@@ -37,6 +37,7 @@ type WatchEvent struct {
 
 type RepoObjectStat struct {
 	LastUpdated time.Time
+	IsDir       bool
 }
 
 var ErrFileAlreadyExists = errors.New("file already exists")

runtime/reconcilers/project_parser.go

@@ -11,6 +11,7 @@ import (
 	"github.com/rilldata/rill/runtime"
 	compilerv1 "github.com/rilldata/rill/runtime/compilers/rillv1"
 	"github.com/rilldata/rill/runtime/drivers"
+	"github.com/rilldata/rill/runtime/pkg/arrayutil"
 	"go.uber.org/zap"
 	"google.golang.org/protobuf/proto"
 )
@@ -173,16 +174,42 @@ func (r *ProjectParserReconciler) Reconcile(ctx context.Context, n *runtimev1.Re
 	err = repo.Watch(ctx, func(events []drivers.WatchEvent) {
 		// Get changed paths that are not directories
 		changedPaths := make([]string, 0, len(events))
+		hasDuplicates := false
 		for _, e := range events {
 			if e.Dir {
 				continue
 			}
 			if parser.IsIgnored(e.Path) {
 				continue
 			}
+			if parser.IsSkippable(e.Path) {
+				// We do not get events for files in deleted/renamed directories.
+				// So we need to manually find paths we're tracking in the directory and add them to changedPaths.
+				//
+				// Note that e.Dir is always false for deletes, so we don't actually know if the path was a directory.
+				// Calling TrackedPathsInDir is safe even if the given path isn't a directory.
+				//
+				// NOTE: This is nested under IsSkippable as an optimization because IsSkippable is true for directories.
+				// This is pretty hacky and should be refactored (probably more fundamentally in the watcher itself).
+				if e.Type == runtimev1.FileEvent_FILE_EVENT_DELETE {
+					ps := parser.TrackedPathsInDir(e.Path)
+					if len(ps) > 0 {
+						changedPaths = append(changedPaths, ps...)
+						hasDuplicates = true
+					}
+					continue
+				}
+
+				continue
+			}
 			changedPaths = append(changedPaths, e.Path)
 		}
 
+		// Small optimization to avoid deduplicating if we know we didn't append to it.
+		if hasDuplicates {
+			changedPaths = arrayutil.Dedupe(changedPaths)
+		}
+
 		if len(changedPaths) == 0 {
 			return
 		}