Use the new property cache in ListEvaluationResults (#4731)

* Use the property caching wrapper in ListRuleEvaluations

* Add a test for properties.Len()

* Add tests for property cache

* Review feedback: drop size hint

internal/controlplane/handlers_evalstatus.go

@@ -372,6 +372,12 @@ func (s *Server) sortEntitiesEvaluationStatus(
 	profileStatuses = map[uuid.UUID]*minderv1.ProfileStatus{}
 	statusByEntity = map[string]map[uuid.UUID][]*minderv1.RuleEvaluationStatus{}
 
+	psc, err := propSvc.WithEntityCache(s.props, 0)
+	if err != nil {
+		return nil, nil, nil,
+			status.Errorf(codes.Internal, "error creating entity cache: %v", err)
+	}
+
 	for _, p := range profileList {
 		p := p
 		evals, err := store.ListRuleEvaluationsByProfileId(
@@ -393,7 +399,7 @@ func (s *Server) sortEntitiesEvaluationStatus(
 				continue
 			}
 
-			efp, err := s.props.EntityWithPropertiesByID(ctx, e.EntityID, nil)
+			efp, err := psc.EntityWithPropertiesByID(ctx, e.EntityID, nil)
 			if err != nil {
 				if errors.Is(err, propSvc.ErrEntityNotFound) {
 					// If the entity is not found, log and skip

internal/entities/properties/properties_test.go

@@ -855,3 +855,57 @@ func TestProperties_ToLogDict(t *testing.T) {
 		require.Equal(t, expectedValue, actualValue)
 	}
 }
+
+func TestProperties_Len(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		p    *Properties
+		want int
+	}{
+		{
+			name: "nil Properties",
+			p:    nil,
+			want: 0,
+		},
+		{
+			name: "empty Properties",
+			p: func() *Properties {
+				p, _ := NewProperties(map[string]any{})
+				return p
+			}(),
+			want: 0,
+		},
+		{
+			name: "Properties with one item",
+			p: func() *Properties {
+				p, _ := NewProperties(map[string]any{"key1": "value1"})
+				return p
+			}(),
+			want: 1,
+		},
+		{
+			name: "Properties with multiple items",
+			p: func() *Properties {
+				p, _ := NewProperties(map[string]any{
+					"key1": "value1",
+					"key2": 42,
+					"key3": true,
+				})
+				return p
+			}(),
+			want: 3,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			if got := tt.p.Len(); got != tt.want {
+				t.Errorf("Properties.Len() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

internal/entities/properties/service/service_test.go

@@ -1162,3 +1162,99 @@ func TestPropertiesService_EntityWithProperties(t *testing.T) {
 		})
 	}
 }
+
+func TestPropertiesService_EntityWithProperties_WithCache(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	t.Cleanup(cancel)
+
+	ctrl := gomock.NewController(t)
+	t.Cleanup(ctrl.Finish)
+
+	t.Run("Test caching", func(t *testing.T) {
+		t.Parallel()
+
+		mockDB := mockdb.NewMockStore(ctrl)
+
+		entityID := uuid.New()
+		entityName := "myorg/bad-go"
+
+		entityRet := db.EntityInstance{
+			ID:   entityID,
+			Name: entityName,
+		}
+		propertyRet := []db.Property{
+			{
+				EntityID: entityID,
+				Key:      "name",
+				Value:    []byte(`{"value": "myorg/bad-go", "version": "v1"}`),
+			},
+			{
+				EntityID: entityID,
+				Key:      "is_private",
+				Value:    []byte(`{"value": false, "version": "v1"}`),
+			},
+		}
+
+		// we verify that the entity is only fetched once even though we call the service twice
+		mockDB.EXPECT().
+			GetEntityByID(ctx, entityID).
+			Return(entityRet, nil).
+			Times(1)
+		mockDB.EXPECT().
+			GetAllPropertiesForEntity(ctx, entityID).
+			Return(propertyRet, nil).
+			Times(1)
+
+		ps := NewPropertiesService(mockDB)
+		cps, err := WithEntityCache(ps, 100)
+		require.NoError(t, err)
+
+		t.Log("First call, no cache")
+		result, err := cps.EntityWithPropertiesByID(ctx, entityID, nil)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+		require.Equal(t, result.Entity.ID, entityID)
+		require.Equal(t, result.Entity.Name, entityName)
+		require.Equal(t, result.Properties.GetProperty("name").GetString(), "myorg/bad-go")
+		require.Equal(t, result.Properties.GetProperty("is_private").GetBool(), false)
+
+		t.Log("Second call, cache hit")
+		result, err = cps.EntityWithPropertiesByID(ctx, entityID, nil)
+		require.NoError(t, err)
+		require.NotNil(t, result)
+		require.Equal(t, result.Entity.ID, entityID)
+		require.Equal(t, result.Entity.Name, entityName)
+		require.Equal(t, result.Properties.GetProperty("name").GetString(), "myorg/bad-go")
+		require.Equal(t, result.Properties.GetProperty("is_private").GetBool(), false)
+	})
+
+	t.Run("Errors are propagated", func(t *testing.T) {
+		t.Parallel()
+
+		mockDB := mockdb.NewMockStore(ctrl)
+
+		entityID := uuid.New()
+
+		mockDB.EXPECT().
+			GetEntityByID(ctx, entityID).
+			Return(db.EntityInstance{}, ErrEntityNotFound).
+			Times(1)
+
+		ps := NewPropertiesService(mockDB)
+		cps, err := WithEntityCache(ps, 100)
+		require.NoError(t, err)
+
+		result, err := cps.EntityWithPropertiesByID(ctx, entityID, nil)
+		require.ErrorIs(t, err, ErrEntityNotFound)
+		require.Nil(t, result)
+	})
+
+	t.Run("PropertyService is required", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := WithEntityCache(nil, 100)
+		require.Error(t, err)
+	})
+}