Skip to content

Commit 214d8c7

Browse files
committed
feat: add pagination envelope {data,page,count,items_per_page} to /query
1 parent de70d02 commit 214d8c7

13 files changed

Lines changed: 196 additions & 69 deletions

File tree

frontend/src/api.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
TickInfo,
99
Flow,
1010
FlowsQuery,
11+
FlowsResponse,
1112
} from "./types";
1213

1314
export const tulipApi = createApi({
@@ -22,7 +23,7 @@ export const tulipApi = createApi({
2223
getFlow: builder.query<FullFlow, string>({
2324
query: (id) => `/flow/${id}`,
2425
}),
25-
getFlows: builder.query<Flow[], FlowsQuery>({
26+
getFlows: builder.query<FlowsResponse, FlowsQuery>({
2627
query: (query) => ({
2728
url: `/query`,
2829
method: "POST",

frontend/src/components/Corrie.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ export function Corrie() {
9999

100100
// TODO: fix the below transformation - move it to server
101101
// Diederik gives you a beer once it has been fixed
102-
const transformedFlowData = flowData?.map((flow) => ({
102+
const transformedFlowData = flowData?.data.map((flow) => ({
103103
...flow,
104104
service_tag:
105105
services?.find((s) => s.ip === flow.dst_ip && s.port === flow.dst_port)

frontend/src/components/FlowList.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,12 @@ export function FlowList() {
157157
offset: allFlows.length,
158158
}).unwrap();
159159

160-
if (result.length < PAGE_SIZE) {
160+
if (allFlows.length + result.data.length >= result.count) {
161161
setHasMore(false);
162162
}
163163

164164
// Transform new flows with service tags
165-
const transformedNewFlows = result.map((flow) => ({
165+
const transformedNewFlows = result.data.map((flow) => ({
166166
...flow,
167167
service_tag:
168168
services?.find(
@@ -188,15 +188,15 @@ export function FlowList() {
188188
// Reset flows when filters change
189189
useEffect(() => {
190190
if (flowData) {
191-
const transformed = flowData.map((flow) => ({
191+
const transformed = flowData.data.map((flow) => ({
192192
...flow,
193193
service_tag:
194194
services?.find(
195195
(s) => s.ip === flow.dst_ip && s.port === flow.dst_port,
196196
)?.name ?? "unknown",
197197
}));
198198
setAllFlows(transformed);
199-
setHasMore(flowData.length === PAGE_SIZE);
199+
setHasMore(transformed.length < flowData.count);
200200
} else if (!isLoading) {
201201
// Clear flows if no data and not loading
202202
setAllFlows([]);

frontend/src/pages/Fingerprinter.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function Fingerprinter() {
3636
);
3737
}
3838

39-
if (!data || !Array.isArray(data) || data.length === 0) {
39+
if (!data || !data.data || data.data.length === 0) {
4040
return (
4141
<div className="flex justify-center items-center h-[60vh]">
4242
<span className="text-2xl font-semibold text-gray-500">
@@ -50,7 +50,7 @@ export function Fingerprinter() {
5050
<div className="max-w-2xl mx-auto mt-10">
5151
<h2 className="text-3xl font-bold mb-6 text-center">Timeline for Fingerprint {idNumber}</h2>
5252
<ol className="relative border-l-2 border-blue-500">
53-
{data.map((item, idx) => (
53+
{data.data.map((item, idx) => (
5454
<li key={item._id || idx} className="mb-10 ml-6">
5555
<Link
5656
to={`/flow/${item._id}`}

frontend/src/pages/Fingerprints.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function FingerprintSection({ fingerprint }: { fingerprint: number }) {
5757
);
5858
}
5959

60-
if (!flows || flows.length === 0) {
60+
if (!flows || !flows.data || flows.data.length === 0) {
6161
return (
6262
<section
6363
key={fingerprint}
@@ -80,12 +80,12 @@ function FingerprintSection({ fingerprint }: { fingerprint: number }) {
8080
{fingerprint}
8181
</span>
8282
<span className="ml-2 bg-blue-100 dark:bg-blue-900 dark:text-blue-200 text-blue-800 text-xs font-semibold px-2.5 py-0.5 rounded">
83-
{flows.length} flow{flows.length !== 1 ? "s" : ""}
83+
{flows.data.length} flow{flows.data.length !== 1 ? "s" : ""}
8484
</span>
8585
</div>
8686
<div>
8787
<ul className="space-y-1">
88-
{flows.slice(0, 5).map((flow, idx: number) => (
88+
{flows.data.slice(0, 5).map((flow, idx: number) => (
8989
<li key={flow._id || idx}>
9090
Flow
9191
<Link

frontend/src/pages/FlowView.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -780,8 +780,8 @@ function FlowFingerprintTimeline({
780780
<div className="text-red-500 dark:text-red-400">
781781
Error fetching related flows
782782
</div>
783-
) : relatedFlows && relatedFlows.length > 0 ? (
784-
renderTimeline(relatedFlows)
783+
) : relatedFlows?.data && relatedFlows.data.length > 0 ? (
784+
renderTimeline(relatedFlows.data)
785785
) : (
786786
<div className="text-gray-500 dark:text-gray-400">
787787
No related flows found.

frontend/src/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ export type FlowsQuery = {
5959
fingerprints?: number[];
6060
};
6161

62+
export type FlowsResponse = {
63+
data: Flow[];
64+
page: number;
65+
count: number;
66+
items_per_page: number;
67+
};
68+
6269
export type Service = {
6370
ip: string;
6471
port: number;
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# Plan: Progressive Improvement of Tulip
2+
3+
**TL;DR:** Address the highest-impact issues first (performance regressions), then improve code robustness (error handling, context propagation), then clean up UX hacks. Each phase is independently shippable.
4+
5+
---
6+
7+
## Phase 1 — Performance Quick Wins (~2–3 days, highest ROI)
8+
9+
**1.1 Fix N+1 signature queries** — services/cmd/api/api.go ~L236–256
10+
Currently fetches each SuricataSig individually in a loop per flow. Add `GetSignaturesBatch(ctx, ids []string) []SuricataSig` to services/pkg/db/db.go backed by a `$in` query in services/pkg/db/mongo.go. 50 flows × 3 sigs = 150 round trips → 1.
11+
12+
**1.2 Add compound index for enricher lookups** — services/pkg/db/mongo.go ~L201–206
13+
`AddSignatureToFlow` and `AddTagsToFlow` filter on `{src_ip, dst_ip, src_port, dst_port, time}` but no compound index covers this. Add index: `{src_ip: 1, dst_ip: 1, src_port: 1, dst_port: 1, time: 1}`.
14+
15+
**1.3 Fix redundant tag aggregation** — services/pkg/db/mongo.go ~L60–111
16+
`GetTagList()` runs two full collection scans and merges with an O(n²) loop. Replace with a single authoritative source: keep only the `tags` collection lookup (fed by `InsertTag` calls which already exist) and drop the flows aggregation pass.
17+
18+
**1.4 Add text index for flow content search** — services/pkg/db/mongo.go ~L465
19+
`flow.data` regex search does a full collection scan. Add a denormalized `search_data` root-level field populated at insert time by joining all `FlowItem.Data` values — enabling the existing text index to cover it.
20+
21+
---
22+
23+
## Phase 2 — Robustness & API Correctness (~3–4 days)
24+
25+
**2.1 Propagate request context through DB calls** — services/pkg/db/mongo.go
26+
31 uses of `context.TODO()` mean in-flight DB queries can't be cancelled on client disconnect or timeout. Change all DB interface methods in services/pkg/db/db.go to accept `ctx context.Context` and pass the Echo request context from services/cmd/api/api.go.
27+
28+
**2.2 Return errors from tag inserts** — services/pkg/db/mongo.go ~L415–424
29+
`InsertTags` / `InsertTag` silently swallow all errors. Change to return `error`; callers can ignore duplicate-key errors specifically (`mongo.IsDuplicateKeyError`) and log others.
30+
31+
**2.3 Fix empty-tags edge case** — services/cmd/api/api.go + frontend/src/api.ts ~L33
32+
Empty `includeTags`/`excludeTags` arrays cause incorrect query behavior (known TODO with a beer bounty). Backend: guard — if array is empty, skip the `$all`/`$nin` clause. Remove the frontend workaround once backend handles it.
33+
34+
**2.4 Add pagination metadata to `/query` response** — services/cmd/api/api.go ~L166
35+
Wrap the response: `{data: [...], meta: {total: N, hasMore: bool}}`. Run `CountFlows` in parallel with `GetFlows`. Frontend can use `hasMore` to disable infinite scroll rather than guessing.
36+
37+
---
38+
39+
## Phase 3 — Technical Debt & UX Hacks (~1 week, incremental)
40+
41+
**3.1 Fix timezone display in chart** — frontend/src/components/Corrie.tsx ~L235, L324
42+
FIXME comment: datetime axis shows wrong timezone. Audit Chart.js time scale config; use UTC-aware formatting with `date-fns-tz` or standardize on UTC display throughout.
43+
44+
**3.2 Replace O(n) flow index scan with Map** — frontend/src/pages/FlowList.tsx ~L251–280
45+
Keyboard navigation triggers a linear search over all loaded flows on each keypress. Replace with a `Map<flowId, index>` maintained alongside the flow list — O(1) lookup.
46+
47+
**3.3 Fix scroll ↔ keyboard focus desync** — frontend/src/pages/FlowView.tsx ~L579
48+
TODO: manual scroll doesn't update `currentFlow`, causing keyboard navigation to jump. Add an `IntersectionObserver` or scroll event listener to sync the active flow with the visible item.
49+
50+
**3.4 Decouple API layer from BSON** — services/cmd/api/api.go ~L73
51+
TODO comment: "API layer should not be aware of database structure." Move filter-building logic into a `db.FlowQuery` struct in services/pkg/db/db.go; api.go populates the struct, mongo.go translates it to BSON. Enables unit-testing API logic without a real DB.
52+
53+
---
54+
55+
## Verification per Phase
56+
- **Phase 1:** Benchmark `/query` + enrichment time before/after with `mongosh` + `hey`; run existing Go tests
57+
- **Phase 2:** Integration test empty-tag query; confirm context cancellation propagates; check `/query` response shape in frontend
58+
- **Phase 3:** Manual test keyboard nav + scroll; regression test chart rendering; run `npx eslint` on changed frontend files
59+
60+
---
61+
62+
## Priority order if time is limited
63+
1. Step 1.1 (N+1 fix) — biggest single perf gain, small code change
64+
2. Step 1.2 (compound index) — zero-risk, one line
65+
3. Step 2.1 (context propagation) — correctness, enables cancellation
66+
4. Step 2.3 (empty tags) — fixes a known bug with a beer bounty on it
67+
68+
## Decisions
69+
- MongoDB stays — migration not worth it at CTF scale
70+
- Each phase is independently mergeable
71+
- Step 3.4 (BSON decoupling) is optional/long-term — only pursue if a DB swap becomes relevant later

services/cmd/api/api.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"path/filepath"
1717
"strconv"
1818
"strings"
19+
"sync"
1920
"tulip/pkg/db"
2021

2122
"github.com/labstack/echo/v4"
@@ -250,6 +251,16 @@ func (api *Router) query(c echo.Context) error {
250251
return c.JSON(http.StatusInternalServerError, map[string]string{"error": err.Error()})
251252
}
252253

254+
// Count total matching flows in parallel with signature fetching.
255+
var totalCount int64
256+
var countErr error
257+
var wg sync.WaitGroup
258+
wg.Add(1)
259+
go func() {
260+
defer wg.Done()
261+
totalCount, countErr = api.DB.CountFlowsByOpts(c.Request().Context(), opts)
262+
}()
263+
253264
// Collect all unique signature IDs across all flows for a single batch query.
254265
allSigIDs := make([]string, 0)
255266
sigIDSeen := make(map[string]bool)
@@ -305,7 +316,23 @@ func (api *Router) query(c echo.Context) error {
305316
apiResults[i] = res
306317
}
307318

308-
return c.JSON(http.StatusOK, apiResults)
319+
wg.Wait()
320+
if countErr != nil {
321+
slog.Error("Failed to count flows", slog.Any("err", countErr))
322+
return c.JSON(http.StatusInternalServerError, apiError{"Could not count flows. See server logs for details."})
323+
}
324+
325+
page := 1
326+
if opts.Limit > 0 {
327+
page = opts.Offset/opts.Limit + 1
328+
}
329+
330+
return c.JSON(http.StatusOK, map[string]any{
331+
"data": apiResults,
332+
"page": page,
333+
"count": totalCount,
334+
"items_per_page": opts.Limit,
335+
})
309336
}
310337

311338
func (api *Router) getTags(c echo.Context) error {

services/cmd/api/query_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ func (m *mockDB) GetSignaturesBatch(ctx context.Context, ids []string) ([]db.Sur
3535
}
3636
func (m *mockDB) InsertFlows(context.Context, []db.FlowEntry) error { return nil }
3737
func (m *mockDB) CountFlows(context.Context, bson.D) (int64, error) { return 0, nil }
38+
func (m *mockDB) CountFlowsByOpts(context.Context, *db.FindFlowsOptions) (int64, error) {
39+
return 0, nil
40+
}
3841
func (m *mockDB) SetStar(context.Context, string, bool) error { return nil }
3942
func (m *mockDB) GetFlowDetail(context.Context, string) (*db.FlowEntry, error) { return nil, nil }
4043
func (m *mockDB) GetTagList(context.Context) ([]string, error) { return nil, nil }
@@ -108,15 +111,20 @@ func TestQueryExcludeTagsForwarded(t *testing.T) {
108111
assert.Equal(t, []string{"starred"}, mdb.lastOpts.ExcludeTags)
109112
}
110113

111-
// TestQueryResponseIsJSON verifies that /query always returns a JSON array.
114+
// TestQueryResponseIsJSON verifies that /query returns a paginated JSON envelope.
112115
func TestQueryResponseIsJSON(t *testing.T) {
113116
mdb := &mockDB{}
114117
router := newTestRouter(mdb)
115118

116119
rec := doQuery(t, router, `{}`)
117120

118121
assert.Equal(t, http.StatusOK, rec.Code)
119-
var result []json.RawMessage
122+
var result struct {
123+
Data []json.RawMessage `json:"data"`
124+
Page int `json:"page"`
125+
Count int64 `json:"count"`
126+
ItemsPerPage int `json:"items_per_page"`
127+
}
120128
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &result))
121129
}
122130

0 commit comments

Comments
 (0)