fix: meter delete in tx

[chrisgacsal]

Overview

Make meter delete to be atomic operation by running it in database transaction alongside the read operations used for lookup.

Greptile Summary

The PR makes meter deletion checks, soft deletion, and the post-delete lookup share one PostgreSQL transaction. It also adds integration coverage for successful commit and rollback when event publication fails, but publishing from inside the transaction permits an acknowledged event to outlive a later database commit failure.

• Makes GetMeterByIDOrSlug transaction-aware.
• Wraps the complete DeleteMeter service flow in a transaction.
• Adds commit and publisher-error rollback tests.

Confidence Score: 4/5

This PR should not merge until deletion events are prevented from becoming externally visible before the corresponding database deletion is durably committed.

DeleteMeter now sends an acknowledged Kafka event from inside the transaction callback, so a later PostgreSQL commit failure leaves consumers with a deletion event for a meter that remains active.

Files Needing Attention: openmeter/meter/service/manage.go

Important Files Changed

Filename	Overview
openmeter/meter/adapter/meter.go	Makes point lookups use the transaction-bound adapter when a transaction is present while preserving nontransactional behavior otherwise.
openmeter/meter/service/manage.go	Makes deletion database operations atomic, but publishes the deletion event before commit and can therefore emit a phantom deletion.
openmeter/meter/service/manage_test.go	Covers successful commit and rollback on publication failure, but not the irreconcilable successful-publication followed by failed-database-commit ordering.

Sequence Diagram

sequenceDiagram
    participant S as ManageService
    participant DB as PostgreSQL transaction
    participant K as Kafka
    participant C as Consumer
    S->>DB: Soft-delete meter
    S->>DB: Read deleted meter
    S->>K: Publish meter.deleted
    K-->>S: Acknowledge event
    K-->>C: Event becomes observable
    S->>DB: Commit transaction
    alt Commit fails
        DB-->>S: Rollback/error
        Note over K,C: Published event cannot be rolled back
    end

Loading

Prompt To Fix All With AI

### Issue 1
openmeter/meter/service/manage.go:150-152
**Deletion event precedes commit**

When Kafka acknowledges the deletion event but the enclosing database transaction subsequently fails to commit, consumers receive a deletion event for a meter that remains active in PostgreSQL. Publishing inside the transaction callback cannot make the Kafka message atomic with the later database commit.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix: meter delete in tx" | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Meter lookup now uses a transaction-scoped repository. DeleteMeter now performs deletion and event publication in one transaction. Tests verify commit on success and rollback on publisher failure.

Meter transaction handling

Layer / File(s)	Summary
Transactional meter lookup openmeter/meter/adapter/meter.go	GetMeterByIDOrSlug runs database lookup and entity mapping through entutils.TransactingRepo. Existing error handling remains unchanged.
Transactional meter deletion and rollback openmeter/meter/service/manage.go, openmeter/meter/service/manage_test.go	DeleteMeter uses transaction.RunWithNoValue for validation, soft deletion, re-fetching, and event publication. Tests verify committed deletion after successful publication and rollback after publication failure.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant DeleteMeter
  participant Database
  participant EventPublisher
  Client->>DeleteMeter: delete meter
  DeleteMeter->>Database: validate and soft-delete meter
  Database-->>DeleteMeter: deleted meter state
  DeleteMeter->>EventPublisher: publish deletion event
  EventPublisher-->>DeleteMeter: success or error
  DeleteMeter->>Database: commit or roll back transaction
  DeleteMeter-->>Client: result or publication error

Loading

Possibly related PRs

• openmeterio/openmeter#4835: Modifies related meter deletion validation and transaction behavior.

Suggested reviewers: tothandras, galexihu

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: making meter deletion transactional.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/meter-delete

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@openmeter/meter/service/manage.go`:
- Around line 86-157: Extract the meter deletion workflow from the anonymous
callback in DeleteMeter into a private named helper method, including lookup,
validation, feature and entitlement checks, deletion, re-fetch, and event
publication. Keep DeleteMeter’s transaction.RunWithNoValue callback as a thin
call-through to that helper, preserving the existing context, error handling,
and behavior.
- Around line 150-154: Move the meter-deleted event publishing out of
DeleteMeter’s transaction: keep the soft delete and event construction within
the existing transaction, then publish only after a successful commit using the
established RunInNewTransaction post-commit pattern or outbox flow. Ensure
transaction rollback or commit failure cannot publish meterDeletedEvent, while
preserving the existing publish error propagation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: db3df8ce-eed6-44e8-ab49-45ce41c66c6c

📥 Commits

Reviewing files that changed from the base of the PR and between b079b6c and 78ef4a9.

📒 Files selected for processing (3)

• openmeter/meter/adapter/meter.go
• openmeter/meter/service/manage.go
• openmeter/meter/service/manage_test.go