From 74b2cf58fec708a0f9af9dd176855f8862342f07 Mon Sep 17 00:00:00 2001 From: tzahij Date: Sun, 16 Aug 2020 12:52:55 +0300 Subject: [PATCH] Feature/list commits with children (#458) --- catalog/cataloger_diff.go | 2 +- catalog/cataloger_list_commits.go | 15 ++- catalog/cataloger_list_commits_test.go | 151 +++++++++++++++++++++++-- catalog/db.go | 4 +- catalog/views.go | 2 +- 5 files changed, 159 insertions(+), 15 deletions(-) diff --git a/catalog/cataloger_diff.go b/catalog/cataloger_diff.go index c4c3f2a443e..7c6e10f5c35 100644 --- a/catalog/cataloger_diff.go +++ b/catalog/cataloger_diff.go @@ -160,7 +160,7 @@ func (c *cataloger) diffFromChild(tx db.Tx, childID, parentID int64) (Difference return nil, fmt.Errorf("child lineage failed: %w", err) } - childLineageValues := getLineageAsValues(childLineage, childID) + childLineageValues := getLineageAsValues(childLineage, childID, MaxCommitID) mainDiffFromChild := sqDiffFromChildV(parentID, childID, effectiveCommits.ParentEffectiveCommit, effectiveCommits.ChildEffectiveCommit, parentLineage, childLineageValues) diffFromChildSQL, args, err := mainDiffFromChild. Prefix("CREATE TEMP TABLE " + diffResultsTableName + " ON COMMIT DROP AS"). diff --git a/catalog/cataloger_list_commits.go b/catalog/cataloger_list_commits.go index 9dfe0b4b89b..4d47e62a7a1 100644 --- a/catalog/cataloger_list_commits.go +++ b/catalog/cataloger_list_commits.go @@ -34,14 +34,21 @@ func (c *cataloger) ListCommits(ctx context.Context, repository, branch string, if err != nil { return nil, err } - lineage, err := getLineage(tx, branchID, CommittedID) + lineage, err := getLineage(tx, branchID, fromCommitID) if err != nil { return nil, fmt.Errorf("get lineage: %w", err) } - lineageAsValuesTable := getLineageAsValues(lineage, branchID) - query := `SELECT b_name.name as branch_name,c.commit_id,c.previous_commit_id,c.committer,c.message,c.creation_date,c.metadata, + lineageAsValuesTable := getLineageAsValues(lineage, branchID, fromCommitID) + cte := `WITH RECURSIVE lineage_graph AS ( + select branch_id,commit_id from ` + lineageAsValuesTable + ` + union all + select * from (Select distinct on (c.branch_id,c.merge_source_branch) merge_source_branch,merge_source_commit from catalog_commits c + join lineage_graph l on l.branch_id = c.branch_id and c.merge_type='from_child' and c.merge_source_commit < l.commit_id + order by c.branch_id,c.merge_source_branch,c.commit_id desc )t) +` + query := cte + `SELECT b_name.name as branch_name,c.commit_id,c.previous_commit_id,c.committer,c.message,c.creation_date,c.metadata, COALESCE(bb.name,'') as merge_source_branch_name,COALESCE(c.merge_source_commit,0) as merge_source_commit - FROM catalog_commits c JOIN (SELECT * FROM ` + lineageAsValuesTable + `) l ON c.branch_id = l.branch_id and c.commit_id <= l.commit_id + FROM catalog_commits c JOIN lineage_graph l ON c.branch_id = l.branch_id and c.commit_id <= l.commit_id JOIN catalog_branches b_name ON c.branch_id = b_name.id LEFT JOIN catalog_branches bb ON bb.id = c.merge_source_branch WHERE c.commit_id < $1 diff --git a/catalog/cataloger_list_commits_test.go b/catalog/cataloger_list_commits_test.go index b5e5a2dbf5c..e2c383c793c 100644 --- a/catalog/cataloger_list_commits_test.go +++ b/catalog/cataloger_list_commits_test.go @@ -45,9 +45,9 @@ func TestCataloger_ListCommits(t *testing.T) { limit: -1, }, want: []*CommitLog{ - {Reference: commits[2].Reference, Committer: "tester", Message: "commit3", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96a"}}, - {Reference: commits[1].Reference, Committer: "tester", Message: "commit2", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Z"}}, - {Reference: commits[0].Reference, Committer: "tester", Message: "commit1", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Y"}}, + {Reference: commits[2].Reference, Committer: "tester", Message: "commit3 on branch master", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96a"}}, + {Reference: commits[1].Reference, Committer: "tester", Message: "commit2 on branch master", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Z"}}, + {Reference: commits[0].Reference, Committer: "tester", Message: "commit1 on branch master", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Y"}}, {Reference: initialCommitReference, Committer: CatalogerCommitter, Message: createRepositoryCommitMessage, Metadata: Metadata{}}, }, wantMore: false, @@ -62,8 +62,8 @@ func TestCataloger_ListCommits(t *testing.T) { limit: 2, }, want: []*CommitLog{ - {Reference: commits[2].Reference, Committer: "tester", Message: "commit3", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96a"}}, - {Reference: commits[1].Reference, Committer: "tester", Message: "commit2", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Z"}}, + {Reference: commits[2].Reference, Committer: "tester", Message: "commit3 on branch master", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96a"}}, + {Reference: commits[1].Reference, Committer: "tester", Message: "commit2 on branch master", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Z"}}, }, wantMore: true, wantErr: false, @@ -91,7 +91,7 @@ func TestCataloger_ListCommits(t *testing.T) { limit: 1, }, want: []*CommitLog{ - {Reference: commits[1].Reference, Committer: "tester", Message: "commit2", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Z"}}, + {Reference: commits[1].Reference, Committer: "tester", Message: "commit2 on branch master", Metadata: Metadata{}, Parents: []string{"~KJ8Wd1Rs96Z"}}, }, wantMore: true, wantErr: false, @@ -204,7 +204,7 @@ func setupListCommitsByBranchData(t *testing.T, ctx context.Context, c Cataloger }, CreateEntryParams{}); err != nil { t.Fatal("Write entry for list repository commits failed", err) } - message := "commit" + strconv.Itoa(i+1) + message := "commit" + strconv.Itoa(i+1) + " on branch " + branch commitLog, err := c.Commit(ctx, repository, branch, message, "tester", nil) if err != nil { t.Fatalf("Commit for list repository commits failed '%s': %s", message, err) @@ -270,3 +270,140 @@ func TestCataloger_ListCommits_Lineage(t *testing.T) { t.Error("br_1 did not inherit commits correctly", diff) } } + +func TestCataloger_ListCommits_LineageFromChild(t *testing.T) { + ctx := context.Background() + c := testCataloger(t) + + repository := testCatalogerRepo(t, ctx, c, "repository", "master") + _ = setupListCommitsByBranchData(t, ctx, c, repository, "master") + + testCatalogerBranch(t, ctx, c, repository, "br_1_1", "master") + testCatalogerBranch(t, ctx, c, repository, "br_1_2", "br_1_1") + masterCommits, _, err := c.ListCommits(ctx, repository, "master", "", 100) + testutil.MustDo(t, "list master commits", err) + + br1Commits, _, err := c.ListCommits(ctx, repository, "br_1_1", "", 100) + testutil.MustDo(t, "list br_1_1 commits", err) + + // get all commits without the first one + if diff := deep.Equal(masterCommits, br1Commits[1:]); diff != nil { + t.Error("br_1_1 did not inherit commits correctly", diff) + } + + b2Commits, _, err := c.ListCommits(ctx, repository, "br_1_2", "", 100) + testutil.MustDo(t, "list br_1_2 commits", err) + + if diff := deep.Equal(br1Commits, b2Commits[1:]); diff != nil { + t.Error("br_1_2 did not inherit commits correctly", diff) + } + + if err := c.CreateEntry(ctx, repository, "master", Entry{ + Path: "master-file", + Checksum: "ssss", + PhysicalAddress: "xxxxxxx", + Size: 10000, + }, CreateEntryParams{}); err != nil { + t.Fatal("Write entry for list repository commits failed", err) + } + _, err = c.Commit(ctx, repository, "master", "commit master-file on master", "tester", nil) + if err != nil { + t.Fatalf("Commit for list repository commits failed '%s': %s", "master commit failed", err) + } + _, err = c.Merge(ctx, repository, "master", "br_1_1", "tester", "merge master to br_1_1", nil) + testutil.MustDo(t, "merge master into br_1_1", err) + + got, _, err := c.ListCommits(ctx, repository, "br_1_2", "", 100) + testutil.MustDo(t, "list br_1_2 commits", err) + if diff := deep.Equal(got, b2Commits); diff != nil { + t.Error("br_1_2 changed although not merged", diff) + } + masterCommits, _, err = c.ListCommits(ctx, repository, "master", "", 100) + testutil.MustDo(t, "list master commits", err) + + br11BaseList, _, err := c.ListCommits(ctx, repository, "br_1_1", "", 100) + testutil.MustDo(t, "list br_1_1 commits", err) + if diff := deep.Equal(masterCommits[0], br11BaseList[1]); diff != nil { + t.Error("br_1_1 did not inherit commits correctly", diff) + } + + testCatalogerBranch(t, ctx, c, repository, "br_2_1", "master") + testCatalogerBranch(t, ctx, c, repository, "br_2_2", "br_2_1") + if err := c.CreateEntry(ctx, repository, "br_2_2", Entry{ + Path: "master-file", + Checksum: "zzzzz", + PhysicalAddress: "yyyyy", + Size: 20000, + }, CreateEntryParams{}); err != nil { + t.Fatal("Write entry to br_2_2 failed", err) + } + _, err = c.Commit(ctx, repository, "br_2_2", "commit master-file to br_2_2", "tester", nil) + if err != nil { + t.Fatalf("Commit for list repository commits failed '%s': %s", "br_2_2 commit failed", err) + } + br22List, _, err := c.ListCommits(ctx, repository, "br_2_2", "", 100) + testutil.MustDo(t, "list br_2_2 commits", err) + _ = br22List + _, err = c.Merge(ctx, repository, "br_2_2", "br_2_1", "tester", "merge br_2_2 to br_2_1", nil) + testutil.MustDo(t, "merge br_2_2 into br_2_1", err) + br21List, _, err := c.ListCommits(ctx, repository, "br_2_1", "", 100) + testutil.MustDo(t, "list br_2_1 commits", err) + _ = br21List + masterList, _, err := c.ListCommits(ctx, repository, "master", "", 100) + testutil.MustDo(t, "list master commits", err) + if diff := deep.Equal(masterCommits, masterList); diff != nil { + t.Error("master commits changed before merge", diff) + } + merge2, err := c.Merge(ctx, repository, "br_2_1", "master", "tester", "merge br_2_1 to master", nil) + testutil.MustDo(t, "merge br_2_1 into master", err) + if merge2.Differences[0].Type != DifferenceTypeChanged || merge2.Differences[0].Path != "master-file" { + t.Error("merge br_2_1 into master with unexpected results", merge2.Differences[0]) + } + + masterList, _, err = c.ListCommits(ctx, repository, "master", "", 100) + testutil.MustDo(t, "list master commits", err) + if diff := deep.Equal(br21List, masterList[1:]); diff != nil { + t.Error("master commits list mismatch with br_2_1_list", diff) + } + + br11List, _, err := c.ListCommits(ctx, repository, "br_1_1", "", 100) + testutil.MustDo(t, "list br_1_1 commits", err) + if diff := deep.Equal(br11BaseList, br11List); diff != nil { + t.Error("br_1_1 commits changed before merge", diff) + } + _, err = c.Merge(ctx, repository, "master", "br_1_1", "tester", "merge master to br_1_1", nil) + testutil.MustDo(t, "merge master into br_1_1", err) + br11List, _, err = c.ListCommits(ctx, repository, "br_1_1", "", 100) + testutil.MustDo(t, "list br_1_1 commits", err) + if diff := deep.Equal(masterList[:5], br11List[1:6]); diff != nil { + t.Error("master 5 first different from br_1_1 [1:6]", diff) + } + if diff := deep.Equal(masterList[6:], br11List[9:]); diff != nil { + t.Error("master 5 first different from br_1_1 [1:6]", diff) + } + // test that a change to br_2_2 does not propagate to master + if err := c.CreateEntry(ctx, repository, "br_2_2", Entry{ + Path: "no-propagate-file", + Checksum: "aaaaaaaa", + PhysicalAddress: "yybbbbbbyyy", + Size: 20000, + }, CreateEntryParams{}); err != nil { + t.Fatal("Write no-propagate-file to br_2_2 failed", err) + } + _, err = c.Commit(ctx, repository, "br_2_2", "commit master-file to br_2_2", "tester", nil) + if err != nil { + t.Fatalf("no-propagate-Commit for list repository commits failed '%s': %s", "br_2_2 commit failed", err) + } + _, err = c.Merge(ctx, repository, "br_2_2", "br_2_1", "tester", "merge br_2_2 to br_2_1", nil) + testutil.MustDo(t, "second merge br_2_2 into br_2_1", err) + newBr21List, _, err := c.ListCommits(ctx, repository, "br_2_1", "", 100) + testutil.MustDo(t, "second list br_2_1 commits", err) + if diff := deep.Equal(br21List, newBr21List); diff == nil { + t.Error("br_2_1 commits did not changed after merge", diff) + } + newMasterList, _, err := c.ListCommits(ctx, repository, "master", "", 100) + testutil.MustDo(t, "third list master commits", err) + if diff := deep.Equal(newMasterList, masterList); diff != nil { + t.Error("master commits changed without merge", diff) + } +} diff --git a/catalog/db.go b/catalog/db.go index 9a2816808a5..d6e06de6853 100644 --- a/catalog/db.go +++ b/catalog/db.go @@ -142,9 +142,9 @@ func getLineage(tx db.Tx, branchID int64, commitID CommitID) ([]lineageCommit, e return requestedLineage, nil } -func getLineageAsValues(lineage []lineageCommit, branchID int64) string { +func getLineageAsValues(lineage []lineageCommit, branchID int64, commitID CommitID) string { valArray := make([]string, 1, len(lineage)+1) - valArray[0] = fmt.Sprintf("(0,%d,%d)", branchID, MaxCommitID) + valArray[0] = fmt.Sprintf("(0,%d::bigint,%d::bigint)", branchID, commitID) for precedence, lineageBranch := range lineage { valArray = append(valArray, fmt.Sprintf("(%d,%d,%d)", precedence+1, lineageBranch.BranchID, lineageBranch.CommitID)) } diff --git a/catalog/views.go b/catalog/views.go index b5cae728974..c362b7248bf 100644 --- a/catalog/views.go +++ b/catalog/views.go @@ -147,7 +147,7 @@ func sqDiffFromChildV(parentID, childID int64, parentEffectiveCommit, childEffec } func sqDiffFromParentV(parentID, childID int64, lastChildMergeWithParent CommitID, parentUncommittedLineage, childUncommittedLineage []lineageCommit) sq.SelectBuilder { - childLineageValues := getLineageAsValues(childUncommittedLineage, childID) + childLineageValues := getLineageAsValues(childUncommittedLineage, childID, MaxCommitID) childLineage := sqEntriesLineage(childID, UncommittedID, childUncommittedLineage) sqChild := sq.Select("*"). FromSelect(childLineage, "s").