Instances

[System-Glitch]

Proposal

Instead of using globals for the server, the config, the database connection, etc, we could use structures. That way, every component that is currently more or less a "singleton" could have multiple properly separated instances.

Using config as an example, the config.Load() function would return a *config.Config, a structure that would implement Get(), Set(), etc.

This would require changes to fundamentals of the framework, such as the Handlers. Instead of receiving *goyave.Request and *goyave.Response, they would receive *goyave.Context, containing the request, the response, but also the config, a DB connection, etc.

Benefits

• Easier and parallel testing.
• More flexible API thanks to using a context for handlers.
• Multiple servers could run within the same app (not recommended though. Goyave was originally made thinking you would/should only run one REST service from your app).
• Less mutex locks because some global values such as the database connection would be passed by the context.
• Cleaner overall than using globals everywhere.

Drawbacks

• This would be a huge breaking change and would require a lot of work to upgrade from v3.
• Some more code would be required in all components. From config.Get() to ctx.Config.Get() for example.
• Some features would be less accessible from outside handlers, middleware, etc.
• Will probably require a bit more code in the main function (need to specify config for example).

Questions

• Should the language package be rewritten using the same logic? This wouldn't have many benefits compared to the rest.
• Are all these changes worth it? The main concrete advantage would be parallel testing, but at the cost of some ease of writing the application code.
• Feel free to discuss and express your opinion.

[System-Glitch]

I think it would be wiser to push back this kind of change to at least v5. The release of v4 already contains more than enough breaking changes and is long overdue.

[agbaraka]

Hi @System-Glitch

From the above, I think having handler function signature with only *goyave.Context containing components such as db, config is a great.

Will there be a way to add external components? and do you think the generics in 1.18 could help on this?

Thanks.

[System-Glitch]

@agbaraka
This instance system alone would require a huge rewrite already.

I would love to be able to neatly integrate external components / plugins to this. Do you think of a way we could implement this?

[agbaraka]

Hi @System-Glitch

After doing some research on this, I think a cleaner way to have a consistent way for external and internal components is for all components to use globals (which is the current structure).

Can we use sync.Once to reduce mutex locking?

[System-Glitch]

Hello @agbaraka

Can you elaborate? Can you provide a simple example of what you have in mind?

[agbaraka]

Hello @agbaraka

Can you elaborate? Can you provide a simple example of what you have in mind?

Ok.. So, let's take func GetConnection() *gorm.DB found in the database package.

Using mutex (current implementation)

...
var (
	dbConnection *gorm.DB
	mu           sync.Mutex
...
)


func GetConnection() *gorm.DB {
	mu.Lock()
	defer mu.Unlock()
	if dbConnection == nil {
		dbConnection = newConnection()
	}
	return dbConnection
}
...

Using sync.Once

...
var (
	dbConnection *gorm.DB
	dbConnOnce sync.Once
...
)

func GetConnection() *gorm.DB {

        dbConnOnce.Do(func() {
              dbConnection = newConnection()
         })

	return dbConnection
}
...

[System-Glitch]

If we do this we sacrifice the ability to Close() the connection and create a new one. This would break a lot of tests.
I would love to be able to reduce the synchronization overhead of the database package though, and using "instances" as described in the issue can be a possibility.

[System-Glitch]

Implemented in v5.