customcontrol-add behavior limits and conflict warning logs with new unit tests #3567
Conversation
z-soulx
force-pushed
the
1.8_customcontrol
branch
from
0d8790e to
59f2c4c
Compare
| /** | ||
| * Indicates whether this factory is a built-in Sentinel implementation. | ||
| * Built-in factories are allowed to use control behavior values in the reserved range [0, 255]. | ||
| * User-defined factories should return {@code false} (default) and use values >= 256. | ||
| * | ||
| * This method is used internally for validation during factory registration to ensure | ||
| * proper namespace separation and prevent conflicts. | ||
| * | ||
| * @return {@code true} if this is a Sentinel built-in factory, {@code false} for user-defined implementations | ||
| */ | ||
| default boolean isBuiltIn() { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I think built in impl could be override and improve by user, it seems no need to limit
There was a problem hiding this comment.
I think built in impl could be override and improve by user, it seems no need to limit
The main point here is to give a light warning—keeping it flexible while still notifying the user.
For instance, the default is false (not built-in). If a user forces the override, it means they are aware of the warning and its consequences.
Thought process:
I spent a lot of time thinking about this. As a beginner, without any restrictions or warnings, maintaining and ensuring compatibility down the line could be a real challenge. I considered three options:
-
Fully open: great flexibility, but tough to maintain and keep compatible.
-
Light restriction (this one): default false, can still be overridden. ☑️
-
Fully restricted: only custom interfaces with strict enforcement, but might limit third-party flexibility and design.
LearningGp
added
area/flow-control
4 participants
Describe what this PR does / why we need it
This PR addresses the remaining issues from #3477:
add behavior limits and conflict warning logs with new unit tests
1 No existing unit tests — added new tests for better coverage.
2 TrafficShapingControllerFactory allowed unrestricted custom controlBehavior definitions.
3 No warning when configuration conflicts occurred.
This caused conflicts with future official behaviors (e.g., type 5).
Does this pull request fix one issue?
#3189 and #3477
Describe how you did it
1 Added unit tests to verify control behavior and configuration correctness.
2 Introduced controlBehavior range restriction:
Describe how to verify it
unit tests
Special notes for reviews
Original initiator: myself — the initial PR (#3189) was incomplete and inactive
Perfected by: @icodening — this PR builds upon and improves his previous work.
This feature has been pending for 2–3 years; as the original author, I hope to help finalize it.
Thanks to @icodening for the collaboration and previous contributions — happy to sync anytime.