Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/41233-host-software-has-results
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Fixed pagination on the host software page incorrectly disabling the "Next" button when a software title has multiple installer versions.
6 changes: 3 additions & 3 deletions server/datastore/mysql/software.go
Original file line number Diff line number Diff line change
Expand Up @@ -5549,7 +5549,7 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt
ctx,
ds.reader(ctx),
&titleCount,
fmt.Sprintf("SELECT COUNT(id) FROM (%s) AS combined_results", countStmt),
fmt.Sprintf("SELECT COUNT(DISTINCT id) FROM (%s) AS combined_results", countStmt),
args...,
); err != nil {
return nil, nil, ctxerr.Wrap(ctx, err, "get host software count")
Expand Down Expand Up @@ -6067,10 +6067,10 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt
metaData = &fleet.PaginationMetadata{
HasPreviousResults: opts.ListOptions.Page > 0,
TotalResults: titleCount,
HasNextResults: titleCount > (opts.ListOptions.Page+1)*perPage,
}
if len(hostSoftwareList) > int(perPage) { //nolint:gosec // dismiss G115
metaData.HasNextResults = true
hostSoftwareList = hostSoftwareList[:len(hostSoftwareList)-1]
hostSoftwareList = hostSoftwareList[:perPage]
Comment on lines +6070 to +6073
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Page boundaries are still based on pre-deduped rows.

HasNextResults is now computed from the distinct title count, but the SQL pagination is still applied earlier at Line 5687, before Lines 5771-6059 collapse duplicate installer rows into one item per title. That means this change makes later pages reachable, but those pages can still overlap or come back short whenever one title expands into multiple raw rows. Page the distinct software_titles.id set first, then hydrate installer/app details for just that page.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/datastore/mysql/software.go` around lines 6070 - 6073, HasNextResults
is computed from the distinct title count but SQL pagination is still applied to
raw rows (hostSoftwareList) before deduplication, causing overlap/short pages;
change the query flow so you first page the distinct software_titles.id set (use
software_titles.id as the pagination key) to fetch only the titles for the
requested page, then hydrate installer/app details for those title ids to build
hostSoftwareList and set HasNextResults from the distinct count; update code
paths around HasNextResults, hostSoftwareList population, and the query that
currently slices hostSoftwareList (the pre-deduplication pagination) so
pagination is done against the deduplicated title ids before fetching
installers.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed: #41277

}
Comment thread
getvictor marked this conversation as resolved.
}

Expand Down
95 changes: 95 additions & 0 deletions server/datastore/mysql/software_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func TestSoftware(t *testing.T) {
{"InsertHostSoftwareInstalledPaths", testInsertHostSoftwareInstalledPaths},
{"VerifySoftwareChecksum", testVerifySoftwareChecksum},
{"ListHostSoftware", testListHostSoftware},
{"ListHostSoftwarePaginationWithMultipleInstallers", testListHostSoftwarePaginationWithMultipleInstallers},
{"ListLinuxHostSoftware", testListLinuxHostSoftware},
{"ListIOSHostSoftware", testListIOSHostSoftware},
{"ListHostSoftwareWithVPPApps", testListHostSoftwareWithVPPApps},
Expand Down Expand Up @@ -11510,6 +11511,100 @@ func testListHostSoftwareShPackageForDarwin(t *testing.T, ds *Datastore) {
require.False(t, foundDeb, ".deb package should NOT be visible to windows host")
}

// testListHostSoftwarePaginationWithMultipleInstallers verifies that pagination metadata
// (HasNextResults, TotalResults) is correct when a software title has multiple installers
// (different versions). The UNION data query LEFT JOINs software_installers and groups by
// installer ID, so multiple installers for the same title produce multiple rows. Without the
// fix, the row-count heuristic for HasNextResults would report false prematurely because
// post-query deduplication shrinks the result set below perPage.
// See https://github.com/fleetdm/fleet/issues/41233
func testListHostSoftwarePaginationWithMultipleInstallers(t *testing.T, ds *Datastore) {
ctx := t.Context()

host := test.NewHost(t, ds, "pag-host", "", "pagkey", "paguuid", time.Now(), test.WithPlatform("darwin"))

// Install 10 software titles on the host with perPage=5, giving 2 pages.
const totalTitles = 10
software := make([]fleet.Software, totalTitles)
for i := range software {
software[i] = fleet.Software{
Name: fmt.Sprintf("pagsw-%02d", i),
Version: "1.0.0",
Source: "apps",
}
}
_, err := ds.UpdateHostSoftware(ctx, host.ID, software)
require.NoError(t, err)

// Give the last title on page 0 (pagsw-04, the 5th item when ordered by name)
// two installers (different versions).
//
// The data query produces these raw rows for page 0 (LIMIT 6, OFFSET 0):
// pagsw-00 (1 row), pagsw-01 (1 row), pagsw-02 (1 row), pagsw-03 (1 row),
// pagsw-04 installer-v1 (1 row), pagsw-04 installer-v2 (1 row)
// = 6 raw rows → 5 unique titles after dedup.
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
insertScript := func(script string) (int64, error) {
res, err := q.ExecContext(ctx,
`INSERT INTO script_contents (md5_checksum, contents) VALUES (UNHEX(MD5(?)), ?)`, script, script,
)
if err != nil {
return 0, err
}
return res.LastInsertId()
}
installScriptID, err := insertScript("echo install")
if err != nil {
return err
}
uninstallScriptID, err := insertScript("echo uninstall")
if err != nil {
return err
}

var titleID uint
if err := sqlx.GetContext(ctx, q, &titleID,
`SELECT id FROM software_titles WHERE name = 'pagsw-04' AND source = 'apps'`,
); err != nil {
return err
}

for _, version := range []string{"1.0.0", "2.0.0"} {
if _, err := q.ExecContext(ctx, `
INSERT INTO software_installers
(team_id, global_or_team_id, title_id, filename, extension, version,
install_script_content_id, uninstall_script_content_id, storage_id, platform, self_service, package_ids)
VALUES (NULL, 0, ?, ?, 'pkg', ?, ?, ?, ?, 'darwin', 0, '[]')`,
titleID, fmt.Sprintf("installer-%s.pkg", version), version,
installScriptID, uninstallScriptID, fmt.Appendf(nil, "storage-%s", version),
); err != nil {
return err
}
}

return nil
})

const perPage = 5
opts := fleet.HostSoftwareTitleListOptions{
ListOptions: fleet.ListOptions{
PerPage: perPage,
IncludeMetadata: true,
OrderKey: "name",
},
}

// Page 0: 5 unique titles, HasNextResults must be true (there are 10 total).
opts.ListOptions.Page = 0
sw, meta, err := ds.ListHostSoftware(ctx, host, opts)
require.NoError(t, err)
require.Len(t, sw, perPage, "page 0 should have %d items", perPage)
require.NotNil(t, meta)
assert.Equal(t, uint(totalTitles), meta.TotalResults, "total results should reflect unique titles, not duplicated rows")
assert.True(t, meta.HasNextResults, "page 0 of %d items with perPage=%d should have next results", totalTitles, perPage)
assert.False(t, meta.HasPreviousResults)
}

// TestUniqueSoftwareTitleStrNormalization tests that UniqueSoftwareTitleStr
// produces consistent keys regardless of Unicode format characters (like RTL mark)
// which MySQL's utf8mb4_unicode_ci collation ignores.
Expand Down
Loading