Dedupe observable/entity/indicator get-by-name/value endpoints (item #5, third slice) - #1335
Merged
Merged
Conversation
…dies Third slice of item #5's router dedup. The GET / (find-by-value/name) endpoint on each router built a lookup dict, called the model's find(), raised 404 if nothing matched, called get_tags(), then ran the same RBAC read-permission check (admin/RBAC-disabled bypass, else check has_permissions and raise 403) before returning. Identical logic across all three, save for the not-found message wording and the query parameter name itself (value for observable, name for entity/ indicator) -- the latter stays in each router's own function signature since it's part of the OpenAPI contract, not something to unify. Add core/web/apiv2/crud.py::get_by_lookup(), taking the base type, the already-built lookup dict, and the not-found message; it does the find() call plus the shared not-found/tags/RBAC logic. Each router's `get` endpoint still builds its own lookup dict (preserving the exact per-type "only include type in the query if set" behavior) and passes it straight through -- no behavior change, verified via a byte-for-byte OpenAPI diff (md5 match, throwaway pre/post build) same as the earlier two slices. No incidental bugs found this time -- all three endpoints already agreed on status codes and message format, unlike the tag/delete endpoints fixed in #1333/#1334.
Merged
6 tasks
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
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.
Summary
Third incremental slice of item #5 (observable/entity/indicator router
triplication), following #1333 (tag endpoint) and #1334 (details/delete
endpoints). This one covers the
GET /(find-by-value/name) endpoint.Each router's
getendpoint built a lookup dict, called the model'sfind(), raised 404 if nothing matched, calledget_tags(), then ran theidentical RBAC read-permission check (admin/RBAC-disabled bypass, else
has_permissionscheck raising 403) before returning. The only realdifferences were the not-found message wording and the query parameter
name itself —
valuefor observable,namefor entity/indicator.Change
Added
core/web/apiv2/crud.py::get_by_lookup(), taking the base type, analready-built lookup dict, and the not-found message; it performs the
find()call plus the shared not-found/tags/RBAC logic. Each router'sgetendpoint still builds its own lookup dict — preserving the exactper-type "only include
typein the query if set" behavior and, crucially,keeping the differing query parameter name (
value/name) in its ownfunction signature, since that's part of the OpenAPI contract and not
something to unify.
OpenAPI spec verified byte-identical (md5 match between a throwaway
pre-change build and the post-change build, same method used for the
previous two slices).
No incidental bugs found this time — all three endpoints already agreed
on status codes and message format, unlike the tag/delete endpoints fixed
in #1333/#1334.
Test plan
tests/apiv2suite: 201/203 pass (same 2 known pre-existingfailures in
tests/apiv2/tasks.py, unrelated) — includes theexisting get-by-value/name and RBAC-forbidden tests for all three
routers, unchanged.
tests/schemas(190/190) andtests/core_tests(31/31) passunchanged.
ty check(core+yetictl and plugins jobs): 0 errors.ruff check/ruff format --check: clean.Scope note
The one remaining item #5 candidate from the original list — the
searchendpoint body — is the most entangled: entity/indicator support
filter_aliasesand a separateget_multipleendpoint that observablehas neither of. Normalizing that safely needs a real design decision
(add the missing fields to observable's search, changing its OpenAPI
shape — additive but deliberate) rather than a blind extraction, so I'd
treat it as its own follow-up rather than bundling it here.