fix: entitlement config compare

[chrisgacsal]

Overview

Use jsondiff library to compare JSON strings in order to detect any drifts.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced entitlement configuration validation with improved comparison logic to more accurately detect when entitlement configurations do not match their templates.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The change replaces a direct string equality check with a JSON diff-based comparison when validating entitlement configuration against its template, adding error handling for the diff operation.

Changes

Cohort / File(s)	Change Summary
Entitlement Validation Logic openmeter/subscription/subscriptionview.go	Added jsondiff import. Replaced string equality check (configJSON != lo.FromPtr(ent.Config)) with jsondiff.CompareJSON to perform semantic JSON comparison. Added error handling for diff computation failures and updated validation error messaging to reflect the new comparison method.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly and concisely describes the main change: switching from direct string equality to JSON diff-based comparison for entitlement config validation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/config-cmp

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

openmeter/subscription/subscriptionview.go (1)

162-169: Nice improvement! JSON diff handles semantic equivalence better than string comparison.

Using jsondiff properly handles cases where JSON is semantically equivalent but formatted differently (whitespace, key ordering, etc.). This is definitely an upgrade from direct string comparison.

A couple of small suggestions:

1. Consider validating that ent.Config is not nil before the comparison. Currently, lo.FromPtr(ent.Config) returns an empty string if Config is nil, which might mask a legitimate configuration issue. If nil Config should fail validation, an explicit check would be clearer.

2. Include the diff in the error message for easier debugging. Something like:

   return fmt.Errorf("entitlement %s config does not match template config: %v", s.Entitlement.Entitlement.ID, diff)

🔎 Potential improvements

+		if ent.Config == nil {
+			return fmt.Errorf("entitlement %s config is nil but template expects config", s.Entitlement.Entitlement.ID)
+		}
+
 		diff, err := jsondiff.CompareJSON([]byte(configJSON), []byte(lo.FromPtr(ent.Config)))
 		if err != nil {
 			return fmt.Errorf("failed to compare entitlement with template config for Item %s: %w", s.SubscriptionItem.Key, err)
 		}

 		if len(diff) > 0 {
-			return fmt.Errorf("entitlement %s config does not match template config", s.Entitlement.Entitlement.ID)
+			return fmt.Errorf("entitlement %s config does not match template config: %v", s.Entitlement.Entitlement.ID, diff)
 		}

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c00297 and c569cc8.

⛔ Files ignored due to path filters (2)

• go.mod is excluded by !**/*.mod
• go.sum is excluded by !**/*.sum, !**/*.sum

📒 Files selected for processing (1)

• openmeter/subscription/subscriptionview.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.

Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.

Files:

• openmeter/subscription/subscriptionview.go

⏰ 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). (7)

• GitHub Check: E2E
• GitHub Check: Quickstart
• GitHub Check: Build
• GitHub Check: Lint
• GitHub Check: Code Generators
• GitHub Check: Test
• GitHub Check: Analyze (go)

[tothandras]

is there a more lightweight alternative to jsondiff?