docs: document full response contract for POST /api/v1/locations - #91
Open
dev-aditya-hub wants to merge 1 commit into
Open
docs: document full response contract for POST /api/v1/locations#91dev-aditya-hub wants to merge 1 commit into
dev-aditya-hub wants to merge 1 commit into
Conversation
The README listed only 201, 400, and 415 for the ingest endpoint. The handler also returns 401 (requireAuth middleware), 429 (per-driver rate limiter), and 500 (SaveLocation failure), so a client written against these docs has no way to know it must handle them. Document the three missing codes, note the bearer-token and rate-limit requirements, and fix the curl examples, which as written returned 401 (no Authorization header) and 400 (hard-coded timestamp outside the five-minute skew window) rather than the responses they claimed to show. Closes OneBusAway#73
|
dev-aditya-hub seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughThe README now documents JWT authentication, per-driver rate limiting, additional error responses, persistence-failure behavior, and authenticated request examples for location ingest. ChangesLocation ingest documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 157-158: Update handlePostLocation to validate that the
authenticated JWT includes the required driver role before accepting or writing
location data, while preserving the existing sub validation; alternatively,
revise the README ingest-endpoint documentation to accurately state that
authentication alone is sufficient. Ensure the implementation and documentation
consistently describe the enforced access requirements.
- Around line 160-172: Update VehicleRateLimiter.Allow so that when the
tracked-driver capacity reaches 10,000 keys, newly untracked driver keys are
rejected or rate-limited rather than returning true. Preserve the existing
five-second allowance for tracked keys and normal behavior while capacity
remains available.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+157
to
+158
| The ingest endpoint requires a driver JWT (`Authorization: Bearer <token>`) | ||
| and performs strict request validation before writing data: |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 12 \
'handlePostLocation|requireAuth|adminMiddleware|driver|role|claims' \
--glob '*.go' .Repository: OneBusAway/vehicle-positions
Length of output: 50385
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- route wiring ---'
rg -n -C 8 'func newMux|handlePostLocation|requireAuth|requireAdmin|/api/v1/locations' --glob '*.go' .
printf '%s\n' '--- authentication implementation ---'
rg -n -C 14 'func requireAuth|func requireAdmin|generateJWT|claimsKey' --glob '*.go' .
printf '%s\n' '--- README context ---'
sed -n '150,165p' README.mdRepository: OneBusAway/vehicle-positions
Length of output: 50384
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
files = [p for p in Path(".").glob("*.go")]
text = "\n".join(p.read_text() for p in files)
route = re.search(
r'(?s)(?:Handle|Methods|Path|Route|Get|Post)\([^;]{0,500}/api/v1/locations[^;]{0,500}',
text,
)
print("location route references:")
for i, line in enumerate(text.splitlines(), 1):
if "/api/v1/locations" in line or "handlePostLocation" in line:
print(f"{i}: {line}")
print("\nrole checks near location route:")
lines = text.splitlines()
for i, line in enumerate(lines):
if "/api/v1/locations" in line:
lo, hi = max(0, i - 8), min(len(lines), i + 4)
print("\n".join(f"{j+1}: {lines[j]}" for j in range(lo, hi)))
PYRepository: OneBusAway/vehicle-positions
Length of output: 12869
Enforce the driver role or update the documentation. POST /api/v1/locations uses authentication only. handlePostLocation reads sub but does not check role, so any valid JWT with a non-empty sub can submit locations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 157 - 158, Update handlePostLocation to validate that
the authenticated JWT includes the required driver role before accepting or
writing location data, while preserving the existing sub validation;
alternatively, revise the README ingest-endpoint documentation to accurately
state that authentication alone is sufficient. Ensure the implementation and
documentation consistently describe the enforced access requirements.
Comment on lines
160
to
+172
| - `Content-Type` must be `application/json` (charset parameters are allowed). | ||
| - The request body must contain exactly one JSON object. | ||
| - Unknown JSON fields are rejected. | ||
| - Standard payload validation still applies (`vehicle_id`, coordinates, timestamp). | ||
| - Reports are rate limited to one per five seconds per driver. | ||
|
|
||
| Response codes: | ||
|
|
||
| - `201 Created` — location accepted and persisted. | ||
| - `400 Bad Request` — invalid JSON or payload validation failure. | ||
| - `401 Unauthorized` — missing, malformed, or expired bearer token. | ||
| - `415 Unsupported Media Type` — non-JSON `Content-Type`. | ||
| - `429 Too Many Requests` — driver exceeded the ingest rate limit. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 12 \
'handlePostLocation|VehicleRateLimiter|DisallowUnknownFields|StatusBadRequest|StatusUnsupportedMediaType|StatusTooManyRequests|json\.Decoder' \
--glob '*.go' .Repository: OneBusAway/vehicle-positions
Length of output: 50385
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- matching files ---'
rg -l --glob '*.go' 'handlePostLocation|VehicleRateLimiter' .
printf '%s\n' '--- matching declarations and uses ---'
rg -n --glob '*.go' '^(type|func) .*?(VehicleRateLimiter|handlePostLocation)|VehicleRateLimiter|handlePostLocation' .Repository: OneBusAway/vehicle-positions
Length of output: 3555
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline handlers.go
ast-grep outline ratelimit.go
ast-grep outline handlers_test.goRepository: OneBusAway/vehicle-positions
Length of output: 4901
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- handlers.go ---'
sed -n '1,140p' handlers.go
printf '%s\n' '--- ratelimit.go ---'
sed -n '1,95p' ratelimit.go
printf '%s\n' '--- README.md ---'
sed -n '150,180p' README.md
printf '%s\n' '--- validation and response tests ---'
sed -n '180,290p' handlers_test.go
sed -n '470,610p' handlers_test.go
printf '%s\n' '--- rate-limit and identity tests ---'
sed -n '606,755p' handlers_test.goRepository: OneBusAway/vehicle-positions
Length of output: 24803
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- authentication and route wiring ---'
rg -n -C 18 'func authMiddleware|func requireAuth|StatusUnauthorized|Authorization|jwt.Parse|POST /api/v1/locations' --glob '*.go' .
printf '%s\n' '--- complete README contract and examples ---'
sed -n '160,215p' README.md
printf '%s\n' '--- static contract verifier ---'
python3 - <<'PY'
from pathlib import Path
handlers = Path("handlers.go").read_text()
ratelimit = Path("ratelimit.go").read_text()
checks = {
"JSON media type with parameters": 'mime.ParseMediaType(contentType)' in handlers,
"unknown fields rejected": 'decoder.DisallowUnknownFields()' in handlers,
"trailing JSON rejected": 'decoder.Decode(new(json.RawMessage))' in handlers,
"payload validation before save": 'if err := loc.validate(); err != nil' in handlers and
handlers.index('if err := loc.validate(); err != nil') < handlers.index('store.SaveLocation'),
"rate-limit key is JWT subject": 'rl.Allow(loc.DriverID)' in handlers and 'loc.DriverID = sub' in handlers,
"429 mapping": 'http.StatusTooManyRequests' in handlers,
"save precedes tracker update": handlers.index('store.SaveLocation') < handlers.index('tracker.Update'),
"capacity bypass exists": 'allowing untracked key' in ratelimit and
'return true' in ratelimit[ratelimit.index('if len(vrl.limiters) >= maxTrackedRates'):],
}
for name, result in checks.items():
print(f"{name}: {'PASS' if result else 'FAIL'}")
PYRepository: OneBusAway/vehicle-positions
Length of output: 48530
Enforce the rate limit when the limiter reaches capacity.
VehicleRateLimiter.Allow permits every new driver key after 10,000 keys are tracked. A new driver can therefore bypass the documented five-second limit. Reject or rate-limit untracked keys instead of returning true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 160 - 172, Update VehicleRateLimiter.Allow so that
when the tracked-driver capacity reaches 10,000 keys, newly untracked driver
keys are rejected or rate-limited rather than returning true. Preserve the
existing five-second allowance for tracked keys and normal behavior while
capacity remains available.
Member
Code reviewNo issues found. Checked for bugs and project convention compliance. 🤖 Generated with Claude Code |
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
3 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.
The README lists 201, 400, and 415 for the ingest endpoint, but the handler also returns 401 from
requireAuth, 429 from the per-driver rate limiter, and 500 whenSaveLocationfails. A client written against these docs has no way to know it has to handle those.This documents the three missing codes and notes the bearer-token and rate-limit requirements alongside the existing validation rules.
I also fixed the curl examples in the same section, since none of them currently produce the response they claim to demonstrate: they send no
Authorizationheader, so they all return 401, and the hard-coded1752566400timestamps are outside the five-minute skew window enforced inhandlers.go, so they would return 400 even with a valid token. They now use$TOKENand$(date +%s), and I added a 401 example.Docs only — no code changes.
Closes #73
Summary by CodeRabbit
401,429, and500responses.