Skip to content

Conversation

@cweidenkeller
Copy link
Contributor

@cweidenkeller cweidenkeller commented Jan 16, 2026

Description

Describe your changes in detail
Extend DB service to select by Finding Name

Motivation and Context

This is a requirement for graph schema

Resolves BED-6893

Why is this change required? What problem does it solve?

How Has This Been Tested?

Wrote integration tests.

Screenshots (optional):

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

Summary by CodeRabbit

  • New Features

    • Added capability to retrieve schema relationship findings by their name, providing an alternative lookup method alongside existing ID-based searches.
  • Tests

    • Added comprehensive test coverage for the new name-based finding retrieval functionality, including success and error handling scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

Walkthrough

A new GetSchemaRelationshipFindingByName method is added to the OpenGraphSchema interface and BloodhoundDB implementation, enabling schema relationship findings to be retrieved by name. Corresponding integration tests and mock implementations are included to support this new retrieval path.

Changes

Cohort / File(s) Summary
Core Database Implementation
cmd/api/src/database/graphschema.go
Added GetSchemaRelationshipFindingByName method to both the OpenGraphSchema interface and BloodhoundDB type for name-based retrieval of schema relationship findings. SQL query structure mirrors existing by-id methods with minor formatting adjustments.
Test Coverage
cmd/api/src/database/graphschema_integration_test.go
New test suite TestGetSchemaRelationshipFindingByName validating successful retrieval by name and error handling for non-existent findings.
Mock Infrastructure
cmd/api/src/database/mocks/db.go
Added mock method and recorder for GetSchemaRelationshipFindingByName following existing gomock patterns used by other database methods.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PR #2207 — Implements the core CRUD operations for schema relationship findings, which this PR extends with name-based lookup functionality.

Suggested labels

enhancement, api

Suggested reviewers

  • kpowderly
  • AD7ZJ

Poem

🐰 A finding by its name at last,
No need to search the ID past,
The database hops with newfound cheer,
Relationships revealed so clear! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a GetRelationshipFindingByName method to the database service.
Description check ✅ Passed The description covers the main change, motivation, testing approach, and includes required checklist items with ticket reference, though some sections are minimal.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
cmd/api/src/database/graphschema_integration_test.go (1)

1779-1852: Prefer assertions that don’t depend on fixed IDs.
If sequences aren’t reset between tests (or other tests run in parallel), expecting ID == 1 can flake. Consider asserting ID > 0 and checking the other fields explicitly.

♻️ Suggested tweak for more resilient assertions
-			if testCase.want.err != nil {
-				assert.ErrorIs(t, err, testCase.want.err)
-			} else {
-				got.CreatedAt = time.Time{}
-				assert.Equal(t, testCase.want.res, got)
-				assert.NoError(t, err)
-			}
+			if testCase.want.err != nil {
+				assert.ErrorIs(t, err, testCase.want.err)
+			} else {
+				got.CreatedAt = time.Time{}
+				assert.Greater(t, got.ID, int32(0))
+				assert.Equal(t, testCase.want.res.SchemaExtensionId, got.SchemaExtensionId)
+				assert.Equal(t, testCase.want.res.RelationshipKindId, got.RelationshipKindId)
+				assert.Equal(t, testCase.want.res.EnvironmentId, got.EnvironmentId)
+				assert.Equal(t, testCase.want.res.Name, got.Name)
+				assert.Equal(t, testCase.want.res.DisplayName, got.DisplayName)
+				assert.NoError(t, err)
+			}

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 648bb17 and 2db9e3e.

📒 Files selected for processing (3)
  • cmd/api/src/database/graphschema.go
  • cmd/api/src/database/graphschema_integration_test.go
  • cmd/api/src/database/mocks/db.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-06T23:12:14.181Z
Learnt from: elikmiller
Repo: SpecterOps/BloodHound PR: 1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.

Applied to files:

  • cmd/api/src/database/graphschema.go
📚 Learning: 2025-12-18T21:50:43.837Z
Learnt from: brandonshearin
Repo: SpecterOps/BloodHound PR: 2165
File: cmd/api/src/database/sourcekinds.go:64-69
Timestamp: 2025-12-18T21:50:43.837Z
Learning: In cmd/api/src/database/sourcekinds.go, the GetSourceKinds query should sort results by name ASC at the database layer to match the UI's alphanumeric sorting behavior and maintain consistency.

Applied to files:

  • cmd/api/src/database/graphschema.go
📚 Learning: 2025-11-25T22:11:53.518Z
Learnt from: LawsonWillard
Repo: SpecterOps/BloodHound PR: 2107
File: cmd/api/src/database/graphschema.go:86-100
Timestamp: 2025-11-25T22:11:53.518Z
Learning: In cmd/api/src/database/graphschema.go, the CreateSchemaEdgeKind method intentionally does not use AuditableTransaction or audit logging because it would create too much noise in the audit log, unlike CreateGraphSchemaExtension which does use auditing.

Applied to files:

  • cmd/api/src/database/graphschema.go
🧬 Code graph analysis (3)
cmd/api/src/database/graphschema.go (3)
cmd/api/src/model/graphschema.go (2)
  • SchemaRelationshipFinding (113-121)
  • SchemaRelationshipFinding (123-125)
cmd/api/src/database/db.go (2)
  • BloodhoundDB (194-197)
  • ErrNotFound (42-42)
cmd/api/src/database/helper.go (1)
  • CheckError (39-45)
cmd/api/src/database/mocks/db.go (2)
cmd/api/src/vendormocks/dawgs/graph/mock.go (2)
  • MockDatabase (38-42)
  • MockDatabaseMockRecorder (45-47)
cmd/api/src/model/graphschema.go (2)
  • SchemaRelationshipFinding (113-121)
  • SchemaRelationshipFinding (123-125)
cmd/api/src/database/graphschema_integration_test.go (4)
cmd/api/src/model/graphschema.go (2)
  • SchemaRelationshipFinding (113-121)
  • SchemaRelationshipFinding (123-125)
cmd/api/src/database/database_integration_test.go (1)
  • IntegrationTestSuite (36-39)
packages/go/graphschema/common/common.go (1)
  • DisplayName (54-54)
cmd/api/src/database/db.go (1)
  • ErrNotFound (42-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build BloodHound Container Image / Build and Package Container
  • GitHub Check: run-analysis
  • GitHub Check: build-ui
  • GitHub Check: run-tests
🔇 Additional comments (3)
cmd/api/src/database/mocks/db.go (1)

2235-2248: LGTM: mock method mirrors existing gomock patterns.
The new mock + recorder pairing is consistent with the rest of the file.

cmd/api/src/database/graphschema.go (2)

60-63: Interface update looks consistent.
The new method addition matches the existing schema-retrieval surface.


663-678: Implementation looks solid and consistent with by-id lookup.
The query and ErrNotFound handling mirror existing patterns.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cweidenkeller cweidenkeller self-assigned this Jan 16, 2026
@cweidenkeller cweidenkeller added the api A pull request containing changes affecting the API code. label Jan 16, 2026
@cweidenkeller cweidenkeller deleted the BED-6893 branch January 16, 2026 20:15
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api A pull request containing changes affecting the API code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants