🐛 normalize paths in cache

kai_analyzer_rpc/pkg/rpc/server.go

@@ -57,7 +57,7 @@ func (s *Server) Accept(pipePath string) {
 	analyzerService, err := service.NewPipeAnalyzer(s.ctx, 10000, 10, 10, pipePath, s.rules, s.sourceDirectory, s.log.WithName("analyzer-service"))
 	if err != nil {
 		s.log.Error(err, "unable to create analyzer service")
-		return
+		panic(err)
 	}
 	s.Server.Handle("analysis_engine.Analyze", analyzerService.Analyze)
 	// s.Server.Handle("analysis_engine.Stop", analyzerService.Stop)

kai_analyzer_rpc/pkg/service/analyzer.go

@@ -33,13 +33,6 @@ var (
 	tracer = otel.Tracer(name)
 )
 
-type cacheValue struct {
-	incident      konveyor.Incident
-	ViolationName string
-	violation     konveyor.Violation
-	ruleset       konveyor.RuleSet
-}
-
 type Analyzer interface {
 	Analyze(client *rpc.Client, args Args, response *Response) error
 	NotifyFileChanges(client *rpc.Client, changes NotifyFileChangesArgs, response *NotifyFileChangesResponse) error
@@ -67,8 +60,7 @@ type analyzer struct {
 
 	discoveryCacheMutex sync.Mutex
 	discoveryCache      []konveyor.RuleSet
-	cache               map[string][]cacheValue
-	cacheMutex          sync.RWMutex
+	cache               IncidentsCache
 
 	contextLines int
 	location     string
@@ -182,8 +174,7 @@ func NewAnalyzer(limitIncidents, limitCodeSnips, contextLines int, location, inc
 		violationRulesets:   violationRulesets,
 		discoveryCache:      []konveyor.RuleSet{},
 		discoveryCacheMutex: sync.Mutex{},
-		cache:               map[string][]cacheValue{},
-		cacheMutex:          sync.RWMutex{},
+		cache:               NewIncidentsCache(log),
 	}, nil
 
 }
@@ -270,7 +261,7 @@ func (a *analyzer) Analyze(client *rpc.Client, args Args, response *Response) er
 	// Then we should return early, with results from the cache
 	if len(scopes) == 0 && !args.ResetCache {
 		a.Logger.Info("no scopes and not resetting cache, return early with results from cache")
-		a.Logger.Info("Current cache len", len(a.cache))
+		a.Logger.Info("Current cache len", a.cache.Len())
 		response.Rulesets = a.createRulesetsFromCache()
 		return nil
 	}
@@ -305,6 +296,8 @@ func (a *analyzer) Analyze(client *rpc.Client, args Args, response *Response) er
 		return rulesets[i].Name < rulesets[j].Name
 	})
 
