Repository: yorkie-team/yorkie
Pull request #271 (2021-11-15 07:21:40)

Set registry for conversion between `ObjectID` and `db.ID`
<!--  Thanks for sending a pull request! -->

**What this PR does / why we need it**:

Add a registry for conversion between `ObjectID` and `db.ID`

We didn't set up codecs before and did the encoding and decoding manually. In this PR, I set the codec provided by the MongoDB driver and process it with a shorter code.

https://github.com/mongodb/mongo-go-driver/blob/master/bson/registry.go

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #

**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

```

**Checklist**:
- [x] Added relevant tests or not required
- [x] Didn't break anything


codecov[bot]: # [Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/271?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) Report
> Merging [#271](https://codecov.io/gh/yorkie-team/yorkie/pull/271?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) (ecb4d53) into [main](https://codecov.io/gh/yorkie-team/yorkie/commit/d23fc14d0f7ef3356d83dc43ffb3d3e8a1e256db?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) (d23fc14) will **increase** coverage by `0.94%`.
> The diff coverage is `40.90%`.

[![Impacted file tree graph](https://codecov.io/gh/yorkie-team/yorkie/pull/271/graphs/tree.svg?width=650&height=150&src=pr&token=P6COB1ZNIC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team)](https://codecov.io/gh/yorkie-team/yorkie/pull/271?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team)

```diff
@@            Coverage Diff             @@
##             main     #271      +/-   ##
==========================================
+ Coverage   49.32%   50.26%   +0.94%     
==========================================
  Files          53       53              
  Lines        4428     4363      -65     
==========================================
+ Hits         2184     2193       +9     
+ Misses       2032     1958      -74     
  Partials      212      212              
```


| [Impacted Files](https://codecov.io/gh/yorkie-team/yorkie/pull/271?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/change\_info.go](https://codecov.io/gh/yorkie-team/yorkie/pull/271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-eW9ya2llL2JhY2tlbmQvZGIvY2hhbmdlX2luZm8uZ28=) | `26.92% <ø> (ø)` | |
| [yorkie/backend/db/client\_info.go](https://codecov.io/gh/yorkie-team/yorkie/pull/271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-eW9ya2llL2JhY2tlbmQvZGIvY2xpZW50X2luZm8uZ28=) | `53.84% <ø> (ø)` | |
| [yorkie/backend/db/doc\_info.go](https://codecov.io/gh/yorkie-team/yorkie/pull/271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-eW9ya2llL2JhY2tlbmQvZGIvZG9jX2luZm8uZ28=) | `0.00% <ø> (ø)` | |
| [yorkie/backend/db/mongo/client.go](https://codecov.io/gh/yorkie-team/yorkie/pull/271/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/mongo/registry.go](https://codecov.io/gh/yorkie-team/yorkie/pull/271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-eW9ya2llL2JhY2tlbmQvZGIvbW9uZ28vcmVnaXN0cnkuZ28=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/271?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/271?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team). Last update [d23fc14...ecb4d53](https://codecov.io/gh/yorkie-team/yorkie/pull/271?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).


Patch:
diff --git a/yorkie/backend/db/change_info.go b/yorkie/backend/db/change_info.go
index dd7bcee0..d87913ce 100644
--- a/yorkie/backend/db/change_info.go
+++ b/yorkie/backend/db/change_info.go
@@ -29,11 +29,11 @@ import (
 
 // ChangeInfo is a structure representing information of a change.
 type ChangeInfo struct {
-	DocID      ID       `bson:"doc_id_fake"`
+	DocID      ID       `bson:"doc_id"`
 	ServerSeq  uint64   `bson:"server_seq"`
 	ClientSeq  uint32   `bson:"client_seq"`
 	Lamport    uint64   `bson:"lamport"`
-	Actor      ID       `bson:"actor_fake"`
+	Actor      ID       `bson:"actor"`
 	Message    string   `bson:"message"`
 	Operations [][]byte `bson:"operations"`
 }
diff --git a/yorkie/backend/db/client_info.go b/yorkie/backend/db/client_info.go
index 5dbdd459..4424ca1d 100644
--- a/yorkie/backend/db/client_info.go
+++ b/yorkie/backend/db/client_info.go
@@ -52,7 +52,7 @@ type ClientDocInfo struct {
 
 // ClientInfo is a structure representing information of a client.
 type ClientInfo struct {
-	ID        ID                    `bson:"_id_fake"`
+	ID        ID                    `bson:"_id"`
 	Key       string                `bson:"key"`
 	Status    string                `bson:"status"`
 	Documents map[ID]*ClientDocInfo `bson:"documents"`
diff --git a/yorkie/backend/db/doc_info.go b/yorkie/backend/db/doc_info.go
index 916917c9..2e08cb76 100644
--- a/yorkie/backend/db/doc_info.go
+++ b/yorkie/backend/db/doc_info.go
@@ -24,10 +24,10 @@ import (
 
 // DocInfo is a structure representing information of the document.
 type DocInfo struct {
-	ID         ID        `bson:"_id_fake"`
+	ID         ID        `bson:"_id"`
 	Key        string    `bson:"key"`
 	ServerSeq  uint64    `bson:"server_seq"`
-	Owner      ID        `bson:"owner_fake"`
+	Owner      ID        `bson:"owner"`
 	CreatedAt  time.Time `bson:"created_at"`
 	AccessedAt time.Time `bson:"accessed_at"`
 	UpdatedAt  time.Time `bson:"updated_at"`
diff --git a/yorkie/backend/db/mongo/client.go b/yorkie/backend/db/mongo/client.go
index e35f35a0..6659d330 100644
--- a/yorkie/backend/db/mongo/client.go
+++ b/yorkie/backend/db/mongo/client.go
@@ -47,7 +47,9 @@ func Dial(conf *Config) (*Client, error) {
 
 	client, err := mongo.Connect(
 		ctx,
-		options.Client().ApplyURI(conf.ConnectionURI),
+		options.Client().
+			ApplyURI(conf.ConnectionURI).
+			SetRegistry(newRegistryBuilder().Build()),
 	)
 	if err != nil {
 		log.Logger.Error(err)
@@ -120,7 +122,7 @@ func (c *Client) ActivateClient(ctx context.Context, key string) (*db.ClientInfo
 		})
 	}
 
-	if err = decodeClientInfo(result, &clientInfo); err != nil {
+	if err = result.Decode(&clientInfo); err != nil {
 		return nil, err
 	}
 
@@ -144,7 +146,7 @@ func (c *Client) DeactivateClient(ctx context.Context, clientID db.ID) (*db.Clie
 		},
 	})
 
-	if err := decodeClientInfo(res, &clientInfo); err != nil {
+	if err := res.Decode(&clientInfo); err != nil {
 		if err == mongo.ErrNoDocuments {
 			return nil, fmt.Errorf("%s: %w", clientID, db.ErrClientNotFound)
 		}
@@ -165,7 +167,7 @@ func (c *Client) FindClientInfoByID(ctx context.Context, clientID db.ID) (*db.Cl
 	result := c.collection(ColClients).FindOne(ctx, bson.M{
 		"_id": encodedClientID,
 	})
-	if err := decodeClientInfo(result, &clientInfo); err != nil {
+	if err := result.Decode(&clientInfo); err != nil {
 		if err == mongo.ErrNoDocuments {
 			return nil, fmt.Errorf("%s: %w", clientID, db.ErrClientNotFound)
 		}
@@ -278,7 +280,7 @@ func (c *Client) FindDocInfoByKey(
 			return nil, result.Err()
 		}
 	}
-	if err := decodeDocInfo(result, &docInfo); err != nil {
+	if err := result.Decode(&docInfo); err != nil {
 		return nil, err
 	}
 
@@ -411,7 +413,7 @@ func (c *Client) FindChangeInfosBetweenServerSeqs(
 		return nil, err
 	}
 
-	var changes []*db.ChangeInfo
+	var infos []*db.ChangeInfo
 	cursor, err := c.collection(ColChanges).Find(ctx, bson.M{
 		"doc_id": encodedDocID,
 		"server_seq": bson.M{
@@ -424,26 +426,12 @@ func (c *Client) FindChangeInfosBetweenServerSeqs(
 		return nil, err
 	}
 
-	defer func() {
-		if err := cursor.Close(ctx); err != nil {
-			log.Logger.Error(err)
-		}
-	}()
-
-	for cursor.Next(ctx) {
-		var changeInfo db.ChangeInfo
-		if err := decodeChangeInfo(cursor, &changeInfo); err != nil {
-			return nil, err
-		}
-		changes = append(changes, &changeInfo)
-	}
-
-	if cursor.Err() != nil {
+	if err := cursor.All(ctx, &infos); err != nil {
 		log.Logger.Error(cursor.Err())
 		return nil, cursor.Err()
 	}
 
-	return changes, nil
+	return infos, nil
 }
 
 // UpdateAndFindMinSyncedTicket updates the given serverSeq of the given client
@@ -505,7 +493,7 @@ func (c *Client) UpdateAndFindMinSyncedTicket(
 		log.Logger.Error(result.Err())
 		return nil, result.Err()
 	}
-	if err := decodeSyncedSeqInfo(result, &syncedSeqInfo); err != nil {
+	if err := result.Decode(&syncedSeqInfo); err != nil {
 		return nil, err
 	}
 
@@ -549,7 +537,7 @@ func (c *Client) FindLastSnapshotInfo(
 		return nil, result.Err()
 	}
 
-	if err := decodeSnapshotInfo(result, snapshotInfo); err != nil {
+	if err := result.Decode(snapshotInfo); err != nil {
 		return nil, err
 	}
 
@@ -580,7 +568,7 @@ func (c *Client) findTicketByServerSeq(
 		return nil, result.Err()
 	}
 
-	if err := decodeChangeInfo(result, &changeInfo); err != nil {
+	if err := result.Decode(&changeInfo); err != nil {
 		return nil, err
 	}
 
diff --git a/yorkie/backend/db/mongo/decoder.go b/yorkie/backend/db/mongo/decoder.go
deleted file mode 100644
index 7a81fb5f..00000000
--- a/yorkie/backend/db/mongo/decoder.go
+++ /dev/null
@@ -1,119 +0,0 @@
-package mongo
-
-import (
-	"go.mongodb.org/mongo-driver/bson/primitive"
-
-	"github.com/yorkie-team/yorkie/internal/log"
-	"github.com/yorkie-team/yorkie/yorkie/backend/db"
-)
-
-type decoder interface {
-	Decode(val interface{}) error
-}
-
-func decodeClientInfo(
-	result decoder,
-	clientInfo *db.ClientInfo,
-) error {
-	idHolder := struct {
-		ID primitive.ObjectID `bson:"_id"`
-	}{}
-	if err := result.Decode(&idHolder); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	if err := result.Decode(clientInfo); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	clientInfo.ID = decodeID(idHolder.ID)
-	return nil
-}
-
-func decodeDocInfo(
-	result decoder,
-	docInfo *db.DocInfo,
-) error {
-	idHolder := struct {
-		ID    primitive.ObjectID `bson:"_id"`
-		Owner primitive.ObjectID `bson:"owner"`
-	}{}
-	if err := result.Decode(&idHolder); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	if err := result.Decode(&docInfo); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	docInfo.ID = decodeID(idHolder.ID)
-	docInfo.Owner = decodeID(idHolder.Owner)
-	return nil
-}
-
-func decodeChangeInfo(
-	cursor decoder,
-	changeInfo *db.ChangeInfo,
-) error {
-	idHolder := struct {
-		DocID primitive.ObjectID `bson:"doc_id"`
-		Actor primitive.ObjectID `bson:"actor"`
-	}{}
-	if err := cursor.Decode(&idHolder); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	if err := cursor.Decode(&changeInfo); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	changeInfo.DocID = decodeID(idHolder.DocID)
-	changeInfo.Actor = decodeID(idHolder.Actor)
-	return nil
-}
-
-func decodeSyncedSeqInfo(
-	result decoder,
-	syncedSeqInfo *db.SyncedSeqInfo,
-) error {
-	idHolder := struct {
-		DocID    primitive.ObjectID `bson:"doc_id"`
-		ClientID primitive.ObjectID `bson:"client_id"`
-	}{}
-	if err := result.Decode(&idHolder); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	if err := result.Decode(&syncedSeqInfo); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	syncedSeqInfo.DocID = decodeID(idHolder.DocID)
-	syncedSeqInfo.ClientID = decodeID(idHolder.ClientID)
-	return nil
-}
-
-func decodeSnapshotInfo(
-	result decoder,
-	snapshotInfo *db.SnapshotInfo,
-) error {
-	idHolder := struct {
-		ID    primitive.ObjectID `bson:"_id"`
-		DocID primitive.ObjectID `bson:"doc_id"`
-	}{}
-	if err := result.Decode(&idHolder); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	if err := result.Decode(&snapshotInfo); err != nil {
-		log.Logger.Error(err)
-		return err
-	}
-	snapshotInfo.ID = decodeID(idHolder.ID)
-	snapshotInfo.DocID = decodeID(idHolder.DocID)
-	return nil
-}
-
-func decodeID(id primitive.ObjectID) db.ID {
-	return db.ID(id.Hex())
-}
diff --git a/yorkie/backend/db/mongo/registry.go b/yorkie/backend/db/mongo/registry.go
new file mode 100644
index 00000000..7db5aec7
--- /dev/null
+++ b/yorkie/backend/db/mongo/registry.go
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 The Yorkie Authors. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package mongo
+
+import (
+	"reflect"
+
+	"go.mongodb.org/mongo-driver/bson"
+	"go.mongodb.org/mongo-driver/bson/bsoncodec"
+	"go.mongodb.org/mongo-driver/bson/bsonoptions"
+
+	"github.com/yorkie-team/yorkie/yorkie/backend/db"
+)
+
+func newRegistryBuilder() *bsoncodec.RegistryBuilder {
+	rb := bsoncodec.NewRegistryBuilder()
+
+	bsoncodec.DefaultValueEncoders{}.RegisterDefaultEncoders(rb)
+	bsoncodec.DefaultValueDecoders{}.RegisterDefaultDecoders(rb)
+	bson.PrimitiveCodecs{}.RegisterPrimitiveCodecs(rb)
+
+	rb.RegisterCodec(
+		reflect.TypeOf(db.ID("")),
+		bsoncodec.NewStringCodec(bsonoptions.StringCodec().SetDecodeObjectIDAsHex(true)),
+	)
+
+	return rb
+}
diff --git a/yorkie/backend/db/snapshot_info.go b/yorkie/backend/db/snapshot_info.go
index 7a6de73f..eee710b1 100644
--- a/yorkie/backend/db/snapshot_info.go
+++ b/yorkie/backend/db/snapshot_info.go
@@ -6,8 +6,8 @@ import (
 
 // SnapshotInfo is a structure representing information of the snapshot.
 type SnapshotInfo struct {
-	ID        ID        `bson:"_id_fake"`
-	DocID     ID        `bson:"doc_id_fake"`
+	ID        ID        `bson:"_id"`
+	DocID     ID        `bson:"doc_id"`
 	ServerSeq uint64    `bson:"server_seq"`
 	Snapshot  []byte    `bson:"snapshot"`
 	CreatedAt time.Time `bson:"created_at"`
diff --git a/yorkie/backend/db/synced_seq_info.go b/yorkie/backend/db/synced_seq_info.go
index ce479111..4518f3b8 100644
--- a/yorkie/backend/db/synced_seq_info.go
+++ b/yorkie/backend/db/synced_seq_info.go
@@ -3,7 +3,7 @@ package db
 // SyncedSeqInfo is a structure representing information about the synchronized
 // sequence for each client.
 type SyncedSeqInfo struct {
-	DocID     ID     `bson:"doc_id_fake"`
-	ClientID  ID     `bson:"client_id_fake"`
+	DocID     ID     `bson:"doc_id"`
+	ClientID  ID     `bson:"client_id"`
 	ServerSeq uint64 `bson:"server_seq"`
 }


Test patch:
diff --git a/yorkie/backend/db/mongo/registry_test.go b/yorkie/backend/db/mongo/registry_test.go
new file mode 100644
index 00000000..f6808ee7
--- /dev/null
+++ b/yorkie/backend/db/mongo/registry_test.go
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 The Yorkie Authors. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package mongo
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"go.mongodb.org/mongo-driver/bson"
+	"go.mongodb.org/mongo-driver/bson/primitive"
+
+	"github.com/yorkie-team/yorkie/yorkie/backend/db"
+)
+
+func TestRegistry(t *testing.T) {
+	registry := newRegistryBuilder().Build()
+
+	id := db.ID(primitive.NewObjectID().Hex())
+	data, err := bson.MarshalWithRegistry(registry, bson.M{
+		"_id": id,
+	})
+	assert.NoError(t, err)
+
+	info := db.ClientInfo{}
+	assert.NoError(t, bson.UnmarshalWithRegistry(registry, data, &info))
+	assert.Equal(t, id, info.ID)
+
+}