♻️ v3: fix!: ContextKey collisions

[sixcolors]

Description

Fixes context key collision issues in middleware and ctx.go. Follows best practice/convention from context by using unexported key types to avoid collisions.

Related to #2684 which outlines the issue and rationale for change.

Changes Introduced

List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.

• Documentation Update: Detail the updates made to the documentation and links to the changed files.
• Changelog/What's New: Include a summary of the additions for the upcoming release notes.
• Migration Guide: If necessary, provide a guide or steps for users to migrate their existing code to accommodate these changes.
• API Longevity: Discuss the steps taken to ensure that the new or updated APIs are consistent and not prone to breaking changes.
• Examples: Provide examples demonstrating the new features or changes in action.

Type of Change

Please delete options that are not relevant.

• Enhancement (improvement to existing features and functionality)
• Documentation update (changes to documentation)
• Code consistency (breaking change which improves code reliability and robustness)
• Breaking change

Checklist

Before you submit your pull request, please make sure you meet these requirements:

• Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
• Conducted a self-review of the code and provided comments for complex or critical parts.
• Updated the documentation in the /docs/ directory for Fiber's documentation.
• Added or updated unit tests to validate the effectiveness of the changes or new features.
• Ensured that new and existing unit tests pass locally with the changes.
• Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
• Aimed for optimal performance with minimal allocations in the new code.

Commit Formatting

Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md

[sixcolors]

fixed to contrib packages: gofiber/contrib#896

[sixcolors]

@benjajaja tag

[sixcolors]

@ReneWerner87 I didn't see a v3 branch in the repo, please let me know how you want to handle this for contrib.

[ReneWerner87]

@ReneWerner87 I didn't see a v3 branch in the repo, please let me know how you want to handle this for contrib.

we haven't created one there yet, but you are welcome to create a beta branch

[ReneWerner87]

with this concept it is no longer possible to customize the context key from the outside

does this have any disadvantages for us? @gofiber/maintainers @gofiber/contributors
i only have one usecase in mind where you use the middlewares multiple times, but that would only lead to problems with the same routes, but where you don't use these special ones twice or ?
does anyone have any worst cases in mind ?

otherwise the code looks well implemented according to this concept

[renanbastos93]

Awesome, you did a good job!

[efectn]

with this concept it is no longer possible to customize the context key from the outside

does this have any disadvantages for us? @gofiber/maintainers @gofiber/contributors i only have one usecase in mind where you use the middlewares multiple times, but that would only lead to problems with the same routes, but where you don't use these special ones twice or ? does anyone have any worst cases in mind ?

otherwise the code looks well implemented according to this concept

I am happy with the change since it's recommended behavior by Go context docs.

[gaby]

with this concept it is no longer possible to customize the context key from the outside
does this have any disadvantages for us? @gofiber/maintainers @gofiber/contributors i only have one usecase in mind where you use the middlewares multiple times, but that would only lead to problems with the same routes, but where you don't use these special ones twice or ? does anyone have any worst cases in mind ?
otherwise the code looks well implemented according to this concept

I am happy with the change since it's recommended behavior by Go context docs.

Agree, with the statement

[sixcolors]

with this concept it is no longer possible to customize the context key from the outside

does this have any disadvantages for us? @gofiber/maintainers @gofiber/contributors i only have one usecase in mind where you use the middlewares multiple times, but that would only lead to problems with the same routes, but where you don't use these special ones twice or ? does anyone have any worst cases in mind ?

otherwise the code looks well implemented according to this concept

I didn’t envision anyone using the same middleware twice with the same routes.

In my opinion the package functions improve the developer experience and make the framework more user-friendly. Also some existing middleware already use this approach with predicate functions.

Is there is a compelling use case for changing the approach or some documentation changes requested?

[ReneWerner87]

Currently this should work, just wanted to mention it
Already had 1 or 2 reports where people wanted to use the limiter middleware multiple times for the same route with different settings, which also made sense because these then ensured with the internal next that they were not both executed
But the code should work. As long as no one else has something in mind that speaks against it

[GalvinGao]

A pretty minor point here, but you could use var contextKey struct{} to achieve zero memory allocation during initialization and still able to ensure type safety.

[sixcolors]

A pretty minor point here, but you could use var contextKey struct{} to achieve zero memory allocation during initialization and still able to ensure type safety.

Where there are multiple keys it would wind up something like this:

type (
	contextKeyUsername struct{}
	contextKeyPassword struct{}
)

var (
	usernameKey contextKeyUsername
	passwordKey contextKeyPassword
)

In the above code, two key variables are stored on the stack, and they point to an instance of contextKey*. Because contextKey* is an empty struct, it does not take up any memory, and there are no heap allocations. However, the variable keys take up a small amount of stack space (the size of a pointer).

vs

type contextKey int

 const (
 	usernameKey contextKey = iota
 	passwordKey
 )

This approach does not cause any heap allocations or take up any runtime stack space because constants in Go are replaced by their values at compile time where they are used.

So, AFAIK both approaches would ensure type safety and uniqueness of keys. And have zero allocs on the heap.

---

I preferred the style of iota when multiple context keys were used, also stand lib context package uses a type key int in its example.

However, what does everyone else prefer? Do you find the distinct struct types or iota style more readable and maintainable?

[ReneWerner87]

Since the allocations are at the registration time it doesn't matter for me

But i prefer the iota variant

[sixcolors]

Since the allocations are at the registration time it doesn't matter for me

But i prefer the iota variant

From what I understand, in Go, constants are not allocated in memory like variables are. Instead, they are replaced by their values at compile time and do not have an address. This implies that constants do not exist while the program is running and there is no memory allocation for them other than the executable program code. Therefore, using const with iota would appear to be a zero allocation solution.

However, please let me know if I've misunderstood anything.