Fix partitioned system topic check bug #10529
Conversation
hangc0276
requested review from
codelipenghui,
congbobo184,
315157973 and
eolivelli
| @@ -168,7 +168,10 @@ | |||
| } | |||
|
|
|||
| static boolean isSystemTopic(TopicName topicName) { | |||
| return EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getLocalName()); | |||
| String localName = topicName.getLocalName(); | |||
There was a problem hiding this comment.
what about
topicName.getLocalName().startsWith(EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME);
There was a problem hiding this comment.
+1
return topicName != null && topicName.getLocalName().startsWith(EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME);
There was a problem hiding this comment.
or
return EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getPartitionedTopicName());
There was a problem hiding this comment.
what about
topicName.getLocalName().startsWith(EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME);
@linlinnn if the topic name is __change_events_v1, using startsWith, it will return true. However it isn't system topic.
There was a problem hiding this comment.
or
return EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getPartitionedTopicName());
@315157973 topicName.getPartitionedTopicName() method will return topic name with prefix, such as, persistent://public/defualt/__change_events, it will always return false.
There was a problem hiding this comment.
TopicName.get(topicName.getPartitionedTopicName()).getLocalName()
There was a problem hiding this comment.
We have a method in the TopicName is isPartitioned(). You can check as followings
if (topicName.isPartitioned()) {
return EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(TopicName.get(topicName.getPartitionedTopicName));
} else {
return
EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getLocalName());
}There was a problem hiding this comment.
@315157973 good job. I have update the code, PTAL.
There was a problem hiding this comment.
@codelipenghui I have update the code, PTAL.
codelipenghui
added
type/bug
### Motivation
When checking a partitioned topic whether a system topic, it will always be `false`. The check logic is.
```Java
static boolean isSystemTopic(TopicName topicName) {
return EventsTopicNames.NAMESPACE_EVENTS_LOCAL_NAME.equals(topicName.getLocalName());
}
```
```Java
public static final String NAMESPACE_EVENTS_LOCAL_NAME = "__change_events";
```
The partitioned topic name is like `__change_events-partition-0`.
### Modification
1. Trim the partition suffix for the topic local name if exists.
2. Add a test to cover this case.
(cherry picked from commit 4a8d40c)
Motivation
When checking a partitioned topic whether a system topic, it will always be
false. The check logic is.The partitioned topic name is like
__change_events-partition-0.Modification