Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 19 Packages Projects Releases Wiki Activity

Removed the ptr helper package from Activity bounded context (#42161)

Browse Source 
Refactoring. No functional changes.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Simplified internal pointer value construction across activity-related
test and service code by consolidating helper functions and using Go's
built-in operators. No changes to user-facing functionality.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Victor Lyuboslavsky
2026-03-23 14:10:07 -05:00
committed by GitHub
parent ff35963a56
commit eb71cd43b9
6 changed files with 56 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/activity/arch_test.go
+4 -8
View File
@@ -2,6 +2,7 @@ package activity_test
import (
"regexp"
"slices"
"testing"
"github.com/fleetdm/fleet/v4/server/archtest"
@@ -73,15 +74,12 @@ func TestActivityPackageDependencies(t *testing.T) {
m + "/server/platform/mysql",
m + "/server/platform/mysql/testing_utils",
m + "/server/contexts/ctxerr",
m + "/server/ptr",
},
},
{
name: "internal/service depends on activity and platform packages",
pkg: m + "/server/activity/internal/service",
ignoreDeps: append(append([]string{
m + "/server/ptr",
}, activityPkgs...), platformPkgs...),
name: "internal/service depends on activity and platform packages",
pkg: m + "/server/activity/internal/service",
ignoreDeps: slices.Concat(activityPkgs, platformPkgs),
},
{
name: "bootstrap depends on activity and platform packages",
@@ -89,14 +87,12 @@ func TestActivityPackageDependencies(t *testing.T) {
ignoreDeps: append(append([]string{
m + "/server/activity/internal/mysql",
m + "/server/activity/internal/service",
m + "/server/ptr",
}, activityPkgs...), platformPkgs...),
},
{
name: "all packages only depend on activity and platform",
pkg: m + "/server/activity/...",
ignoreDeps: append(append([]string{
m + "/server/ptr",
m + "/server/activity/internal/mysql",
m + "/server/activity/internal/service",
m + "/server/activity/internal/testutils",
server/activity/internal/mysql/activity_cleanup_test.go
+7 -8
View File
@@ -6,7 +6,6 @@ import (
"time"
"github.com/fleetdm/fleet/v4/server/activity/internal/testutils"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -42,7 +41,7 @@ func testCleanupExpiredActivitiesNoop(t *testing.T, env *testEnv) {
// Create a recent activity -should not be deleted.
userID := env.InsertUser(t, "user", "user@example.com")
env.InsertActivity(t, ptr.Uint(userID), "recent_activity", map[string]any{})
env.InsertActivity(t, &userID, "recent_activity", map[string]any{})
err = env.ds.CleanupExpiredActivities(ctx, 500, 1)
require.NoError(t, err)
@@ -62,12 +61,12 @@ func testCleanupExpiredActivitiesBasic(t *testing.T, env *testEnv) {
// Create activities with different states:
// 1. Expired, no host link → should be deleted
expiredNoHost := env.InsertActivityWithTime(t, ptr.Uint(userID), "expired_no_host", map[string]any{}, expiredTime)
expiredNoHost := env.InsertActivityWithTime(t, &userID, "expired_no_host", map[string]any{}, expiredTime)
// 2. Expired, linked to host → should be preserved
expiredWithHost := env.InsertActivityWithTime(t, ptr.Uint(userID), "expired_with_host", map[string]any{}, expiredTime)
expiredWithHost := env.InsertActivityWithTime(t, &userID, "expired_with_host", map[string]any{}, expiredTime)
env.InsertHostActivity(t, hostID, expiredWithHost)
// 3. Recent, no host link → should be preserved
recentNoHost := env.InsertActivityWithTime(t, ptr.Uint(userID), "recent_no_host", map[string]any{}, recentTime)
recentNoHost := env.InsertActivityWithTime(t, &userID, "recent_no_host", map[string]any{}, recentTime)
err := env.ds.CleanupExpiredActivities(ctx, 500, 1)
require.NoError(t, err)
@@ -98,8 +97,8 @@ func testCleanupHostActivities(t *testing.T, env *testEnv) {
hostA := env.InsertHost(t, "hostA.local", nil)
hostB := env.InsertHost(t, "hostB.local", nil)
actA := env.InsertActivity(t, ptr.Uint(userID), "ran_script", map[string]any{})
actB := env.InsertActivity(t, ptr.Uint(userID), "ran_script", map[string]any{})
actA := env.InsertActivity(t, &userID, "ran_script", map[string]any{})
actB := env.InsertActivity(t, &userID, "ran_script", map[string]any{})
env.InsertHostActivity(t, hostA, actA)
env.InsertHostActivity(t, hostB, actB)
@@ -138,7 +137,7 @@ func testCleanupExpiredActivitiesBatch(t *testing.T, env *testEnv) {
// Create 10 expired activities (no host links).
for i := range 10 {
env.InsertActivityWithTime(t, ptr.Uint(userID), fmt.Sprintf("expired_%d", i), map[string]any{}, expiredTime)
env.InsertActivityWithTime(t, &userID, fmt.Sprintf("expired_%d", i), map[string]any{}, expiredTime)
}
// Cleanup with maxCount=3 -only 3 should be deleted per call.
server/activity/internal/mysql/activity_test.go
+20 -21
View File
@@ -8,7 +8,6 @@ import (
activityapi "github.com/fleetdm/fleet/v4/server/activity/api"
"github.com/fleetdm/fleet/v4/server/activity/internal/testutils"
"github.com/fleetdm/fleet/v4/server/activity/internal/types"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -53,7 +52,7 @@ func testListActivitiesBasic(t *testing.T, env *testEnv) {
// Create user activities and a system activity (nil user)
for i := range 3 {
env.InsertActivity(t, ptr.Uint(userID), fmt.Sprintf("test_activity_%d", i), map[string]any{"detail": i})
env.InsertActivity(t, &userID, fmt.Sprintf("test_activity_%d", i), map[string]any{"detail": i})
}
env.InsertActivity(t, nil, "system_activity", map[string]any{})
@@ -85,7 +84,7 @@ func testListActivitiesStreamed(t *testing.T, env *testEnv) {
activityIDs := make([]uint, 0, 3)
for i := range 3 {
id := env.InsertActivity(t, ptr.Uint(userID), "test_activity", map[string]any{"detail": i})
id := env.InsertActivity(t, &userID, "test_activity", map[string]any{"detail": i})
activityIDs = append(activityIDs, id)
}
@@ -99,8 +98,8 @@ func testListActivitiesStreamed(t *testing.T, env *testEnv) {
expectedIDs []uint
}{
{"all", nil, activityIDs},
{"non-streamed only", ptr.Bool(false), activityIDs[1:]},
{"streamed only", ptr.Bool(true), activityIDs[:1]},
{"non-streamed only", new(false), activityIDs[1:]},
{"streamed only", new(true), activityIDs[:1]},
}
for _, tc := range cases {
@@ -121,7 +120,7 @@ func testListActivitiesPaginationMetadata(t *testing.T, env *testEnv) {
userID := env.InsertUser(t, "testuser", "test@example.com")
for i := range 3 {
env.InsertActivity(t, ptr.Uint(userID), fmt.Sprintf("test_%d", i), map[string]any{})
env.InsertActivity(t, &userID, fmt.Sprintf("test_%d", i), map[string]any{})
}
cases := []struct {
@@ -153,9 +152,9 @@ func testListActivitiesActivityTypeFilter(t *testing.T, env *testEnv) {
ctx := t.Context()
userID := env.InsertUser(t, "testuser", "test@example.com")
env.InsertActivity(t, ptr.Uint(userID), "edited_script", map[string]any{})
env.InsertActivity(t, ptr.Uint(userID), "edited_script", map[string]any{})
env.InsertActivity(t, ptr.Uint(userID), "mdm_enrolled", map[string]any{})
env.InsertActivity(t, &userID, "edited_script", map[string]any{})
env.InsertActivity(t, &userID, "edited_script", map[string]any{})
env.InsertActivity(t, &userID, "mdm_enrolled", map[string]any{})
cases := []struct {
activityType string
@@ -190,7 +189,7 @@ func testListActivitiesDateRangeFilter(t *testing.T, env *testEnv) {
now,
}
for _, dt := range dates {
env.InsertActivityWithTime(t, ptr.Uint(userID), "test_activity", map[string]any{}, dt)
env.InsertActivityWithTime(t, &userID, "test_activity", map[string]any{}, dt)
}
cases := []struct {
@@ -220,8 +219,8 @@ func testListActivitiesMatchQuery(t *testing.T, env *testEnv) {
johnUserID := env.InsertUser(t, "john_doe", "john@example.com")
janeUserID := env.InsertUser(t, "jane_smith", "jane@example.com")
env.InsertActivity(t, ptr.Uint(johnUserID), "test_activity", map[string]any{})
env.InsertActivity(t, ptr.Uint(janeUserID), "test_activity", map[string]any{})
env.InsertActivity(t, &johnUserID, "test_activity", map[string]any{})
env.InsertActivity(t, &janeUserID, "test_activity", map[string]any{})
cases := []struct {
name string
@@ -252,9 +251,9 @@ func testListActivitiesOrdering(t *testing.T, env *testEnv) {
userID := env.InsertUser(t, "testuser", "test@example.com")
now := time.Now().UTC().Truncate(time.Second)
env.InsertActivityWithTime(t, ptr.Uint(userID), "activity_oldest", map[string]any{}, now.Add(-2*time.Hour))
env.InsertActivityWithTime(t, ptr.Uint(userID), "activity_middle", map[string]any{}, now.Add(-1*time.Hour))
env.InsertActivityWithTime(t, ptr.Uint(userID), "activity_newest", map[string]any{}, now)
env.InsertActivityWithTime(t, &userID, "activity_oldest", map[string]any{}, now.Add(-2*time.Hour))
env.InsertActivityWithTime(t, &userID, "activity_middle", map[string]any{}, now.Add(-1*time.Hour))
env.InsertActivityWithTime(t, &userID, "activity_newest", map[string]any{}, now)
cases := []struct {
name string
@@ -290,7 +289,7 @@ func testListActivitiesCursorPagination(t *testing.T, env *testEnv) {
userID := env.InsertUser(t, "testuser", "test@example.com")
for i := range 5 {
env.InsertActivity(t, ptr.Uint(userID), fmt.Sprintf("activity_%d", i), map[string]any{})
env.InsertActivity(t, &userID, fmt.Sprintf("activity_%d", i), map[string]any{})
}
// Get first page
@@ -368,8 +367,8 @@ func testListHostPastActivities(t *testing.T, env *testEnv) {
hostID := env.InsertHost(t, "h1.local", nil)
// Create activities linked to host with different types
env.InsertHostActivity(t, hostID, env.InsertActivity(t, ptr.Uint(userID), "ran_script", map[string]any{"host_id": float64(hostID)}))
env.InsertHostActivity(t, hostID, env.InsertActivity(t, ptr.Uint(userID), "installed_software", map[string]any{"host_id": float64(hostID)}))
env.InsertHostActivity(t, hostID, env.InsertActivity(t, &userID, "ran_script", map[string]any{"host_id": float64(hostID)}))
env.InsertHostActivity(t, hostID, env.InsertActivity(t, &userID, "installed_software", map[string]any{"host_id": float64(hostID)}))
cases := []struct {
name string
@@ -406,14 +405,14 @@ func testMarkActivitiesAsStreamed(t *testing.T, env *testEnv) {
ctx := t.Context()
userID := env.InsertUser(t, "testuser", "test@example.com")
actA := env.InsertActivity(t, ptr.Uint(userID), "activity_a", map[string]any{})
actB := env.InsertActivity(t, ptr.Uint(userID), "activity_b", map[string]any{})
actA := env.InsertActivity(t, &userID, "activity_a", map[string]any{})
actB := env.InsertActivity(t, &userID, "activity_b", map[string]any{})
err := env.ds.MarkActivitiesAsStreamed(ctx, []uint{actA, actB})
require.NoError(t, err)
// Verify marked as streamed
activities, meta, err := env.ds.ListActivities(ctx, listOpts(withStreamed(ptr.Bool(true)), withMetadata()))
activities, meta, err := env.ds.ListActivities(ctx, listOpts(withStreamed(new(true)), withMetadata()))
require.NoError(t, err)
assert.Len(t, activities, 2)
require.NotNil(t, meta)
server/activity/internal/service/service.go
+1 -2
View File
@@ -14,7 +14,6 @@ import (
"github.com/fleetdm/fleet/v4/server/activity/internal/types"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
platform_authz "github.com/fleetdm/fleet/v4/server/platform/authz"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/hashicorp/go-multierror"
"go.opentelemetry.io/otel"
)
@@ -199,7 +198,7 @@ func (s *Service) StreamActivities(systemCtx context.Context, auditLogger api.JS
OrderDirection: api.OrderAscending,
PerPage: streamBatchSize,
After: idCursor(afterID),
Streamed: ptr.Bool(false),
Streamed: new(false),
})
if err != nil {
return ctxerr.Wrap(systemCtx, err, "list activities")
server/activity/internal/service/service_test.go
+8 -9
View File
@@ -13,7 +13,6 @@ import (
"github.com/fleetdm/fleet/v4/server/activity/api"
"github.com/fleetdm/fleet/v4/server/activity/internal/types"
platform_authz "github.com/fleetdm/fleet/v4/server/platform/authz"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -209,8 +208,8 @@ func TestListActivitiesWithUserEnrichment(t *testing.T) {
ts := setupTest(
withActivities([]*api.Activity{
{ID: 1, Type: "test_activity", ActorID: ptr.Uint(johnUser.ID)},
{ID: 2, Type: "another_activity", ActorID: ptr.Uint(janeUser.ID)},
{ID: 1, Type: "test_activity", ActorID: &johnUser.ID},
{ID: 2, Type: "another_activity", ActorID: &janeUser.ID},
{ID: 3, Type: "system_activity"}, // No actor
}),
withUsers([]*activity.User{johnUser, janeUser}),
@@ -253,7 +252,7 @@ func TestListActivitiesDeletedUserFallsBackToStoredName(t *testing.T) {
ts := setupTest(
withActivities([]*api.Activity{
{ID: 1, Type: "test_activity", ActorID: ptr.Uint(deletedUserID), ActorFullName: &storedName, ActorEmail: &storedEmail},
{ID: 1, Type: "test_activity", ActorID: &deletedUserID, ActorFullName: &storedName, ActorEmail: &storedEmail},
}),
// UsersByIDs returns nothing for the deleted user
withUsers(nil),
@@ -289,7 +288,7 @@ func TestListActivitiesWithMatchQuery(t *testing.T) {
ts := setupTest(
withActivities([]*api.Activity{
{ID: 1, Type: "test_activity", ActorID: ptr.Uint(johnUser.ID)},
{ID: 1, Type: "test_activity", ActorID: &johnUser.ID},
}),
withSearchUserIDs([]uint{100, 200, 300}), // 3 users match "john", but only user 100 has activities
withUsers([]*activity.User{johnUser}),
@@ -347,9 +346,9 @@ func TestListActivitiesWithDuplicateUserIDs(t *testing.T) {
// Multiple activities by the same user
ts := setupTest(
withActivities([]*api.Activity{
{ID: 1, Type: "created_policy", ActorID: ptr.Uint(johnUser.ID)},
{ID: 2, Type: "deleted_policy", ActorID: ptr.Uint(johnUser.ID)},
{ID: 3, Type: "edited_policy", ActorID: ptr.Uint(johnUser.ID)},
{ID: 1, Type: "created_policy", ActorID: &johnUser.ID},
{ID: 2, Type: "deleted_policy", ActorID: &johnUser.ID},
{ID: 3, Type: "edited_policy", ActorID: &johnUser.ID},
}),
withUsers([]*activity.User{johnUser}),
)
@@ -419,7 +418,7 @@ func TestListActivitiesErrors(t *testing.T) {
name: "user enrichment error",
opts: []func(*testSetup){
withActivities([]*api.Activity{
{ID: 1, Type: "test_activity", ActorID: ptr.Uint(100)},
{ID: 1, Type: "test_activity", ActorID: new(uint(100))},
}),
withUsersByIDsError(errors.New("user service error")),
},
server/activity/internal/tests/integration_test.go
+16 -17
View File
@@ -6,7 +6,6 @@ import (
"testing"
"time"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -37,9 +36,9 @@ func testListActivities(t *testing.T, s *integrationTestSuite) {
userID := s.insertUser(t, "admin", "admin@example.com")
// Insert activities
s.InsertActivity(t, ptr.Uint(userID), "applied_spec_pack", map[string]any{})
s.InsertActivity(t, ptr.Uint(userID), "deleted_pack", map[string]any{})
s.InsertActivity(t, ptr.Uint(userID), "edited_pack", map[string]any{})
s.InsertActivity(t, &userID, "applied_spec_pack", map[string]any{})
s.InsertActivity(t, &userID, "deleted_pack", map[string]any{})
s.InsertActivity(t, &userID, "edited_pack", map[string]any{})
result, statusCode := s.getActivities(t, "per_page=100")
@@ -58,7 +57,7 @@ func testListActivitiesPagination(t *testing.T, s *integrationTestSuite) {
// Insert 5 activities
for i := range 5 {
s.InsertActivity(t, ptr.Uint(userID), "test_activity", map[string]any{"index": i})
s.InsertActivity(t, &userID, "test_activity", map[string]any{"index": i})
}
// First page
@@ -84,9 +83,9 @@ func testListActivitiesCursorPagination(t *testing.T, s *integrationTestSuite) {
userID := s.insertUser(t, "admin", "admin@example.com")
// Insert 3 activities
s.InsertActivity(t, ptr.Uint(userID), "applied_spec_pack", map[string]any{})
s.InsertActivity(t, ptr.Uint(userID), "deleted_pack", map[string]any{})
s.InsertActivity(t, ptr.Uint(userID), "edited_pack", map[string]any{})
s.InsertActivity(t, &userID, "applied_spec_pack", map[string]any{})
s.InsertActivity(t, &userID, "deleted_pack", map[string]any{})
s.InsertActivity(t, &userID, "edited_pack", map[string]any{})
// Test cursor-based pagination with after=0
// Meta should be nil for cursor-based pagination (doesn't return metadata)
@@ -117,10 +116,10 @@ func testListActivitiesFilters(t *testing.T, s *integrationTestSuite) {
now := time.Now().UTC().Truncate(time.Second)
// Insert activities with different types, times, and users
s.InsertActivityWithTime(t, ptr.Uint(johnUserID), "type_a", map[string]any{}, now.Add(-48*time.Hour))
s.InsertActivityWithTime(t, ptr.Uint(johnUserID), "type_a", map[string]any{}, now.Add(-24*time.Hour))
s.InsertActivityWithTime(t, ptr.Uint(johnUserID), "type_b", map[string]any{}, now)
s.InsertActivityWithTime(t, ptr.Uint(janeUserID), "type_a", map[string]any{}, now) // Jane's activity
s.InsertActivityWithTime(t, &johnUserID, "type_a", map[string]any{}, now.Add(-48*time.Hour))
s.InsertActivityWithTime(t, &johnUserID, "type_a", map[string]any{}, now.Add(-24*time.Hour))
s.InsertActivityWithTime(t, &johnUserID, "type_b", map[string]any{}, now)
s.InsertActivityWithTime(t, &janeUserID, "type_a", map[string]any{}, now) // Jane's activity
// Filter by type
result, _ := s.getActivities(t, "per_page=100&activity_type=type_a")
@@ -152,7 +151,7 @@ func testListActivitiesFilters(t *testing.T, s *integrationTestSuite) {
func testListActivitiesUserEnrichment(t *testing.T, s *integrationTestSuite) {
userID := s.insertUser(t, "John Doe", "john@example.com")
s.InsertActivity(t, ptr.Uint(userID), "test_activity", map[string]any{})
s.InsertActivity(t, &userID, "test_activity", map[string]any{})
result, _ := s.getActivities(t, "per_page=100")
require.Len(t, result.Activities, 1)
@@ -175,9 +174,9 @@ func testListHostPastActivities(t *testing.T, s *integrationTestSuite) {
hostB := s.insertHost(t, "host-b.example.com", nil)
// Create activities linked to different hosts
actA := s.InsertActivity(t, ptr.Uint(userID), "ran_script", map[string]any{"host": "host-a"})
actB := s.InsertActivity(t, ptr.Uint(userID), "installed_software", map[string]any{"host": "host-a"})
actC := s.InsertActivity(t, ptr.Uint(userID), "ran_script", map[string]any{"host": "host-b"})
actA := s.InsertActivity(t, &userID, "ran_script", map[string]any{"host": "host-a"})
actB := s.InsertActivity(t, &userID, "installed_software", map[string]any{"host": "host-a"})
actC := s.InsertActivity(t, &userID, "ran_script", map[string]any{"host": "host-b"})
// Link activities to hosts
s.InsertHostActivity(t, hostA, actA)
@@ -211,7 +210,7 @@ func testListHostPastActivities(t *testing.T, s *integrationTestSuite) {
// Insert 5 activities for the host
for i := range 5 {
actID := s.InsertActivity(t, ptr.Uint(userID), "test_activity", map[string]any{"index": i})
actID := s.InsertActivity(t, &userID, "test_activity", map[string]any{"index": i})
s.InsertHostActivity(t, host, actID)
}