+	a.Logger.Info("[pg] rulesets", "rulesets", rulesets)
+
 	// This is a full run, set the complete new results
 	if len(args.IncludedPaths) == 0 {
 		a.Logger.V(5).Info("setting cache for full run")
@@ -333,66 +326,39 @@ func (a *analyzer) NotifyFileChanges(client *rpc.Client, args NotifyFileChangesA
 }
 
 func (a *analyzer) setCache(rulesets []konveyor.RuleSet) {
-	a.cacheMutex.Lock()
-	defer a.cacheMutex.Unlock()
-	a.cache = map[string][]cacheValue{}
-
+	a.cache = NewIncidentsCache(a.Logger)
 	a.addRulesetsToCache(rulesets)
 }
 
 func (a *analyzer) updateCache(rulesets []konveyor.RuleSet, includedPaths []string) {
-	a.cacheMutex.Lock()
-	defer a.cacheMutex.Unlock()
 	if includedPaths != nil {
 		a.invalidateCachePerFile(includedPaths)
 	}
 	a.addRulesetsToCache(rulesets)
 }
 
 func (a *analyzer) addRulesetsToCache(rulesets []konveyor.RuleSet) {
-
 	for _, r := range rulesets {
 		for violationName, v := range r.Violations {
 			for _, i := range v.Incidents {
-				a.Logger.V(8).Info("here update cache incident", "incident", i)
-				if l, ok := a.cache[i.URI.Filename()]; ok {
-					l = append(l, cacheValue{
-						incident: i,
-						violation: konveyor.Violation{
-							Description: v.Description,
-							Category:    v.Category,
-							Labels:      v.Labels,
-						},
-						ViolationName: violationName,
-						ruleset: konveyor.RuleSet{
-							Name:        r.Name,
-							Description: r.Description,
-							Tags:        r.Tags,
-							Unmatched:   r.Unmatched,
-							Skipped:     r.Skipped,
-							Errors:      r.Errors,
-						},
-					})
-					a.cache[i.URI.Filename()] = l
-				} else {
-					a.cache[i.URI.Filename()] = []cacheValue{{
-						incident: i,
-						violation: konveyor.Violation{
-							Description: v.Description,
-							Category:    v.Category,
-							Labels:      v.Labels,
-						},
-						ViolationName: violationName,
-						ruleset: konveyor.RuleSet{
-							Name:        r.Name,
-							Description: r.Description,
-							Tags:        r.Tags,
-							Unmatched:   r.Unmatched,
-							Skipped:     r.Skipped,
-							Errors:      r.Errors,
-						},
-					}}
-				}
+				a.Logger.Info("here update cache incident", "incident", i)
+				a.cache.Add(i.URI.Filename(), CacheValue{
+					Incident: i,
+					Violation: konveyor.Violation{
+						Description: v.Description,
+						Category:    v.Category,
+						Labels:      v.Labels,
+					},
+					ViolationName: violationName,
+					Ruleset: konveyor.RuleSet{
+						Name:        r.Name,
+						Description: r.Description,
+						Tags:        r.Tags,
+						Unmatched:   r.Unmatched,
+						Skipped:     r.Skipped,
+						Errors:      r.Errors,
+					},
+				})
 			}
 		}
 	}
@@ -401,34 +367,30 @@ func (a *analyzer) addRulesetsToCache(rulesets []konveyor.RuleSet) {
 func (a *analyzer) invalidateCachePerFile(paths []string) {
 	for _, p := range paths {
 		a.Logger.Info("deleting cache entry for path", "path", p)
-		delete(a.cache, p)
+		a.cache.Delete(p)
 	}
 }
 
 func (a *analyzer) createRulesetsFromCache() []konveyor.RuleSet {
-	a.cacheMutex.RLock()
-	defer a.cacheMutex.RUnlock()
-
 	ruleSetMap := map[string]konveyor.RuleSet{}
 	a.Logger.V(8).Info("cache", "cacheVal", a.cache)
-	for filePath, cacheValue := range a.cache {
+	for filePath, cacheValue := range a.cache.Entries() {
 		for _, v := range cacheValue {
-
-			if ruleset, ok := ruleSetMap[v.ruleset.Name]; ok {
+			if ruleset, ok := ruleSetMap[v.Ruleset.Name]; ok {
 				if vio, ok := ruleset.Violations[v.ViolationName]; ok {
-					vio.Incidents = append(vio.Incidents, v.incident)
+					vio.Incidents = append(vio.Incidents, v.Incident)
 					ruleset.Violations[v.ViolationName] = vio
 					ruleSetMap[ruleset.Name] = ruleset
 				} else {
-					violation := v.violation
-					violation.Incidents = []konveyor.Incident{v.incident}
+					violation := v.Violation
+					violation.Incidents = []konveyor.Incident{v.Incident}
 					ruleset.Violations[v.ViolationName] = violation
 					ruleSetMap[ruleset.Name] = ruleset
 				}
 			} else {
-				violation := v.violation
-				violation.Incidents = []konveyor.Incident{v.incident}
-				ruleset := v.ruleset
+				violation := v.Violation
+				violation.Incidents = []konveyor.Incident{v.Incident}
+				ruleset := v.Ruleset
 				ruleset.Violations = map[string]konveyor.Violation{
 					v.ViolationName: violation,
 				}

kai_analyzer_rpc/pkg/service/cache.go

@@ -0,0 +1,104 @@
+package service
+
+import (
+	"path/filepath"
+	"strings"
+	"sync"
+
+	"github.com/go-logr/logr"
+	"github.com/konveyor/analyzer-lsp/output/v1/konveyor"
+)
+
+type IncidentsCache interface {
+	Get(path string) ([]CacheValue, bool)
+	Add(path string, value CacheValue)
+	Delete(path string)
+	Len() int
+	Entries() map[string][]CacheValue
+}
+
+type CacheValue struct {
+	Incident      konveyor.Incident
+	ViolationName string
+	Violation     konveyor.Violation
+	Ruleset       konveyor.RuleSet
+}
+
+func NewIncidentsCache(logger logr.Logger) IncidentsCache {
+	return &incidentsCache{
+		cache:  map[string][]CacheValue{},
+		logger: logger,
+		mutex:  sync.RWMutex{},
+	}
+}
+
+type incidentsCache struct {
+	cache  map[string][]CacheValue
+	logger logr.Logger
+	mutex  sync.RWMutex
+}
+
+func (i *incidentsCache) Len() int {
+	return len(i.cache)
+}
+
+func (i *incidentsCache) Get(path string) ([]CacheValue, bool) {
+	i.mutex.RLock()
+	defer i.mutex.RUnlock()
+	normalizedPath := normalizePath(path)
+	i.logger.V(8).Info("getting cache entry for path", "path", path, "normalizedPath", normalizedPath)
+	val, ok := i.cache[normalizedPath]
+	return val, ok
+}
+
+func (i *incidentsCache) Add(path string, value CacheValue) {
+	i.mutex.Lock()
+	defer i.mutex.Unlock()
+	normalizedPath := normalizePath(path)
+	i.logger.V(8).Info("adding cache entry for path", "path", path, "normalizedPath", normalizedPath)
+	if _, ok := i.cache[normalizedPath]; !ok {
+		i.cache[normalizedPath] = []CacheValue{}
+	}
+	i.cache[normalizedPath] = append(i.cache[normalizedPath], value)
+}
+
+func (i *incidentsCache) Delete(path string) {
+	i.mutex.Lock()
+	defer i.mutex.Unlock()
+	normalizedPath := normalizePath(path)
+	i.logger.V(8).Info("deleting cache entry for path", "path", path, "normalizedPath", normalizedPath)
+	delete(i.cache, normalizedPath)
+}
+
+func (i *incidentsCache) Keys() []string {
+	i.mutex.RLock()
+	defer i.mutex.RUnlock()
+	keys := make([]string, 0, len(i.cache))
+	for k := range i.cache {
+		keys = append(keys, k)
+	}
+	return keys
+}
+
+func (i *incidentsCache) Entries() map[string][]CacheValue {
+	i.mutex.RLock()
+	defer i.mutex.RUnlock()
+	// make sure we never return a reference to original map or any of its slices
+	clone := make(map[string][]CacheValue, len(i.cache))
+	for k, v := range i.cache {
+		clonedV := make([]CacheValue, len(v))
+		copy(clonedV, v)
+		clone[k] = clonedV
+	}
+	return clone
+}
+
+func normalizePath(path string) string {
+	cleanedPath := filepath.Clean(path)
+	volumeName := filepath.VolumeName(cleanedPath)
+	// make sure all volume names are uppercase
+	if volumeName != "" {
+		cleanedPath = strings.ToUpper(volumeName) + cleanedPath[len(volumeName):]
+	}
+	return filepath.ToSlash(cleanedPath) // prevent forward / backward slashes becoming a problem
+}

kai_analyzer_rpc/pkg/service/pipe_analyzer.go

@@ -79,8 +79,7 @@ func NewPipeAnalyzer(ctx context.Context, limitIncidents, limitCodeSnips, contex
 		violationRulesets:       violationRulesets,
 		discoveryCache:          []konveyor.RuleSet{},
 		discoveryCacheMutex:     sync.Mutex{},
-		cache:                   map[string][]cacheValue{},
-		cacheMutex:              sync.RWMutex{},
+		cache:                   NewIncidentsCache(l),
 		location:                location,
 		contextLines:            contextLines,
 		rules:                   rules,