feat: [SUB-786] Add WithProviders function by krishankumar01 · Pull Request #1236 · goravel/framework

krishankumar01 · 2025-10-19T04:13:01Z

📑 Description

This PR makes the following changes:

Moves ApplicationBuilder contracts to its own file, as application.go is getting too long.
Introduces ProviderRepository to manage all provider-related operations (sorting, registration, booting, cycle detection, etc.).
Refactors Application to delegate all provider logic to the new ProviderRepository, making application.go much cleaner and simpler.
Adds a ProviderRepository interface for decoupling and easier testing.
Updates ApplicationBuilder to use the new Application.SetConfiguredProviders hook.

Note

Currently, this PR won't change how package:install registers providers; it will still use the app.providers key in the config. I will optimize that in a different PR.

Usage-wise, there is not much change, but it does introduce the new WithProviders method, which can now be used with foundation.Configure():

func Boot() {
	foundation.Configure().
		WithConfig(config.Boot).
		WithProviders([]contractsfoundation.ServiceProvider{
			&process.ServiceProvider{},
		}).
		Run()
}

✅ Checks

Added test cases for my code

Copilot

Pull Request Overview

This PR adds a WithProviders function to the ApplicationBuilder interface, enabling registration of custom service providers during application initialization. The changes also reorganize the codebase by moving the ApplicationBuilder interface to its own file.

Adds WithProviders method to register service providers
Moves ApplicationBuilder interface from application.go to dedicated application_builder.go file
Implements the new method in the concrete ApplicationBuilder type

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
contracts/foundation/application_builder.go	New file containing the `ApplicationBuilder` interface with the added `WithProviders` method
contracts/foundation/application.go	Removed `ApplicationBuilder` interface (moved to separate file)
foundation/application_builder.go	Implements `WithProviders` method for the `ApplicationBuilder` struct

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

foundation/application_builder.go

codecov · 2025-10-24T21:58:22Z

Codecov Report

❌ Patch coverage is 94.69027% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.66%. Comparing base (68906f1) to head (733cd85).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
foundation/application.go	70.00%	6 Missing ⚠️
foundation/provider_repository.go	96.96%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1236      +/-   ##
==========================================
+ Coverage   65.73%   67.66%   +1.92%     
==========================================
  Files         248      250       +2     
  Lines       17185    14109    -3076     
==========================================
- Hits        11297     9547    -1750     
+ Misses       5511     4180    -1331     
- Partials      377      382       +5

☔ 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.

Copilot

Pull Request Overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hwbrzzl

Pretty good 💯 Only a few nitpicks

contracts/foundation/application.go

contracts/foundation/provider_repository.go

hwbrzzl · 2025-10-25T08:39:52Z

foundation/provider_repository.go

