Bug

Status Update

No update yet.

Description

fa...@gmail.com

created issue #1

Feb 15, 2024 03:14PM

Step to reproduce

Create a Gerrit instance with project P1
Create change C1 with change number N
Import a project P1 into a different Gerrit primary as described, example `gerrit.importedServerId = <other server id>
Make sure the destination server has a change C2 with project P2 and change number N, which is the same change number as C1
Add a draft comment to C1 of project C1

Expected result

Draft comment should only be available in C1 of project C1

Observed result

Both C1 and C2 contain the same draft comment.

Background

Draft comments information are stored in All-Users in refs like: refs/draft-comments/<changeNumberShard>/<changeNumber>/<accountId>

Since the change number is not unique, a draft comment is assigned to all the changes with the same change number.

Possible solutions are:

use the document ID (aka virtualId, aka legacy_id) instead of the change number in the special ref
add the project name in the special ref
move the starred changes to the project repository

Considering this fix will need to be done in a stable branch, the first solution is possibly the least impactful.

All the solutions will require a migration strategy.

Comments

al...@gmail.com <al...@gmail.com> #2Feb 21, 2024 09:25AM

@fa...@gmail.com I have 2 questions:

Is there a specific Gerrit version?
Is there any plugin apart from the core ones that need to be installed?
Am I right to think that 2 independent Gerrit instances will have the first change created in each instance to have changeNumber 1?

po...@gmail.com <po...@gmail.com> #3Feb 21, 2024 09:29AM

Is there a specific Gerrit version?

See "found in" field

Is there any plugin apart from the core ones that need to be installed?

Nope

Am I right to think that 2 independent Gerrit instances will have the first change created in each instance to have changeNumber 1?

yes

al...@gmail.com <al...@gmail.com> #4Feb 21, 2024 08:15PM

Comment has been deleted.

Message last modified on Feb 28, 2024 05:27PM

Screenshot 2024-02-21 at 20.04.55.png

43 KB

View

Download

al...@gmail.com <al...@gmail.com> #5Feb 21, 2024 08:19PM

Comment has been deleted.

Message last modified on Feb 29, 2024 04:09PM

Screenshot 2024-02-21 at 20.18.03.png

111 KB

View

Download

al...@gmail.com <al...@gmail.com> #6Feb 28, 2024 05:38PM

Comment has been deleted.

Message last modified on Feb 29, 2024 04:08PM

al...@gmail.com <al...@gmail.com> #7Feb 29, 2024 10:51AM

Comment has been deleted.

Message last modified on Feb 29, 2024 04:08PM

al...@gmail.com <al...@gmail.com> #8Feb 29, 2024 06:22PM

Acceptance criteria:

Scenario 1 - Create a draft comment

Create a draft comment in P2/CH1 (imported)
Check that the draft comment has been created in repo All-Users, i.e

66bcdaa330a9f50eaf172fd716b6e29e57ce6ad2 refs/draft-comments/57/268435457/1000000

where 268435457 is the virtualId

Draft comment is not shown in P1/CH1

Scenario 2 - Update a draft comment

Update the comment in P2/CH1 (imported)
Check that the draft comment has been updated in repo All-Users

Scenario 3 - Delete a draft comment

Delete draft comment in P2/CH1 (imported)
Check that the draft comment has been deleted in repo All-Users

Scenarios 1,2 and 3 applied to P1/CH1

Scenario 4 - Post the review

Post the review: P2/CH1
Check that the draft comment has been removed in repo All-Users
Check that there is a new comment in the change P2/CH1
Check the metadata of the change to make sure that the comment is there, i.e:
Refresh the change detail page and check that the comment is shown

ae4370c5c7b22e0a7b9c715d79a4e7740a604f7f refs/changes/01/1/meta

Check that the change P1/CH1 has not been affected.

Scenario 4 applied in P1/CH1

Scenario 5 - Check stream events

For each draft comment operation, a ref updated event has been generated with the right values.

Scenario 6 - Check changes API

List Change Drafts
List Revision Drafts
Get Draft
Create Draft
Update Draft
Delete Draft

Message last modified on Mar 2, 2024 08:22AM

ap...@google.com <ap...@google.com> #9Apr 5, 2024 08:24AM

Project: gerrit
Branch: master

commit 63751a46b0de2652917dd2118650f5a2cfd601af
Author: Dariusz Luksza <dariusz.luksza@gmail.com>
Date: Thu Apr 04 22:13:12 2024

Merge branch 'stable-3.9' into master

Due to significant API changes in the handling of draft comments between
`stable-3.7` and `master`, the change I108671395 is reverted in this
merge. A `master` specific fix for

Issue 325309574

must be implemented
in a separate change.

* stable-3.9: (27 commits)
Revert "During online reindexing of all changes skip changes already present"
Do not allow unload/restart plugins with ApiModule
Reset pageResultSize when PaginationType NONE back-fill results
Fix ReturnMissingNullable bug pattern flagged by error prone
Stop considering WIP changes "unmergeable"
Test submitting a change with WIP parent
Add test paginationType NONE to query only if there are more results
Revert urelated changes to external_plugin_deps.bzl
Fix paginationType NONE to run further queries only if needed
Use virtual change id for account patchset reviewdb clear action
Run tests in FakeQueryChangesTest for paginationType NONE
Fix detection of invalid SSH key algorithm
Demonstrate SshKeyUtil fails to validate invalid SSH keys
Fix starred changes clash after repo import from another site
gr-change-view: use change-model to update change object
Fix issue with gr-file-list-header not working correctly after writing a comment
Resolve draft comment reference ambiguity for imported changes
Bump JGit to c0b415fb0
Fix DereferenceWithNullBranch bug pattern flagged by error prone
Expose change's serverid in Change class
Move comment of PaginatingSource.read()
Fix visibility filter for changes when paginationType NONE
Add tests for pagination to back fill the results
Allow plugins to use PredicateArgs
Make ChangeNumberBitmapMaskAlgorithm a NOOP for local serverId
Set version to 3.7.9-SNAPSHOT
Set version to 3.7.8

Release-Notes: skip
Forward-Compatible: checked
Change-Id: Iad3359fa422ca64c7e2bb2d683fff1529007edb2

M java/com/google/gerrit/server/CommentsUtil.java
M java/com/google/gerrit/server/change/ChangeJson.java
M java/com/google/gerrit/server/index/OnlineReindexer.java
M java/com/google/gerrit/server/index/change/AllChangesIndexer.java
M java/com/google/gerrit/server/notedb/ChangeDraftNotesUpdate.java
M java/com/google/gerrit/server/notedb/ChangeNotes.java
M java/com/google/gerrit/server/notedb/DraftCommentNotes.java
M java/com/google/gerrit/server/notedb/StarredChangesUtilNoteDbImpl.java
M java/com/google/gerrit/server/query/change/ChangeData.java
M java/com/google/gerrit/sshd/SshKeyCacheImpl.java
M javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
M javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java

https://gerrit-review.googlesource.com/416839

ap...@google.com <ap...@google.com> #10Apr 5, 2024 01:51PM

Project: gerrit
Branch: master

commit 0c0e83bb4d0e71a5a3015ddbf21552fc22cd50bb
Author: Dariusz Luksza <dariusz.luksza@gmail.com>
Date: Mon Feb 26 18:37:31 2024

Re-apply: Resolve draft comment reference ambiguity for imported changes

Due to the complexity of `stable-3.9` merge to `master` the change
"Resolve draft comment reference ambiguity for imported changes`
(I108671395) was reverted in the merge commit.

This is re-applying that change separetly to `master`.

Bug:

Issue 325309574

Release-Notes: skip
Forward-Compatible: checked
Change-Id: Ic481a75d036c657a34144a9c1baed522250218d7

M java/com/google/gerrit/server/notedb/ChangeDraftNotesUpdate.java
M java/com/google/gerrit/server/notedb/ChangeNotes.java
M java/com/google/gerrit/server/notedb/DraftCommentNotes.java
M java/com/google/gerrit/server/notedb/DraftCommentsNotesReader.java

https://gerrit-review.googlesource.com/417801

br...@gmail.com <br...@gmail.com> #11Jan 19, 2025 09:10AM

Comment has been deleted.

Message last modified on Jan 19, 2025 09:10AM

Issue 325309574

Description

Step to reproduce

Expected result

Observed result

Background

Issue summary

Comments

al...@gmail.com <al...@gmail.com> #2Feb 21, 2024 09:25AM

po...@gmail.com <po...@gmail.com> #3Feb 21, 2024 09:29AM

al...@gmail.com <al...@gmail.com> #4Feb 21, 2024 08:15PM

al...@gmail.com <al...@gmail.com> #5Feb 21, 2024 08:19PM

al...@gmail.com <al...@gmail.com> #6Feb 28, 2024 05:38PM

al...@gmail.com <al...@gmail.com> #7Feb 29, 2024 10:51AM

al...@gmail.com <al...@gmail.com> #8Feb 29, 2024 06:22PM

ap...@google.com <ap...@google.com> #9Apr 5, 2024 08:24AM

ap...@google.com <ap...@google.com> #10Apr 5, 2024 01:51PM

br...@gmail.com <br...@gmail.com> #11Jan 19, 2025 09:10AM

Add comment

Issue metadata