Refactor database handling and enhance testing infrastructure#138
Merged
Refactor database handling and enhance testing infrastructure#138
Conversation
Introduce server/database package with sqlc-generated queries, DB connection and Store. Remove legacy server/sqlite implementation, switch to database/sql with modernc.org/sqlite, and update imports and go.work accordingly
- Remove pkg/models and update client and server code to use server/database.Flag - Introduce DB Store and Runner; wire store into startup, API handlers and core loops - Update sqlc config, queries and DB models (JSON tags and type overrides) - Add API/websocket request types and minor handler/route refactors
Adjust demo/scripts/setup.sh to use the demo-relative venv and flagchecker paths and remove the tests dir cd. Update justfile working directories and server bin dir to match the new layout, and add a generate target to run sqlc.
Replace the old client-test target with server-test running in cookiefarm/server. Use gotestsum with testdox, generate coverage.out and coverage.html, open the report, and send a desktop notification on successful test completion. Update the nearby comment to "Test client binaries".
Add the following linters: - dupl - zerologlint - whitespace
Introduce FlagCollector, mappers, queries and store helpers Add schema.sql and extensive unit tests for connection, queries, mapper, collector and store, and include generated coverage report
Convert TeamID fields to int64 across server and client, adding casts where needed. Move API request types into pkg/models and update client usage. Add a buffered FlagCollector singleton to batch DB writes with Start/Stop/Flush methods. Embed DB schema and update sqlc config; add mapper helpers and CountFlags query. Misc: change DSN to cookiefarm.db, adjust route handlers, and update some logging and module/sum entries.
Update server-test in justfile to create ./coverage and write coverage.out and coverage.html into that directory. Remove the legacy cookiefarm/server/coverage.html tracked file.
Add comprehensive coverage_test.go to exercise DB transaction paths, QueryContext/ExecContext error branches, Store.WithTx, NewDB edge cases, and FlagCollector timer/flush behaviors (including buffer-overflow drop). Introduce rowsAffectedErrDB and blockingFailStore helpers to simulate error and blocking conditions. Set fc.running = false in FlagCollector.Stop so the background goroutine observes the stopped state before stats are snapshotted, avoiding a race.
akiidjk
requested review from
KronosPNG,
giovanni-iannaccone,
jose-sgariglia and
vympel7
akiidjk
added
documentation
Collaborator
Author
|
I hope I haven't forgotten anything. @KronosPNG don't forget to fix/add the rules for SonarQube |
KronosPNG
approved these changes
Mar 3, 2026
Collaborator
Pushed on feature/sonarqube_ruleset_tweak1 the new ruleset, for now coverage is ignored until we make proper tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request migrates the old database implementation based on raw queries to a new implementation using sqlc, improving maintainability without losing performance. I also set up a command to test the server code using gotestsum.
Some client code was changed to consistently use the Flag struct from the server/database package instead of the previous ClientData struct from models. This is not ideal because some server components are called from the client codebase, but it is a temporary constraint caused by sqlc. I plan to improve this later. This change affects all major flag-handling logic, including parsing, submitting, and communication via HTTP and WebSockets. Additionally, there are some linter improvements and dependency updates.
I also started implementing a dependency injection pattern for the database, which is reflected in the API handling layer as well. The goal is to remove all global instances and variables across the codebase.
The ANALYSIS.md is slopped but reasonably good.
The tests are also slopped and not satisfactory. Next time I will write them manually. I forgot to write tests while migrating functions, which is my mistake and will not happen again.
Test coverage is approximately 89.5%.
Refactoring to use
server/database.Flagfor flag dataReplaced all instances of
models.ClientDatawithdatabase.Flagthroughout the client codebase, including in function signatures, struct fields, and channels in files such asapi.go,exploit.go,parse.go,run.go,submitter.go, and websocket-related files. This ensures a single source of truth for flag data structures and streamlines integration with the server. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Updated imports across client files to reference
server/databaseinstead ofmodelsfor flag-related functionality. [1] [2] [3] [4] [5] [6]Project structure and configuration
./server/databaseinstead of./server/sqlite, reflecting the new location of shared data structures.Linter and code quality improvements
dupl,zerologlint, andwhitespacelinters to.golangci.ymlto improve code quality and catch duplication and formatting issues. [1] [2]Dependency updates
golang.org/x/exp, and updated several indirect dependencies ingo.modandgo.work.sumfiles for various client and logger packages, ensuring compatibility and access to new features. [1] [2] [3] [4] [5] [6]Minor logging improvement
run.goby adding a message to clarify when a command is prepared for execution.Last things
AddFlagfunctions, sqlc with SQLite not support batch insert native, so for now we are obligated to use a for loop with an add inside (view for example line176inhandling_api.go)