Walkthrough

The path.go file undergoes a type system refactoring for constraint handling. The TypeConstraint type is changed from int16 to uint16, constraint encoding shifts from sequential iota to bit-flag representation, and two new composite constraint masks are introduced to centralize data-length validation logic.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Bits aligned in binary grace,
Constraints dance in flag-space,
Masks of data, clean and tight—
One or two, the logic's right!
From signed depths to unsigned height,
Path.go sparkles, pure delight! ✨

Pre-merge checks and finishing touches

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Summary of Changes

Hello @ksw2000, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the internal validation mechanism for TypeConstraint parameters. By transitioning from sequential integer IDs to bitwise flags and introducing bitmask constants, the change aims to improve code readability and consistency. The core impact is seen in the CheckConstraint method, where a switch statement for data requirement validation is replaced with more efficient and expressive bitwise operations.

Highlights

• Type Definition Change: The TypeConstraint type has been updated from int16 to uint16 to better accommodate bitwise operations.
• Bit Flag Implementation: Constants for TypeConstraint are now initialized using bit shifting (1 << iota), transforming them into distinct bit flags.
• New Bitmask Constants: Two new constants, needOneData and needTwoData, have been introduced as bitwise combinations to group constraints based on their data requirements.
• Refactored Validation Logic: The CheckConstraint method's data validation logic has been refactored to replace a switch statement with more concise and efficient bitwise AND operations using the new bitmask constants.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.67%. Comparing base (032ff5f) to head (f82130d).
⚠️ Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3963      +/-   ##
==========================================
- Coverage   91.67%   91.67%   -0.01%     
==========================================
  Files         119      119              
  Lines       10177    10173       -4     
==========================================
- Hits         9330     9326       -4     
  Misses        536      536              
  Partials      311      311              

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

@ksw2000 thx, can you share benchmark results in a comment

Just noticed that we don't have any separate benchmarks for the constraints,
only the path test cases.
Maybe you could add 1-2 benchmarks so we can see the optimizations in numbers.
Otherwise, I can also merge. I'm sure the bit operators are faster.

goos: linux
goarch: amd64
pkg: github.com/gofiber/fiber/v3
cpu: AMD EPYC 7763 64-Core Processor                
                            │   old.txt   │              new.txt               │
                            │   sec/op    │   sec/op     vs base               │
CheckConstraintReg-2          283.7n ± 3%   283.5n ± 2%       ~ (p=0.945 n=50)
CheckConstraintMinLen-2       16.78n ± 3%   16.29n ± 1%  -2.89% (p=0.004 n=50)
CheckConstraintBetweenLen-2   24.58n ± 4%   24.53n ± 1%       ~ (p=0.746 n=50)
geomean                       48.91n        48.39n       -1.06%

Benchmark shows here

func BenchmarkCheckConstraintReg(b *testing.B) {
	reg, err := regexp.Compile("^[a-z0-9]([a-z0-9-]{1,61}[a-z0-9])?$")
	if err != nil {
		b.Fatal()
	}
	c := Constraint{
		RegexCompiler:     reg,
		Name:              "regex",
		Data:              []string{"^[a-z0-9]([a-z0-9-]{1,61}[a-z0-9])?$"},
		customConstraints: []CustomConstraint{},
		ID:                regexConstraint,
	}
	for i := 0; i < b.N; i++ {
		if c.CheckConstraint("12") {
			b.Fail()
		}
		if !c.CheckConstraint("test") {
			b.Fail()
		}
	}
}

func BenchmarkCheckConstraintMinLen(b *testing.B) {
	c := Constraint{
		RegexCompiler:     nil,
		Name:              "minLen",
		Data:              []string{"5"},
		customConstraints: []CustomConstraint{},
		ID:                minLenConstraint,
	}
	for i := 0; i < b.N; i++ {
		if c.CheckConstraint("123") {
			b.Fail()
		}
		if !c.CheckConstraint("12345") {
			b.Fail()
		}
	}
}

func BenchmarkCheckConstraintBetweenLen(b *testing.B) {
	c := Constraint{
		RegexCompiler:     nil,
		Name:              "betweenLen",
		Data:              []string{"2", "5"},
		customConstraints: []CustomConstraint{},
		ID:                betweenLenConstraint,
	}
	for i := 0; i < b.N; i++ {
		if c.CheckConstraint("e") {
			b.Fail()
		}
		if !c.CheckConstraint("en") {
			b.Fail()
		}
	}
}

Thx