Repository: yorkie-team/yorkie
Pull request #312 (2022-04-26 09:05:45)

Add direction to ListDocuments API
<!--  Thanks for sending a pull request! -->

**What this PR does / why we need it**:

Add direction to ListDocuments API to navigate to the next and previous pages.

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Address https://github.com/yorkie-team/yorkie-house/issues/4

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note

```

**Additional documentation**:

<!--
This section can be blank if this pull request does not require a release note.

Please use the following format for linking documentation:
- [Usage]: <link>
- [Other doc]: <link>
-->
```docs
// If isForward is true, this request returns document lists with an id greater than previousID in ascending order.
// Otherwise, this request returns document lists with an id less than previousID in descending order.

listDocuments(previousID: string, pageSize: number, isForward: boolean)
```

**Checklist**:
- [x] Added relevant tests or not required
- [x] Didn't break anything


CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/yorkie-team/yorkie?pullRequest=312) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/yorkie-team/yorkie?pullRequest=312) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/yorkie-team/yorkie?pullRequest=312) it.</sub>
codecov[bot]: # [Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/312?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) Report
> Merging [#312](https://codecov.io/gh/yorkie-team/yorkie/pull/312?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) (4f5f18f) into [main](https://codecov.io/gh/yorkie-team/yorkie/commit/acdefd4ba30b79ae8cc7aa761bbcbc2da89dd90a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) (acdefd4) will **increase** coverage by `0.15%`.
> The diff coverage is `71.42%`.

```diff
@@            Coverage Diff             @@
##             main     #312      +/-   ##
==========================================
+ Coverage   48.07%   48.23%   +0.15%     
==========================================
  Files          59       59              
  Lines        4903     4918      +15     
==========================================
+ Hits         2357     2372      +15     
  Misses       2285     2285              
  Partials      261      261              
```


| [Impacted Files](https://codecov.io/gh/yorkie-team/yorkie/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) | Coverage Δ | |
|---|---|---|
| [yorkie/backend/db/mongo/client.go](https://codecov.io/gh/yorkie-team/yorkie/pull/312/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-eW9ya2llL2JhY2tlbmQvZGIvbW9uZ28vY2xpZW50Lmdv) | `0.00% <0.00%> (ø)` | |
| [yorkie/backend/db/memory/db.go](https://codecov.io/gh/yorkie-team/yorkie/pull/312/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-eW9ya2llL2JhY2tlbmQvZGIvbWVtb3J5L2RiLmdv) | `52.10% <100.00%> (+2.10%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/312?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/312?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team). Last update [acdefd4...4f5f18f](https://codecov.io/gh/yorkie-team/yorkie/pull/312?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team).

chacha912: Thank you for your comment. 

1. The code is much easier to read. Thanks👍 
2. Yes. The `FindDocInfosByPreviousIDAndPageSize` function works the same way in MongoDB.

Patch:
diff --git a/api/admin.pb.go b/api/admin.pb.go
index 85c278d4..94969eba 100644
--- a/api/admin.pb.go
+++ b/api/admin.pb.go
@@ -29,6 +29,7 @@ const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package
 type ListDocumentsRequest struct {
 	PreviousId           string   `protobuf:"bytes,1,opt,name=previous_id,json=previousId,proto3" json:"previous_id,omitempty"`
 	PageSize             int32    `protobuf:"varint,2,opt,name=page_size,json=pageSize,proto3" json:"page_size,omitempty"`
+	IsForward            bool     `protobuf:"varint,3,opt,name=is_forward,json=isForward,proto3" json:"is_forward,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
 	XXX_unrecognized     []byte   `json:"-"`
 	XXX_sizecache        int32    `json:"-"`
@@ -81,6 +82,13 @@ func (m *ListDocumentsRequest) GetPageSize() int32 {
 	return 0
 }
 
+func (m *ListDocumentsRequest) GetIsForward() bool {
+	if m != nil {
+		return m.IsForward
+	}
+	return false
+}
+
 type ListDocumentsResponse struct {
 	Documents            []*DocumentSummary `protobuf:"bytes,1,rep,name=documents,proto3" json:"documents,omitempty"`
 	XXX_NoUnkeyedLiteral struct{}           `json:"-"`
@@ -136,22 +144,23 @@ func init() {
 func init() { proto.RegisterFile("admin.proto", fileDescriptor_73a7fc70dcc2027c) }
 
 var fileDescriptor_73a7fc70dcc2027c = []byte{
-	// 230 bytes of a gzipped FileDescriptorProto
-	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0xe2, 0x4e, 0x4c, 0xc9, 0xcd,
-	0xcc, 0xd3, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0x62, 0x4e, 0x2c, 0xc8, 0x94, 0xe2, 0x2f, 0x4a,
-	0x2d, 0xce, 0x2f, 0x2d, 0x4a, 0x4e, 0x2d, 0x86, 0x88, 0x2a, 0x85, 0x70, 0x89, 0xf8, 0x64, 0x16,
-	0x97, 0xb8, 0xe4, 0x27, 0x97, 0xe6, 0xa6, 0xe6, 0x95, 0x14, 0x07, 0xa5, 0x16, 0x96, 0xa6, 0x16,
-	0x97, 0x08, 0xc9, 0x73, 0x71, 0x17, 0x14, 0xa5, 0x96, 0x65, 0xe6, 0x97, 0x16, 0xc7, 0x67, 0xa6,
-	0x48, 0x30, 0x2a, 0x30, 0x6a, 0x70, 0x06, 0x71, 0xc1, 0x84, 0x3c, 0x53, 0x84, 0xa4, 0xb9, 0x38,
-	0x0b, 0x12, 0xd3, 0x53, 0xe3, 0x8b, 0x33, 0xab, 0x52, 0x25, 0x98, 0x14, 0x18, 0x35, 0x58, 0x83,
-	0x38, 0x40, 0x02, 0xc1, 0x99, 0x55, 0xa9, 0x4a, 0xde, 0x5c, 0xa2, 0x68, 0xa6, 0x16, 0x17, 0xe4,
-	0xe7, 0x15, 0xa7, 0x0a, 0x19, 0x71, 0x71, 0xa6, 0xc0, 0x04, 0x25, 0x18, 0x15, 0x98, 0x35, 0xb8,
-	0x8d, 0x44, 0xf4, 0x12, 0x0b, 0x32, 0xf5, 0x60, 0x4a, 0x83, 0x4b, 0x73, 0x73, 0x13, 0x8b, 0x2a,
-	0x83, 0x10, 0xca, 0x8c, 0x02, 0xb9, 0x58, 0x1d, 0x41, 0xfe, 0x10, 0xf2, 0xe0, 0xe2, 0x45, 0x31,
-	0x55, 0x48, 0x12, 0xac, 0x15, 0x9b, 0xfb, 0xa5, 0xa4, 0xb0, 0x49, 0x41, 0x1c, 0xa1, 0xc4, 0xe0,
-	0x24, 0x70, 0xe2, 0x91, 0x1c, 0xe3, 0x85, 0x47, 0x72, 0x8c, 0x0f, 0x1e, 0xc9, 0x31, 0xce, 0x78,
-	0x2c, 0xc7, 0x90, 0xc4, 0x06, 0x0e, 0x0e, 0x63, 0x40, 0x00, 0x00, 0x00, 0xff, 0xff, 0x42, 0xa1,
-	0x47, 0xd0, 0x33, 0x01, 0x00, 0x00,
+	// 254 bytes of a gzipped FileDescriptorProto
+	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x90, 0xcf, 0x4a, 0xc3, 0x40,
+	0x10, 0x87, 0xbb, 0x86, 0x4a, 0x33, 0x41, 0x94, 0xa5, 0x42, 0x8c, 0x18, 0x43, 0x4e, 0x39, 0xe5,
+	0x10, 0x9f, 0x40, 0x11, 0x51, 0xf4, 0x62, 0xfa, 0x00, 0x61, 0x6d, 0x46, 0x99, 0x43, 0xb2, 0xeb,
+	0x4e, 0x56, 0xb1, 0x4f, 0xe2, 0x23, 0x79, 0xf4, 0x11, 0x24, 0xbe, 0x88, 0xb4, 0x21, 0x88, 0xd2,
+	0xeb, 0x37, 0xbf, 0xf9, 0xf3, 0x0d, 0x04, 0xaa, 0x6e, 0xa8, 0xcd, 0x8d, 0xd5, 0x9d, 0x96, 0x9e,
+	0x32, 0x14, 0xed, 0x5b, 0x64, 0xed, 0xec, 0x12, 0x79, 0xa0, 0x29, 0xc3, 0xfc, 0x8e, 0xb8, 0xbb,
+	0xd4, 0x4b, 0xd7, 0x60, 0xdb, 0x71, 0x89, 0xcf, 0x0e, 0xb9, 0x93, 0xa7, 0x10, 0x18, 0x8b, 0x2f,
+	0xa4, 0x1d, 0x57, 0x54, 0x87, 0x22, 0x11, 0x99, 0x5f, 0xc2, 0x88, 0x6e, 0x6a, 0x79, 0x0c, 0xbe,
+	0x51, 0x4f, 0x58, 0x31, 0xad, 0x30, 0xdc, 0x49, 0x44, 0x36, 0x2d, 0x67, 0x6b, 0xb0, 0xa0, 0x15,
+	0xca, 0x13, 0x00, 0xe2, 0xea, 0x51, 0xdb, 0x57, 0x65, 0xeb, 0xd0, 0x4b, 0x44, 0x36, 0x2b, 0x7d,
+	0xe2, 0xab, 0x01, 0xa4, 0xb7, 0x70, 0xf8, 0x6f, 0x29, 0x1b, 0xdd, 0x32, 0xca, 0x02, 0xfc, 0x7a,
+	0x84, 0xa1, 0x48, 0xbc, 0x2c, 0x28, 0xe6, 0xb9, 0x32, 0x94, 0x8f, 0xd1, 0x85, 0x6b, 0x1a, 0x65,
+	0xdf, 0xca, 0xdf, 0x58, 0x71, 0x0f, 0xd3, 0xf3, 0xb5, 0xa6, 0xbc, 0x86, 0xbd, 0x3f, 0x53, 0xe5,
+	0xd1, 0xa6, 0x75, 0x9b, 0x5e, 0x14, 0x6d, 0x2b, 0x0d, 0x47, 0xa4, 0x93, 0x8b, 0x83, 0x8f, 0x3e,
+	0x16, 0x9f, 0x7d, 0x2c, 0xbe, 0xfa, 0x58, 0xbc, 0x7f, 0xc7, 0x93, 0x87, 0xdd, 0xcd, 0xb7, 0xce,
+	0x7e, 0x02, 0x00, 0x00, 0xff, 0xff, 0x34, 0x37, 0x57, 0x21, 0x52, 0x01, 0x00, 0x00,
 }
 
 // Reference imports to suppress errors if they are not otherwise used.
@@ -258,6 +267,16 @@ func (m *ListDocumentsRequest) MarshalToSizedBuffer(dAtA []byte) (int, error) {
 		i -= len(m.XXX_unrecognized)
 		copy(dAtA[i:], m.XXX_unrecognized)
 	}
+	if m.IsForward {
+		i--
+		if m.IsForward {
+			dAtA[i] = 1
+		} else {
+			dAtA[i] = 0
+		}
+		i--
+		dAtA[i] = 0x18
+	}
 	if m.PageSize != 0 {
 		i = encodeVarintAdmin(dAtA, i, uint64(m.PageSize))
 		i--
@@ -338,6 +357,9 @@ func (m *ListDocumentsRequest) Size() (n int) {
 	if m.PageSize != 0 {
 		n += 1 + sovAdmin(uint64(m.PageSize))
 	}
+	if m.IsForward {
+		n += 2
+	}
 	if m.XXX_unrecognized != nil {
 		n += len(m.XXX_unrecognized)
 	}
@@ -448,6 +470,26 @@ func (m *ListDocumentsRequest) Unmarshal(dAtA []byte) error {
 					break
 				}
 			}
+		case 3:
+			if wireType != 0 {
+				return fmt.Errorf("proto: wrong wireType = %d for field IsForward", wireType)
+			}
+			var v int
+			for shift := uint(0); ; shift += 7 {
+				if shift >= 64 {
+					return ErrIntOverflowAdmin
+				}
+				if iNdEx >= l {
+					return io.ErrUnexpectedEOF
+				}
+				b := dAtA[iNdEx]
+				iNdEx++
+				v |= int(b&0x7F) << shift
+				if b < 0x80 {
+					break
+				}
+			}
+			m.IsForward = bool(v != 0)
 		default:
 			iNdEx = preIndex
 			skippy, err := skipAdmin(dAtA[iNdEx:])
diff --git a/api/admin.proto b/api/admin.proto
index e0be2cd8..b383b7bc 100644
--- a/api/admin.proto
+++ b/api/admin.proto
@@ -27,6 +27,7 @@ service Admin {
 message ListDocumentsRequest {
   string previous_id = 1;
   int32 page_size = 2;
+  bool is_forward = 3;
 }
 
 message ListDocumentsResponse {
diff --git a/yorkie/admin/server.go b/yorkie/admin/server.go
index 362c0e93..50fe56a9 100644
--- a/yorkie/admin/server.go
+++ b/yorkie/admin/server.go
@@ -120,6 +120,7 @@ func (s *Server) ListDocuments(
 		s.backend,
 		db.ID(req.PreviousId),
 		int(req.PageSize),
+		req.IsForward,
 	)
 	if err != nil {
 		return nil, err
diff --git a/yorkie/backend/db/db.go b/yorkie/backend/db/db.go
index f2105ea7..5643c18b 100644
--- a/yorkie/backend/db/db.go
+++ b/yorkie/backend/db/db.go
@@ -124,5 +124,6 @@ type DB interface {
 		ctx context.Context,
 		previousID ID,
 		pageSize int,
+		isForward bool,
 	) ([]*DocInfo, error)
 }
diff --git a/yorkie/backend/db/memory/db.go b/yorkie/backend/db/memory/db.go
index 1a8c0bbb..1cf74746 100644
--- a/yorkie/backend/db/memory/db.go
+++ b/yorkie/backend/db/memory/db.go
@@ -567,15 +567,34 @@ func (d *DB) FindDocInfosByPreviousIDAndPageSize(
 	ctx context.Context,
 	previousID db.ID,
 	pageSize int,
+	isForward bool,
 ) ([]*db.DocInfo, error) {
 	txn := d.db.Txn(false)
 	defer txn.Abort()
 
-	iterator, err := txn.LowerBound(
-		tblDocuments,
-		"id",
-		previousID.String(),
-	)
+	var iterator memdb.ResultIterator
+	var err error
+	if isForward {
+		iterator, err = txn.LowerBound(
+			tblDocuments,
+			"id",
+			previousID.String(),
+		)
+	} else {
+		if previousID == "" {
+			iterator, err = txn.GetReverse(
+				tblDocuments,
+				"id",
+			)
+		} else {
+			iterator, err = txn.ReverseLowerBound(
+				tblDocuments,
+				"id",
+				previousID.String(),
+			)
+		}
+	}
+
 	if err != nil {
 		return nil, err
 	}
diff --git a/yorkie/backend/db/mongo/client.go b/yorkie/backend/db/mongo/client.go
index 2293d32e..6bb4b470 100644
--- a/yorkie/backend/db/mongo/client.go
+++ b/yorkie/backend/db/mongo/client.go
@@ -558,6 +558,7 @@ func (c *Client) FindDocInfosByPreviousIDAndPageSize(
 	ctx context.Context,
 	previousID db.ID,
 	pageSize int,
+	isForward bool,
 ) ([]*db.DocInfo, error) {
 	filter := bson.M{}
 	if previousID != "" {
@@ -565,18 +566,22 @@ func (c *Client) FindDocInfosByPreviousIDAndPageSize(
 		if err != nil {
 			return nil, err
 		}
+
+		key := "$lt"
+		if isForward {
+			key = "$gt"
+		}
 		filter = bson.M{
-			"_id": bson.M{
-				"$gt": encodedPreviousID,
-			},
+			"_id": bson.M{key: encodedPreviousID},
 		}
 	}
 
-	cursor, err := c.collection(colDocuments).Find(
-		ctx,
-		filter,
-		options.Find().SetLimit(int64(pageSize)),
-	)
+	opts := options.Find().SetLimit(int64(pageSize))
+	if !isForward {
+		opts = opts.SetSort(map[string]int{"_id": -1})
+	}
+
+	cursor, err := c.collection(colDocuments).Find(ctx, filter, opts)
 	if err != nil {
 		logging.From(ctx).Error(err)
 		return nil, err
diff --git a/yorkie/documents/admin.go b/yorkie/documents/admin.go
index 183fc604..c8fc1aca 100644
--- a/yorkie/documents/admin.go
+++ b/yorkie/documents/admin.go
@@ -31,8 +31,9 @@ func ListDocumentSummaries(
 	be *backend.Backend,
 	previousID db.ID,
 	pageSize int,
+	isForward bool,
 ) ([]*types.DocumentSummary, error) {
-	docInfo, err := be.DB.FindDocInfosByPreviousIDAndPageSize(ctx, previousID, pageSize)
+	docInfo, err := be.DB.FindDocInfosByPreviousIDAndPageSize(ctx, previousID, pageSize, isForward)
 	if err != nil {
 		return nil, err
 	}


Test patch:
diff --git a/yorkie/backend/db/memory/db_test.go b/yorkie/backend/db/memory/db_test.go
index 45025772..af411d6f 100644
--- a/yorkie/backend/db/memory/db_test.go
+++ b/yorkie/backend/db/memory/db_test.go
@@ -192,33 +192,49 @@ func TestDB(t *testing.T) {
 		assert.Equal(t, uint64(1), snapshot.ServerSeq)
 	})
 
-	t.Run("paging docInfo test", func(t *testing.T) {
+	t.Run("docInfo pagination test", func(t *testing.T) {
 		localDB, err := memory.New()
 		assert.NoError(t, err)
 
-		clientInfo, _ := localDB.ActivateClient(ctx, t.Name())
-
-		var givenKeys []string
-		for i := 0; i < 11; i++ {
-			docInfo, err := localDB.FindDocInfoByKey(ctx, clientInfo, fmt.Sprintf("tests$%s-%d", t.Name(), i), true)
-			assert.NoError(t, err)
-			givenKeys = append(givenKeys, docInfo.CombinedKey)
+		assertKeys := func(expectedKeys []string, infos []*db.DocInfo) {
+			var keys []string
+			for _, info := range infos {
+				keys = append(keys, info.CombinedKey)
+			}
+			assert.EqualValues(t, expectedKeys, keys)
 		}
 
-		var keys []string
-		previousID := db.ID("")
-		for {
-			docInfos, err := localDB.FindDocInfosByPreviousIDAndPageSize(ctx, previousID, 10)
+		pageSize := 5
+		totalSize := 9
+		clientInfo, _ := localDB.ActivateClient(ctx, t.Name())
+		for i := 0; i < totalSize; i++ {
+			_, err := localDB.FindDocInfoByKey(ctx, clientInfo, fmt.Sprintf("%d", i), true)
 			assert.NoError(t, err)
-			if len(docInfos) == 0 {
-				break
-			}
-			for _, docInfo := range docInfos {
-				keys = append(keys, docInfo.CombinedKey)
-			}
-			previousID = docInfos[len(docInfos)-1].ID
 		}
 
-		assert.Equal(t, givenKeys, keys)
+		// initial page, previousID is empty
+		infos, err := localDB.FindDocInfosByPreviousIDAndPageSize(ctx, "", pageSize, false)
+		assert.NoError(t, err)
+		assertKeys([]string{"8", "7", "6", "5", "4"}, infos)
+
+		// backward
+		infos, err = localDB.FindDocInfosByPreviousIDAndPageSize(ctx, infos[len(infos)-1].ID, pageSize, false)
+		assert.NoError(t, err)
+		assertKeys([]string{"3", "2", "1", "0"}, infos)
+
+		// backward again
+		emptyInfos, err := localDB.FindDocInfosByPreviousIDAndPageSize(ctx, infos[len(infos)-1].ID, pageSize, false)
+		assert.NoError(t, err)
+		assertKeys(nil, emptyInfos)
+
+		// forward
+		infos, err = localDB.FindDocInfosByPreviousIDAndPageSize(ctx, infos[0].ID, pageSize, true)
+		assert.NoError(t, err)
+		assertKeys([]string{"4", "5", "6", "7", "8"}, infos)
+
+		// forward again
+		emptyInfos, err = localDB.FindDocInfosByPreviousIDAndPageSize(ctx, infos[len(infos)-1].ID, pageSize, true)
+		assert.NoError(t, err)
+		assertKeys(nil, emptyInfos)
 	})
 }