Repository: apache/rocketmq-client-go
Pull request #300 (2019-11-16 15:18:56)

[ISSUE #299] offset store read missing fallthrough for _ReadMemoryThenStore
#299 missing `fallthrough` statement in offset store read.  

codecov-io: # [Codecov](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=h1) Report
> Merging [#300](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=desc) into [native](https://codecov.io/gh/apache/rocketmq-client-go/commit/4364ebd190588b4a40963182495318002b03e63a?src=pr&el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq-client-go/pull/300/graphs/tree.svg?width=650&token=CiV9ofPkG3&height=150&src=pr)](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           native     #300      +/-   ##
==========================================
+ Coverage   26.15%   26.23%   +0.08%     
==========================================
  Files          46       46              
  Lines        5058     5061       +3     
==========================================
+ Hits         1323     1328       +5     
+ Misses       3558     3557       -1     
+ Partials      177      176       -1
```


| [Impacted Files](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [consumer/offset\_store.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/300/diff?src=pr&el=tree#diff-Y29uc3VtZXIvb2Zmc2V0X3N0b3JlLmdv) | `64.67% <100%> (+1.42%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=footer). Last update [4364ebd...3946ffc](https://codecov.io/gh/apache/rocketmq-client-go/pull/300?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/consumer/offset_store.go b/consumer/offset_store.go
index e86ec3b..1b9a552 100644
--- a/consumer/offset_store.go
+++ b/consumer/offset_store.go
@@ -161,6 +161,7 @@ func (local *localFileOffsetStore) read(mq *primitive.MessageQueue, t readType)
 		if off >= 0 || (off == -1 && t == _ReadFromMemory) {
 			return off
 		}
+		fallthrough
 	case _ReadFromStore:
 		local.load()
 		return readFromMemory(local.OffsetTable, mq)
@@ -277,14 +278,16 @@ func (r *remoteBrokerOffsetStore) read(mq *primitive.MessageQueue, t readType) i
 	r.mutex.RLock()
 	switch t {
 	case _ReadFromMemory, _ReadMemoryThenStore:
-		defer r.mutex.RUnlock()
 		off, exist := r.OffsetTable[*mq]
 		if exist {
+			r.mutex.RUnlock()
 			return off
 		}
 		if t == _ReadFromMemory {
+			r.mutex.RUnlock()
 			return -1
 		}
+		fallthrough
 	case _ReadFromStore:
 		off, err := r.fetchConsumeOffsetFromBroker(r.group, mq)
 		if err != nil {


Test patch:
diff --git a/consumer/offset_store_test.go b/consumer/offset_store_test.go
index 542c5a2..f8bb3f6 100644
--- a/consumer/offset_store_test.go
+++ b/consumer/offset_store_test.go
@@ -18,6 +18,7 @@ limitations under the License.
 package consumer
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -41,21 +42,21 @@ func TestNewLocalFileOffsetStore(t *testing.T) {
 				group:    "testGroup",
 				expectedResult: &localFileOffsetStore{
 					group: "testGroup",
-					path:  _LocalOffsetStorePath + "/testGroup/offset.json",
+					path:  filepath.Join(_LocalOffsetStorePath, "/testGroup/offset.json"),
 				},
 			}, {
 				clientId: "192.168.24.1@default",
 				group:    "",
 				expectedResult: &localFileOffsetStore{
 					group: "",
-					path:  _LocalOffsetStorePath + "/192.168.24.1@default/offset.json",
+					path:  filepath.Join(_LocalOffsetStorePath, "/192.168.24.1@default/offset.json"),
 				},
 			}, {
 				clientId: "192.168.24.1@default",
 				group:    "testGroup",
 				expectedResult: &localFileOffsetStore{
 					group: "testGroup",
-					path:  _LocalOffsetStorePath + "/192.168.24.1@default/testGroup/offset.json",
+					path:  filepath.Join(_LocalOffsetStorePath, "/192.168.24.1@default/testGroup/offset.json"),
 				},
 			},
 		}
@@ -134,6 +135,10 @@ func TestLocalFileOffsetStore(t *testing.T) {
 			localStore.persist(queues)
 			offset = localStore.read(mq, _ReadFromStore)
 			So(offset, ShouldEqual, 1)
+
+			delete(localStore.(*localFileOffsetStore).OffsetTable, MessageQueueKey(*mq))
+			offset = localStore.read(mq, _ReadMemoryThenStore)
+			So(offset, ShouldEqual, 1)
 		})
 	})
 }
@@ -204,7 +209,7 @@ func TestRemoteBrokerOffsetStore(t *testing.T) {
 		Convey("test persist", func() {
 			queues := []*primitive.MessageQueue{mq}
 
-			namesrv.EXPECT().FindBrokerAddrByName(gomock.Any()).Return("192.168.24.1:10911")
+			namesrv.EXPECT().FindBrokerAddrByName(gomock.Any()).Return("192.168.24.1:10911").MaxTimes(2)
 
 			ret := &remote.RemotingCommand{
 				Code: internal.ResSuccess,
@@ -212,11 +217,18 @@ func TestRemoteBrokerOffsetStore(t *testing.T) {
 					"offset": "1",
 				},
 			}
-			rmqClient.EXPECT().InvokeSync(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(ret, nil)
+			rmqClient.EXPECT().InvokeSync(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(ret, nil).MaxTimes(2)
 
 			remoteStore.persist(queues)
 			offset := remoteStore.read(mq, _ReadFromStore)
 			So(offset, ShouldEqual, 1)
+
+			remoteStore.remove(mq)
+			offset = remoteStore.read(mq, _ReadFromMemory)
+			So(offset, ShouldEqual, -1)
+			offset = remoteStore.read(mq, _ReadMemoryThenStore)
+			So(offset, ShouldEqual, 1)
+
 		})
 
 		Convey("test remove", func() {