JSON APIs and bug fixes #717
Conversation
…ges (json api for viewAllMessages)
…lh) 2. sorted messages in searchMessages
There was a problem hiding this comment.
@ndanilin Thank you for this PR and sorry for the long delay in reviewing it.
It generally looks good. I have added a few comments regarding imports and code duplication. After these are resolved, the PR can be merged. Then we'll bring a release of Kafdrop.
| import kafdrop.model.TopicPartitionVO; | ||
| import kafdrop.model.TopicVO; | ||
| import kafdrop.form.SearchMessageFormForJson; | ||
| import kafdrop.model.*; |
There was a problem hiding this comment.
In our coding convention, we don't use wildcard imports. Please restore the original specific imports.
|
|
||
|
|
||
| import kafdrop.util.Serializers; | ||
| import kafdrop.util.*; |
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import kafdrop.model.CreateMessageVO; | ||
| import java.io.File; | ||
| import java.util.*; |
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.ResponseBody; | ||
| import org.springframework.web.bind.annotation.*; |
| final var deserializers = new Deserializers( | ||
| getDeserializer(topicName, defaultKeyFormat, "", "", protobufProperties.getParseAnyProto()), | ||
| getDeserializer(topicName, defaultFormat, "", "", protobufProperties.getParseAnyProto())); | ||
|
|
||
| final List<MessageVO> messages = new ArrayList<>(); | ||
|
|
||
| for (TopicPartitionVO partition : topic.getPartitions()) { | ||
| messages.addAll(messageInspector.getMessages(topicName, | ||
| partition.getId(), | ||
| partition.getFirstOffset(), | ||
| size, | ||
| deserializers)); | ||
| } | ||
|
|
||
| messages.sort(Comparator.comparing(MessageVO::getTimestamp)); |
There was a problem hiding this comment.
This duplicates the lines 102-116 of viewAllMessages. Extract these into a private method and call that method here instead of duplicating them.
| final var deserializers = new Deserializers( | ||
| getDeserializer(topicName, searchMessageForm.getKeyFormat(), null, null, | ||
| protobufProperties.getParseAnyProto()), | ||
| getDeserializer(topicName, searchMessageForm.getFormat(), null, null, | ||
| protobufProperties.getParseAnyProto()) | ||
| ); | ||
|
|
||
| var searchResultsVO = kafkaMonitor.searchMessages( | ||
| topicName, | ||
| searchMessageForm.getSearchText(), | ||
| searchMessageForm.getPartition(), | ||
| searchMessageForm.getMaximumCount(), | ||
| searchMessageForm.getStartTimestamp(), | ||
| deserializers); |
There was a problem hiding this comment.
These lines duplicate the lines 325-339 of searchMessageForm, except that descFile and msgTypeName are set to null. Extract the lines into a method that takes descFile and msgTypeName as parameters and call the method here and above.
| @@ -28,6 +30,7 @@ public final class MessageVO { | |||
| private String message; | |||
| private String key; | |||
| private Map<String, String> headers; | |||
| @JsonFormat(pattern = "yyyy-MM-dd HH:mm:ss.SSS") | |||
There was a problem hiding this comment.
Why this format instead of ISO 8601?
Bug fixes:
viewAllMessages(I think it's a critical bug).searchRecords. Search messages by timestamp didn't work becauseseekToTimestampcan't guarantee needed result.New features:
getAllMessages(equivalent ofviewAllMessagesthat returns JSON array of messages).searchMessages(equivalent ofsearchMessageFormthat returns JSON array of messages). Here I made customSearchMessageFormForJsonand now all fields of form are not required. Also added here search by keys (It might overload this method, but seems very helpful for guaranteed results)