Add Ruby Sinatra PostgreSQL quickstart application - #1
Nsanjayboruds wants to merge 9 commits into
Conversation
|
hi @Nsanjayboruds please make sure DCO check passes by using -s flag while writing a commit message |
Signed-off-by: Nsanjayboruds <nishantborude555@gmail.com>
ee56404 to
a9748a5
Compare
dhananjay6561
left a comment
There was a problem hiding this comment.
Thanks for putting this together Nishant, the breadth here is genuinely nice: search, filtering, pagination, a dependent reviews resource, an analytics aggregate, and a scripted record run of 20 requests that covers both happy and error paths. The SQL is parameterized throughout, and the sort_by and order inputs are mapped through allowlists before they touch the query, which is exactly the right way to keep it safe from injection.
One blocker before this can go in: app.rb does not currently parse. Running ruby -c app.rb reports syntax error, unexpected end-of-input at line 886. It looks like two versions of the app got merged into a single file, so almost every route is defined twice and the top section (the first health route plus the helpers block) is spliced together mid block. As it stands the app will not boot, so the record and replay flow cannot run.
I left inline notes on the specific spots. The main fix is to regenerate app.rb from the one intended version (the refactored routes that use halt_json and the helper methods read clean and complete), delete the older duplicate blocks, and confirm ruby -c app.rb passes and the container actually starts before pushing again. A few smaller design notes are inline too, none of those are blocking. Happy to take another look as soon as it is regenerated.
Signed-off-by: Nsanjayboruds <nishantborude555@gmail.com>
…g, lockfile) Signed-off-by: Nsanjayboruds <nishantborude555@gmail.com>
Thanks so much for the detailed review, @dhananjay6561 ! I've just pushed the updates to address all of your points:
|
dhananjay6561
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround Nishant, this is a big step up. I went back through it and confirmed the main points are sorted: app.rb parses cleanly now (ruby -c passes) and every route is defined exactly once, so the duplicated merge blocks are gone. The old validate_numeric_id calls are all pointing at valid_positive_integer?, the schema runs once at startup instead of on every request, non object JSON bodies are guarded with the is_a?(Hash) check, the raw database error is no longer handed back to clients, and Gemfile.lock is out of .gitignore.
The rest reads well too. SQL stays parameterized, the sort_by and order inputs go through allowlists before hitting the query, the ISBN uniqueness check is backed by the database constraint with a rescue so the race is covered, and the validation across books and reviews lines up with the negative paths the record script exercises.
Approving. I left one tiny optional note on the health endpoint, nothing blocking. Nice work getting this cleaned up.
Signed-off-by: Nsanjayboruds <nishantborude555@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new Sinatra + PostgreSQL “Books + Reviews” sample application intended for Keploy quickstart documentation, including Docker-based setup and an automated recording script that generates a 20-request scenario.
Changes:
- Introduces a Sinatra API implementing books, dependent reviews, and an analytics endpoint, backed by PostgreSQL.
- Adds Docker assets (Dockerfile, compose files) and database initialization SQL for an easy, containerized run.
- Adds Keploy configuration plus an automation script (
run-keploy.sh) to record a deterministic 20-call traffic flow.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| sinatra-postgres-quickstart/app.rb | Implements the Sinatra API, DB access, validation, and analytics endpoint. |
| sinatra-postgres-quickstart/init.sql | Creates tables/indexes and seeds initial sample data for Postgres. |
| sinatra-postgres-quickstart/docker-compose.yml | Defines app + Postgres services for the main Docker run. |
| sinatra-postgres-quickstart/docker-compose.yml.test | Adds a separate compose setup intended for test usage. |
| sinatra-postgres-quickstart/Dockerfile | Builds the Ruby app container and starts Puma. |
| sinatra-postgres-quickstart/Gemfile | Declares Ruby/Sinatra/Postgres dependencies. |
| sinatra-postgres-quickstart/config.ru | Rack entrypoint to run the Sinatra app. |
| sinatra-postgres-quickstart/run-keploy.sh | Automates Keploy record mode and generates 20 API calls. |
| sinatra-postgres-quickstart/keploy.yml | Adds a Keploy configuration file for record/replay defaults. |
| sinatra-postgres-quickstart/README.md | Documents setup, endpoints, and the Keploy record/replay workflow. |
| sinatra-postgres-quickstart/.gitignore | Ignores Keploy output and common Ruby/local artifacts. |
Suppressed comments (1)
sinatra-postgres-quickstart/app.rb:61
- On schema init failure, the app only prints a message and continues, which can leave the container "running" but non-functional. Consider failing fast and adding a next-step hint (e.g., check DB_HOST/DB_PORT and Postgres readiness) to make the failure actionable.
rescue PG::Error => e
puts "Failed to initialize database schema: #{e.message}"
end
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…check Signed-off-by: Nsanjayboruds <nishantborude555@gmail.com>
…dType, correct spelling) Signed-off-by: Nsanjayboruds <nishantborude555@gmail.com>
Thanks so much for the approval and the helpful review! I actually went ahead and implemented that optional fix for /health too. It now uses its own raw PG.connect check so it accurately returns a 503 instead of falling back to 500 when the database is entirely down. |
|
dhananjay6561
left a comment
There was a problem hiding this comment.
Strict review of the Sinatra + PostgreSQL quickstart. Overall solid (parameterized queries throughout, sensible validation and error handlers), but a few real issues: two unhandled crash paths from type-confused params (array-typed q/author, non-string review fields), a dead nil branch that makes unrated books report a 0 rating, schema duplicated across app.rb and init.sql (already drifted), and some port/replay-command inconsistencies between the script, README, and compose files. Details inline.
- Add is_a?(String) guards on params['q'] and params['author'] to prevent NoMethodError when Rack parses bracketed keys as Arrays - Add is_a?(String) type enforcement for reviewer and comment fields in validate_review_payload! - Fail fast (exit 1) on schema init PG::Error instead of swallowing it - Remove redundant ALTER TABLE lines in SCHEMA_SQL (drifted from init.sql) - Fix serialize_book to return nil avg_rating/review_count for unrated books instead of masking with COALESCE(0) - Floor total_pages to minimum 1 in books and reviews list endpoints - Fix replay hint in run-keploy.sh to include --cid-type docker-compose and align delay flags with keploy.yml - Fix README local health check port: APP_HOST_PORT -> PORT (8000) - Align docker-compose.yml.test container name to ruby-books-app
|
Hey @dhananjay6561 , great catch on all of these — addressed everything: 1.params['q'] / params['author'] type crash — Added is_a?(String) guards so bracketed Rack params parsed as Arrays return a clean 400 instead of raising NoMethodError |
dhananjay6561
left a comment
There was a problem hiding this comment.
A few things I ran into going through this, mostly input-validation edge cases and a couple of setup steps. Notes inline.
- Escape LIKE wildcards (% and _) in q/author search params - Normalize empty ISBN string to nil in ensure_unique_isbn! and before binding in PUT/PATCH to avoid UNIQUE index false collisions - Fix PUT to coalesce omitted isbn/published_year from existing row instead of silently NULLing them - Wrap PATCH in a transaction with SELECT FOR UPDATE to prevent lost updates and 500 on concurrent delete - Add --frozen to bundle install in Dockerfile so drifted Gemfile.lock fails the build instead of being silently rewritten - Add .dockerignore to exclude .git/, keploy/, .env*, run-keploy.sh
- Add max-length 255 validation for title and author fields in validate_book_payload! to prevent PG::StringDataRightTruncation 500 - Cap min_year/max_year with max: current_year+1 to match write-path and prevent PG::NumericValueOutOfRange on huge values - Fix pg_isready healthcheck to use -d booksdb in both docker-compose files so app waits for booksdb to be fully ready, not just postgres
|
Hey @dhananjay6561, addressed all the new feedback: title/author max-length — Added 255-char validation checks for title and author in validate_book_payload! (alongside the existing ISBN check), so over-length values now return a clean 400 instead of hitting PG::StringDataRightTruncation → 500 min_year/max_year upper bound — Added max: current_year + 1 to both parse_integer_param calls, matching the write-path cap. Out-of-range values like ?min_year=9999999999 will now return 400 instead of raising PG::NumericValueOutOfRange → 500 pg_isready healthcheck — Changed to pg_isready -U postgres -d booksdb in both docker-compose.yml and docker-compose.yml.test, so the healthcheck only passes once booksdb is fully ready and init.sql has finished running — not just when the postgres process is accepting connections |
Description
This PR introduces a comprehensive Ruby (Sinatra) + PostgreSQL sample application for the Keploy quickstart guides.
As discussed in keploy/docs#773, this application goes beyond a basic CRUD structure to provide a much more realistic scenario that better showcases Keploy's capabilities in a real-world environment.
Key Features Implemented
BooksAPI now supports complex query patterns including search, filtering (by author/year), sorting, and pagination.Reviewsresource to demonstrate how Keploy handles related database tables and foreign keys./analytics/books/top-rated) to showcase complex SQL aggregations.run-keploy.sh):docker-compose.ymlfor seamless containerized execution, along with local setup instructions in theREADME.md.Related PR
How to Test
sinatra-postgres-quickstart./keploydirectory.