Repository: yorkie-team/yorkie
Pull request #39 (2020-04-06 15:44:55)

Fix a bug where the index on an array was not valid
<!--  Thanks for sending a pull request! -->

**What type of PR is this?**
> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespace from that line:
>
> /kind api-change

/kind bug

> /kind cleanup
> /kind deprecation
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature

**What this PR does / why we need it**:

Fix a bug where the index on an array was not valid

**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

```


codecov[bot]: # [Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/39?src=pr&el=h1) Report
> Merging [#39](https://codecov.io/gh/yorkie-team/yorkie/pull/39?src=pr&el=desc) into [master](https://codecov.io/gh/yorkie-team/yorkie/commit/4c2895906b337641cb8bbaabd7efeb23ba187bf9&el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/yorkie-team/yorkie/pull/39/graphs/tree.svg?width=650&height=150&src=pr&token=P6COB1ZNIC)](https://codecov.io/gh/yorkie-team/yorkie/pull/39?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
- Coverage   35.38%   34.85%   -0.54%     
==========================================
  Files          17       17              
  Lines        1933     1934       +1     
==========================================
- Hits          684      674      -10     
- Misses       1180     1190      +10     
- Partials       69       70       +1     
```


| [Impacted Files](https://codecov.io/gh/yorkie-team/yorkie/pull/39?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/document/json/rga.go](https://codecov.io/gh/yorkie-team/yorkie/pull/39/diff?src=pr&el=tree#diff-cGtnL2RvY3VtZW50L2pzb24vcmdhLmdv) | `48.95% <0.00%> (-0.52%)` | :arrow_down: |
| [pkg/llrb/llrb.go](https://codecov.io/gh/yorkie-team/yorkie/pull/39/diff?src=pr&el=tree#diff-cGtnL2xscmIvbGxyYi5nbw==) | `71.42% <0.00%> (-6.81%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/39?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/yorkie-team/yorkie/pull/39?src=pr&el=footer). Last update [4c28959...b330c9c](https://codecov.io/gh/yorkie-team/yorkie/pull/39?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/document/json/rga.go b/pkg/document/json/rga.go
index 62333cd8..33914336 100644
--- a/pkg/document/json/rga.go
+++ b/pkg/document/json/rga.go
@@ -149,12 +149,14 @@ func (a *RGA) Get(idx int) Element {
 		if current == nil {
 			break
 		}
-		if idx == 0 {
+		if idx == 0 && !current.isDeleted() {
 			return current.elem
 		}
 
 		current = current.next
-		idx--
+		if !current.isDeleted() {
+			idx--
+		}
 	}
 	return nil
 }
diff --git a/pkg/document/proxy/array_proxy.go b/pkg/document/proxy/array_proxy.go
index 2e7d985e..39fed906 100644
--- a/pkg/document/proxy/array_proxy.go
+++ b/pkg/document/proxy/array_proxy.go
@@ -131,21 +131,7 @@ func (p *ArrayProxy) Remove(idx int) json.Element {
 func (p *ArrayProxy) addInternal(
 	creator func(ticket *time.Ticket) json.Element,
 ) json.Element {
-	ticket := p.context.IssueTimeTicket()
-	proxy := creator(ticket)
-	value := toOriginal(proxy)
-
-	p.context.Push(operation.NewAdd(
-		p.Array.CreatedAt(),
-		p.Array.LastCreatedAt(),
-		value.DeepCopy(),
-		ticket,
-	))
-
-	p.Add(value)
-	p.context.RegisterElement(value)
-
-	return proxy
+	return p.insertAfterInternal(p.Array.LastCreatedAt(), creator)
 }
 
 func (p *ArrayProxy) insertAfterInternal(


Test patch:
diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go
index bd89e412..6281ab12 100644
--- a/pkg/document/document_test.go
+++ b/pkg/document/document_test.go
@@ -127,6 +127,10 @@ func TestDocument(t *testing.T) {
 			root.GetArray("k1").InsertStringAfter(0, "2")
 			assert.Equal(t, `{"k1":["1","2","3"]}`, root.Marshal())
 			assert.Equal(t, 3, root.GetArray("k1").Len())
+
+			root.GetArray("k1").InsertStringAfter(2, "4")
+			assert.Equal(t, `{"k1":["1","2","3","4"]}`, root.Marshal())
+			assert.Equal(t, 4, root.GetArray("k1").Len())
 			return nil
 		})
 		assert.Nil(t, err)