Skip to content

feat: [#651] optimize queue database driver#1054

Merged
hwbrzzl merged 1 commit intomasterfrom
bowen/#651-1
May 31, 2025
Merged

feat: [#651] optimize queue database driver#1054
hwbrzzl merged 1 commit intomasterfrom
bowen/#651-1

Conversation

@hwbrzzl
Copy link
Contributor

@hwbrzzl hwbrzzl commented May 31, 2025

📑 Description

Closes goravel/goravel#651

This pull request refactors the queue system by removing the JobRecord interface and its associated mock implementation, consolidating functionality into the DatabaseJobRecord struct. It also introduces a new method, GetJobStorer, to enhance job management. The changes primarily focus on simplifying the codebase, improving naming consistency, and updating tests to reflect these modifications.

Codebase Simplification:

  • Removed the JobRecord interface and its mock implementation (mocks/queue/JobRecord.go) and consolidated its methods (Increment and Touch) into the DatabaseJobRecord struct (queue/driver_database_job.go). [1] [2] [3]
  • Replaced references to DatabaseJob with DatabaseJobRecord throughout the codebase, including in the database driver and related tests (queue/driver_database.go, queue/driver_database_job.go, queue/driver_database_job_test.go, queue/driver_database_test.go). [1] [2] [3] [4] [5]

New Functionality:

  • Added a GetJobStorer method to the Queue interface and its mock implementation to facilitate job storage management (contracts/queue/queue.go, mocks/queue/Queue.go, queue/application.go). [1] [2] [3]

Naming Consistency:

  • Updated variable and type names for clarity, such as renaming DatabaseJob to DatabaseJobRecord and updating associated method calls and test assertions (queue/driver_database_job.go, queue/driver_database_test.go). [1] [2] [3]

Documentation and Comments:

  • Fixed minor grammatical issues in comments, such as correcting "get job" to "gets job" in the Queue interface (contracts/queue/queue.go).

✅ Checks

  • Added test cases for my code

@codecov
Copy link

codecov bot commented May 31, 2025

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.24%. Comparing base (28d87a5) to head (53ba283).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
queue/application.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1054      +/-   ##
==========================================
+ Coverage   71.07%   71.24%   +0.17%     
==========================================
  Files         178      178              
  Lines       12446    12548     +102     
==========================================
+ Hits         8846     8940      +94     
- Misses       3211     3217       +6     
- Partials      389      391       +2     

☔ 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 marked this pull request as ready for review May 31, 2025 02:07
Copilot AI review requested due to automatic review settings May 31, 2025 02:07
@hwbrzzl hwbrzzl requested a review from a team as a code owner May 31, 2025 02:07
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 PR simplifies the queue database driver by removing the JobRecord interface and consolidating job logic into DatabaseJobRecord; it also updates tests and adds a new GetJobStorer method to improve job management.

  • Consolidated JobRecord methods into DatabaseJobRecord and removed the interface and its mock
  • Renamed references from DatabaseJob to DatabaseJobRecord and updated corresponding tests
  • Introduced GetJobStorer on the Queue interface and in the application, along with a mock implementation

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
queue/driver_database_test.go Tests updated to use DatabaseJobRecord instead of DatabaseJob
queue/driver_database_job.go Replaced DatabaseJob struct and methods with DatabaseJobRecord
queue/driver_database.go Updated Pop and Push to work with DatabaseJobRecord
queue/application.go Added GetJobStorer method
mocks/queue/Queue.go Added mock implementation for GetJobStorer
mocks/queue/JobRecord.go Removed obsolete JobRecord mock
contracts/queue/queue.go Added GetJobStorer to the Queue interface and fixed comments
contracts/queue/job.go Removed the now-unused JobRecord interface
Comments suppressed due to low confidence (3)

queue/driver_database.go:53

  • [nitpick] Rename the local variable from databaseJob to jobRecord to align with its type name and improve clarity.
var databaseJob DatabaseJobRecord

queue/driver_database.go:105

  • [nitpick] Consider renaming the variable job to jobRecord for consistency with the struct name.
job := DatabaseJobRecord{

// GetJobs get all jobs
// GetJobs gets all jobs
GetJobs() []Job
// GetJobStorer gets job storer
Copy link

Copilot AI May 31, 2025

Choose a reason for hiding this comment

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

[nitpick] Update the GoDoc comment to follow conventions, e.g., "// GetJobStorer returns the job storer."

Copilot uses AI. Check for mistakes.
return r.jobStorer.All()
}

func (r *Application) GetJobStorer() queue.JobStorer {
Copy link

Copilot AI May 31, 2025

Choose a reason for hiding this comment

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

[nitpick] Add a GoDoc comment above this method, for example: "// GetJobStorer returns the application’s JobStorer."

Copilot uses AI. Check for mistakes.
@hwbrzzl hwbrzzl merged commit f6542cd into master May 31, 2025
14 checks passed
@hwbrzzl hwbrzzl deleted the bowen/#651-1 branch May 31, 2025 02:13
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.

Add DB driver for Queue module

2 participants