[GitHub] [pulsar-client-go] nitishv commented on a change in pull request #222: WIP: [Issue 218] Add Seek functions to Reader

[GitBox <gi...@apache.org>]

nitishv commented on a change in pull request #222:
URL: https://github.com/apache/pulsar-client-go/pull/222#discussion_r449955387



##########
File path: pulsar/reader_test.go
##########
@@ -362,3 +365,108 @@ func TestReaderHasNext(t *testing.T) {
 
 	assert.Equal(t, 10, i)
 }
+
+func TestReaderSeek(t *testing.T) {

Review comment:
       There can be an added test case which does a Seek on a custom MessageID refer to https://github.com/apache/pulsar-client-go/pull/305

##########
File path: pulsar/reader_impl.go
##########
@@ -150,3 +151,33 @@ func (r *reader) hasMoreMessages() bool {
 func (r *reader) Close() {
 	r.pc.Close()
 }
+
+func (r *reader) messageID(msgID MessageID) (*messageID, bool) {
+	mid, ok := msgID.(*messageID)
+	if !ok {
+		r.log.Warnf("invalid message id type")
+		return nil, false
+	}
+
+	partition := mid.partitionIdx
+	// did we receive a valid partition index?
+	if partition != 0 {
+		r.log.Warnf("invalid partition index %d expected 0", partition)
+		return nil, false
+	}
+
+	return mid, true
+}

Review comment:
       Given the method name, the purpose of this method is to convert MessageID to get *messageID
   In doing so failure in type assertion is not really a failure. The given msgID can still be converted. Refer to a this fix I just made https://github.com/apache/pulsar-client-go/pull/305
   
   I think that change can be re-factored into this unexported method for reader, as it being used in couple of places now with this change.
   Also checking for non-zero partition index does not seem to be the purpose of this method, and should not be included here. That check is only relevant to Seek APIs, and should be moved there.
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org