+	loaded      bool
+}
+
+func NewProviderRepository() *ProviderRepository {


Suggested change

func NewProviderRepository() *ProviderRepository {

func NewProviderRepository(app foundation.Application) *ProviderRepository {

Passing a struct-level parameter for this felt confusing to me, specifically, the fact that
Application accepts a ProviderRepository, and ProviderRepository in turn accepts an Application in constructor. And given that we only need the app for passing it to the service provider during Register and Boot.
That kind of circular dependency feels odd.

type Application struct { *Container ctx context.Context cancel context.CancelFunc providerRepository foundation.ProviderRepository publishes map[string]map[string]string publishGroups map[string]map[string]string json foundation.Json } type ProviderRepository struct { app foundation.ServiceProvider states map[string]*ProviderState providers []foundation.ServiceProvider sorted []foundation.ServiceProvider sortedValid bool loaded bool }

Agree about odd, LoadFromConfig can be optimized, config can be passed via NewProviderRepository .

But we still need the app in the Register and Boot methods, so I'm not sure if it's even necessary to accept config in constructor.

The config facade is optional, and this method (LoadFromConfig) may be removed in a future version when we deprecate setting providers through the config.

Yes, but passing config is more reasonable than app.

Yeah, we can accept config, but my only concern is whether it’s unnecessary to take it as a constructor parameter. We could just accept config in LoadFromConfig instead of app. What do you think?

Em, I think we can add a constructor parameter directly, it's consistent with other modules.

I think there is one more issue with adding it in constructor param:

func init() { setEnv() setRootPath() ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) app := &Application{ Container: NewContainer(), ctx: ctx, cancel: cancel, publishes: make(map[string]map[string]string), publishGroups: make(map[string]map[string]string), } app.providerRepository = NewProviderRepository() // I will need to call app.MakeConfig() will will return an error since config service provider is not booted yet, will that be an issue here? App = app baseProviders := app.getBaseServiceProviders() app.providerRepository.Add(baseProviders) app.providerRepository.Register(app) app.providerRepository.Boot(app) app.SetJson(json.New()) }

Good catch, makes sense, we have to pass the config to the LoadFromConfig function due to this.

foundation/provider_repository.go

contracts/foundation/provider_repository.go

foundation/provider_repository.go

contracts/foundation/provider_repository.go

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite	Current: `733cd85`	Previous: `c1347c3`	Ratio
`Benchmark_EncryptString`	`4330` ns/op 2152 B/op 14 allocs/op	`1550` ns/op 2152 B/op 14 allocs/op	`2.79`
`Benchmark_EncryptString - ns/op`	`4330` ns/op	`1550` ns/op	`2.79`

This comment was automatically generated by workflow using github-action-benchmark.

hwbrzzl

Thanks, LGTM except one tip.

hwbrzzl

Amazing, LGTM.

add WithProviders in interfaces

109eeea

krishankumar01 requested a review from a team as a code owner October 19, 2025 04:13

Copilot AI review requested due to automatic review settings October 19, 2025 04:13

krishankumar01 marked this pull request as draft October 19, 2025 04:13

Copilot AI reviewed Oct 19, 2025

View reviewed changes

foundation/application_builder.go Outdated Show resolved Hide resolved

krishankumar01 and others added 3 commits October 19, 2025 04:13

chore: update mocks

bcae732

Merge branch 'master' into kkumar-gcc/#790

a0ae498

add provider repository

ef75506

krishankumar01 marked this pull request as ready for review October 24, 2025 22:07

krishankumar01 added the 🚀 Review Ready label Oct 24, 2025

krishankumar01 requested a review from Copilot October 24, 2025 22:09

Copilot AI reviewed Oct 24, 2025

View reviewed changes

optimise provider repository

140f50e

hwbrzzl reviewed Oct 25, 2025

View reviewed changes

krishankumar01 added 2 commits October 25, 2025 14:51

rename AddProviders to Add

bb1f603

rename AddProviders to Add

93b8e05

github-actions bot reviewed Oct 25, 2025

View reviewed changes

fix test

cc2b314

hwbrzzl reviewed Oct 26, 2025

View reviewed changes

accept config instead of app in LoadFromConfig

733cd85

krishankumar01 requested a review from hwbrzzl October 26, 2025 05:24

hwbrzzl approved these changes Oct 26, 2025

View reviewed changes

krishankumar01 merged commit 3747c0f into master Oct 26, 2025
13 of 14 checks passed

krishankumar01 deleted the kkumar-gcc/#790 branch October 26, 2025 08:16

	func NewProviderRepository() *ProviderRepository {
	func NewProviderRepository(app foundation.Application) *ProviderRepository {

Conversation

krishankumar01 commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📑 Description

✅ Checks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

codecov bot commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

hwbrzzl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hwbrzzl Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

krishankumar01 Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hwbrzzl Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

krishankumar01 Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krishankumar01 Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hwbrzzl Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

krishankumar01 Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

hwbrzzl Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

krishankumar01 Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

hwbrzzl Oct 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

hwbrzzl left a comment

Choose a reason for hiding this comment

Uh oh!

hwbrzzl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

krishankumar01 commented Oct 19, 2025 •

edited

Loading

codecov bot commented Oct 24, 2025 •

edited

Loading

krishankumar01 Oct 25, 2025 •

edited

Loading

krishankumar01 Oct 25, 2025 •

edited

Loading

krishankumar01 Oct 25, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading