Skip to content

fix: Orm WithContext function#979

Merged
hwbrzzl merged 2 commits intov1.15.xfrom
bowen/fix-orm-with-context-1
Mar 29, 2025
Merged

fix: Orm WithContext function#979
hwbrzzl merged 2 commits intov1.15.xfrom
bowen/fix-orm-with-context-1

Conversation

@hwbrzzl
Copy link
Contributor

@hwbrzzl hwbrzzl commented Mar 29, 2025

📑 Description

  1. The Gorm instance should not be modified directly, we need to call the WithContext function to get a new Gorm instance to avoid concurrence.
  2. Optimize db pool configuration.

✅ Checks

  • Added test cases for my code

Copilot AI review requested due to automatic review settings March 29, 2025 10:38
@hwbrzzl hwbrzzl requested a review from a team as a code owner March 29, 2025 10:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request updates the ORM context handling by replacing direct modifications of the Gorm instance with proper usage of the WithContext method and optimizes DB pool configuration. Key changes include:

  • Removing the QueryWithSetContext mocks and introducing QueryWithContext mocks.
  • Updating the Orm and Query functions to use WithContext for better concurrency safety.
  • Refactoring Gorm initialization by moving logic to BuildGorm and removing the old Gorm file.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
mocks/database/orm/QueryWithSetContext.go Removed obsolete mock implementation for setting context
mocks/database/orm/QueryWithContext.go Added new mock implementation for WithContext
database/orm/orm.go Updated Query and WithContext functions for proper context handling
database/gorm/query.go Refactored context handling in Query and improved Gorm pool configuration
database/gorm/gorm.go Removed outdated Gorm initialization code
contracts/database/orm/orm.go Updated interface from QueryWithSetContext to QueryWithContext
Comments suppressed due to low confidence (1)

database/orm/orm.go:121

  • Comparing r.ctx with context.Background() directly might lead to unintended behavior. Consider using a nil check or a different mechanism to verify if the context has been explicitly set.
if r.ctx != context.Background() {

@codecov
Copy link

codecov bot commented Mar 29, 2025

Codecov Report

Attention: Patch coverage is 62.12121% with 25 lines in your changes missing coverage. Please review.

Please upload report for BASE (v1.15.x@03bfb25). Learn more about missing BASE report.

Files with missing lines Patch % Lines
database/gorm/query.go 59.67% 18 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             v1.15.x     #979   +/-   ##
==========================================
  Coverage           ?   68.63%           
==========================================
  Files              ?      218           
  Lines              ?    18944           
  Branches           ?        0           
==========================================
  Hits               ?    13003           
  Misses             ?     5278           
  Partials           ?      663           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hwbrzzl hwbrzzl enabled auto-merge (squash) March 29, 2025 10:52
@hwbrzzl hwbrzzl disabled auto-merge March 29, 2025 10:53
@hwbrzzl hwbrzzl merged commit de030af into v1.15.x Mar 29, 2025
11 checks passed
@hwbrzzl hwbrzzl deleted the bowen/fix-orm-with-context-1 branch March 29, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants