Optimize isCloudEventsHeader check
#445
Conversation
Instead of using: ```java key.substring(0, CE_PREFIX.length()).toLowerCase().startsWith(CE_PREFIX); ``` we can just use: ```java key.regionMatches(true /* ignoreCase */, 0, CE_PREFIX, 0, CE_PREFIX.length()); ``` Signed-off-by: Pierangelo Di Pilato <[email protected]>
Signed-off-by: Pierangelo Di Pilato <[email protected]>
| @@ -44,7 +44,7 @@ protected boolean isContentTypeHeader(String key) { | |||
|
|
|||
| @Override | |||
| protected boolean isCloudEventsHeader(String key) { | |||
| return key.length() > 3 && key.substring(0, KafkaHeaders.CE_PREFIX.length()).startsWith(KafkaHeaders.CE_PREFIX); | |||
| return key.length() > KafkaHeaders.CE_PREFIX.length() && key.startsWith(KafkaHeaders.CE_PREFIX); | |||
There was a problem hiding this comment.
Any particular reason this isn't using your new method ?
Also ... Might be worth adding the null-check on key (most of the other readers do this)
There was a problem hiding this comment.
Any particular reason this isn't using your new method ?
it wasn't case-insensitive before like other ones and I think the reason is that other methods are HTTP based (headers normalization) vs this one being Kafka based.
Do you know if the spec is case-insensitive for every defined protocol binding?
Honestly, from a user perspective, I would be surprised if a header isn't considered a CE header due to the case, so I agree with you.
There was a problem hiding this comment.
Hey @JemDay since most of the comments aren't strictly related to the original goal of the PR and the code is strictly equivalent to the existing one, I think, we can follow up on these comments in a different PR/issue.
Instead of using:
that allocates 2 new strings, we can just use:
Signed-off-by: Pierangelo Di Pilato [email protected]