Add "options" to properties service calls (#4459)

Instead of passing the db store and other optional values as
explicit paramters, this introduces options to the properties service.
This allows us to start building queries in a more dynamic way.

The querier is passed always now as an option.

Disregarding stale data was introduced as an option, and it was also
used in the places it's needed, that is, in GET calls.

Signed-off-by: Juan Antonio Osorio <[email protected]>

Author: JAORMX

internal/controlplane/handlers_evalstatus.go

@@ -597,7 +597,8 @@ func (s *Server) buildRuleEvaluationStatusFromDBEvaluation(
 			Msg("error converting severity will use defaults")
 	}
 
-	err = s.props.RetrieveAllPropertiesForEntity(ctx, efp, s.providerManager, s.store)
+	err = s.props.RetrieveAllPropertiesForEntity(ctx, efp, s.providerManager,
+		propSvc.ReadBuilder().WithStoreOrTransaction(s.store).TolerateStaleData())
 	if err != nil {
 		return nil, fmt.Errorf("error fetching properties for entity: %s: %w", efp.Entity.ID.String(), err)
 	}

internal/controlplane/handlers_githubwebhooks.go

@@ -42,6 +42,7 @@ import (
 	"github.com/stacklok/minder/internal/engine/entities"
 	"github.com/stacklok/minder/internal/entities/models"
 	"github.com/stacklok/minder/internal/entities/properties"
+	"github.com/stacklok/minder/internal/entities/properties/service"
 	"github.com/stacklok/minder/internal/events"
 	"github.com/stacklok/minder/internal/projects/features"
 	"github.com/stacklok/minder/internal/providers/github/clients"
@@ -730,7 +731,9 @@ func (s *Server) processPackageEvent(
 		refreshedPkgProperties, err = s.props.RetrieveAllProperties(
 			ctx, provider,
 			repoEnt.Entity.ProjectID, repoEnt.Entity.ProviderID,
-			pkgLookupProps, pb.Entity_ENTITY_ARTIFACTS, tx)
+			pkgLookupProps, pb.Entity_ENTITY_ARTIFACTS,
+			service.ReadBuilder().WithStoreOrTransaction(tx))
+
 		if err != nil {
 			return nil, fmt.Errorf("error retrieving properties: %w", err)
 		}
@@ -776,7 +779,8 @@ func (s *Server) processPackageEvent(
 		refreshedPkgProperties, err = s.props.RetrieveAllProperties(
 			ctx, provider,
 			ent.ProjectID, ent.ProviderID,
-			refreshedPkgProperties, pb.Entity_ENTITY_ARTIFACTS, tx)
+			refreshedPkgProperties, pb.Entity_ENTITY_ARTIFACTS,
+			service.ReadBuilder().WithStoreOrTransaction(tx))
 		if err != nil {
 			return nil, fmt.Errorf("error retrieving properties: %w", err)
 		}
@@ -1613,7 +1617,9 @@ func (s *Server) updatePullRequestInfoFromProvider(
 
 	prProps, err := s.props.RetrieveAllProperties(ctx, provider,
 		repoEnt.Entity.ProjectID, repoEnt.Entity.ProviderID,
-		lookupProperties, pb.Entity_ENTITY_PULL_REQUESTS, qtx)
+		lookupProperties, pb.Entity_ENTITY_PULL_REQUESTS,
+		service.ReadBuilder().WithStoreOrTransaction(qtx))
+
 	if err != nil {
 		return nil, fmt.Errorf("error retrieving properties: %w", err)
 	}

internal/controlplane/handlers_profile.go

@@ -461,7 +461,8 @@ func (s *Server) getRuleEvalStatus(
 		return nil, fmt.Errorf("error fetching entity for properties: %w", err)
 	}
 
-	err = s.props.RetrieveAllPropertiesForEntity(ctx, efp, s.providerManager, s.store)
+	err = s.props.RetrieveAllPropertiesForEntity(ctx, efp, s.providerManager,
+		propSvc.ReadBuilder().WithStoreOrTransaction(s.store).TolerateStaleData())
 	if err != nil {
 		return nil, fmt.Errorf("error fetching properties for entity: %w", err)
 	}

internal/engine/executor.go

@@ -235,7 +235,8 @@ func (e *executor) profileEvalStatus(
 	}
 
 	// get the entity with properties by the entity UUID
-	ewp, err := e.propService.EntityWithProperties(ctx, entityID, e.querier)
+	ewp, err := e.propService.EntityWithProperties(ctx, entityID,
+		service.CallBuilder().WithStoreOrTransaction(e.querier))
 	if err != nil {
 		return fmt.Errorf("error getting entity with properties: %w", err)
 	}

internal/entities/properties/service/helpers.go

@@ -36,8 +36,9 @@ import (
 func (ps *propertiesService) retrieveAllPropertiesForEntity(
 	ctx context.Context, provider provifv1.Provider, entID uuid.UUID,
 	lookupProperties *properties.Properties, entType minderv1.Entity,
-	qtx db.ExtendQuerier, l zerolog.Logger,
+	opts *ReadOptions, l zerolog.Logger,
 ) (*properties.Properties, error) {
+	qtx := ps.getStoreOrTransaction(opts)
 
 	var dbProps []db.Property
 	if entID != uuid.Nil {
@@ -61,7 +62,7 @@ func (ps *propertiesService) retrieveAllPropertiesForEntity(
 		if err != nil {
 			return nil, fmt.Errorf("failed to convert properties: %w", err)
 		}
-		if ps.areDatabasePropertiesValid(dbProps) {
+		if ps.areDatabasePropertiesValid(dbProps, opts) {
 			l.Info().Msg("properties are valid, skipping provider fetch")
 			return modelProps, nil
 		}
@@ -84,7 +85,7 @@ func (ps *propertiesService) retrieveAllPropertiesForEntity(
 	}
 
 	// save updated properties to db, thus making sure that the updatedAt are bumped
-	err = ps.ReplaceAllProperties(ctx, entID, refreshedProps, qtx)
+	err = ps.ReplaceAllProperties(ctx, entID, refreshedProps, opts.getPropertiesServiceCallOptions())
 	if err != nil {
 		return nil, fmt.Errorf("failed to update properties: %w", err)
 	}
@@ -93,15 +94,15 @@ func (ps *propertiesService) retrieveAllPropertiesForEntity(
 	return refreshedProps, nil
 }
 
-func (ps *propertiesService) getEntityIdByProperties(
+func getEntityIdByProperties(
 	ctx context.Context, projectId uuid.UUID,
 	providerID uuid.UUID,
 	props *properties.Properties, entType minderv1.Entity,
 	qtx db.ExtendQuerier,
 ) (uuid.UUID, error) {
 	upstreamID := props.GetProperty(properties.PropertyUpstreamID)
 	if upstreamID != nil {
-		ent, getErr := ps.getEntityIdByUpstreamID(ctx, projectId, providerID, upstreamID.GetString(), entType, qtx)
+		ent, getErr := getEntityIdByUpstreamID(ctx, projectId, providerID, upstreamID.GetString(), entType, qtx)
 		if getErr == nil {
 			return ent, nil
 		} else if !errors.Is(getErr, ErrEntityNotFound) {
@@ -114,14 +115,14 @@ func (ps *propertiesService) getEntityIdByProperties(
 	// Fall back to name if no upstream ID is provided
 	name := props.GetProperty(properties.PropertyName)
 	if name != nil {
-		return ps.getEntityIdByName(ctx, projectId, providerID, name.GetString(), entType, qtx)
+		return getEntityIdByName(ctx, projectId, providerID, name.GetString(), entType, qtx)
 	}
 
 	// returning nil ID and nil error would make us just go to the provider. Slow, but we'd continue.
 	return uuid.Nil, nil
 }
 
-func (_ *propertiesService) getEntityIdByName(
+func getEntityIdByName(
 	ctx context.Context, projectId uuid.UUID,
 	providerID uuid.UUID,
 	name string, entType minderv1.Entity,
@@ -142,7 +143,7 @@ func (_ *propertiesService) getEntityIdByName(
 	return ent.ID, nil
 }
 
-func (_ *propertiesService) getEntityIdByUpstreamID(
+func getEntityIdByUpstreamID(
 	ctx context.Context, projectId uuid.UUID,
 	providerID uuid.UUID,
 	upstreamID string, entType minderv1.Entity,
@@ -173,20 +174,21 @@ func (_ *propertiesService) getEntityIdByUpstreamID(
 	return uuid.Nil, ErrEntityNotFound
 }
 
-func (ps *propertiesService) areDatabasePropertiesValid(dbProps []db.Property) bool {
+func (ps *propertiesService) areDatabasePropertiesValid(
+	dbProps []db.Property, opts *ReadOptions) bool {
 	// if the all the properties are to be valid, neither must be older than
 	// the cache timeout
 	for _, prop := range dbProps {
-		if !ps.isDatabasePropertyValid(prop) {
+		if !ps.isDatabasePropertyValid(prop, opts) {
 			return false
 		}
 	}
 	return true
 }
 
-func (ps *propertiesService) isDatabasePropertyValid(dbProp db.Property) bool {
-	if ps.entityTimeout == bypassCacheTimeout {
-		// this is mostly for testing
+func (ps *propertiesService) isDatabasePropertyValid(
+	dbProp db.Property, opts *ReadOptions) bool {
+	if ps.entityTimeout == bypassCacheTimeout || opts.canTolerateStaleData() {
 		return false
 	}
 	return time.Since(dbProp.UpdatedAt) < ps.entityTimeout