github: fix 3 edge-case failures

2 files changed, 61 insertions, 44 deletions

diff --git a/bridge/github/import.go b/bridge/github/import.go
index 38278911..4627145e 100644
--- a/bridge/github/import.go
+++ b/bridge/github/import.go
@@ -108,13 +108,13 @@ func (gi *githubImporter) Import(repo *cache.RepoCache, id string) error {
 func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline, rootVariables map[string]interface{}) (*cache.BugCache, error) {
 	fmt.Printf("import issue: %s\n", issue.Title)
 
-	b, err := repo.ResolveBugCreateMetadata(keyGithubId, parseId(issue.Id))
-	if err != nil && err != bug.ErrBugNotExist {
+	author, err := gi.ensurePerson(repo, issue.Author)
+	if err != nil {
 		return nil, err
 	}
 
-	author, err := gi.makePerson(repo, issue.Author)
-	if err != nil {
+	b, err := repo.ResolveBugCreateMetadata(keyGithubId, parseId(issue.Id))
+	if err != nil && err != bug.ErrBugNotExist {
 		return nil, err
 	}
 
@@ -187,7 +187,7 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline
 			continue
 		}
 
-		target, err := b.ResolveTargetWithMetadata(keyGithubId, parseId(issue.Id))
+		target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(issue.Id))
 		if err != nil {
 			return nil, err
 		}
@@ -269,7 +269,7 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline
 				continue
 			}
 
-			target, err := b.ResolveTargetWithMetadata(keyGithubId, parseId(issue.Id))
+			target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(issue.Id))
 			if err != nil {
 				return nil, err
 			}
@@ -318,15 +318,15 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 
 	case "LabeledEvent":
 		id := parseId(item.LabeledEvent.Id)
-		_, err := b.ResolveTargetWithMetadata(keyGithubId, id)
+		_, err := b.ResolveOperationWithMetadata(keyGithubId, id)
 		if err != cache.ErrNoMatchingOp {
 			return err
 		}
-		author, err := gi.makePerson(repo, item.LabeledEvent.Actor)
+		author, err := gi.ensurePerson(repo, item.LabeledEvent.Actor)
 		if err != nil {
 			return err
 		}
-		_, err = b.ChangeLabelsRaw(
+		_, _, err = b.ChangeLabelsRaw(
 			author,
 			item.LabeledEvent.CreatedAt.Unix(),
 			[]string{
@@ -339,15 +339,15 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 
 	case "UnlabeledEvent":
 		id := parseId(item.UnlabeledEvent.Id)
-		_, err := b.ResolveTargetWithMetadata(keyGithubId, id)
+		_, err := b.ResolveOperationWithMetadata(keyGithubId, id)
 		if err != cache.ErrNoMatchingOp {
 			return err
 		}
-		author, err := gi.makePerson(repo, item.UnlabeledEvent.Actor)
+		author, err := gi.ensurePerson(repo, item.UnlabeledEvent.Actor)
 		if err != nil {
 			return err
 		}
-		_, err = b.ChangeLabelsRaw(
+		_, _, err = b.ChangeLabelsRaw(
 			author,
 			item.UnlabeledEvent.CreatedAt.Unix(),
 			nil,
@@ -360,52 +360,55 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 
 	case "ClosedEvent":
 		id := parseId(item.ClosedEvent.Id)
-		_, err := b.ResolveTargetWithMetadata(keyGithubId, id)
+		_, err := b.ResolveOperationWithMetadata(keyGithubId, id)
 		if err != cache.ErrNoMatchingOp {
 			return err
 		}
-		author, err := gi.makePerson(repo, item.ClosedEvent.Actor)
+		author, err := gi.ensurePerson(repo, item.ClosedEvent.Actor)
 		if err != nil {
 			return err
 		}
-		return b.CloseRaw(
+		_, err = b.CloseRaw(
 			author,
 			item.ClosedEvent.CreatedAt.Unix(),
 			map[string]string{keyGithubId: id},
 		)
+		return err
 
 	case "ReopenedEvent":
 		id := parseId(item.ReopenedEvent.Id)
-		_, err := b.ResolveTargetWithMetadata(keyGithubId, id)
+		_, err := b.ResolveOperationWithMetadata(keyGithubId, id)
 		if err != cache.ErrNoMatchingOp {
 			return err
 		}
-		author, err := gi.makePerson(repo, item.ReopenedEvent.Actor)
+		author, err := gi.ensurePerson(repo, item.ReopenedEvent.Actor)
 		if err != nil {
 			return err
 		}
-		return b.OpenRaw(
+		_, err = b.OpenRaw(
 			author,
 			item.ReopenedEvent.CreatedAt.Unix(),
 			map[string]string{keyGithubId: id},
 		)
+		return err
 
 	case "RenamedTitleEvent":
 		id := parseId(item.RenamedTitleEvent.Id)
-		_, err := b.ResolveTargetWithMetadata(keyGithubId, id)
+		_, err := b.ResolveOperationWithMetadata(keyGithubId, id)
 		if err != cache.ErrNoMatchingOp {
 			return err
 		}
-		author, err := gi.makePerson(repo, item.RenamedTitleEvent.Actor)
+		author, err := gi.ensurePerson(repo, item.RenamedTitleEvent.Actor)
 		if err != nil {
 			return err
 		}
-		return b.SetTitleRaw(
+		_, err = b.SetTitleRaw(
 			author,
 			item.RenamedTitleEvent.CreatedAt.Unix(),
 			string(item.RenamedTitleEvent.CurrentTitle),
 			map[string]string{keyGithubId: id},
 		)
+		return err
 
 	default:
 		fmt.Println("ignore event ", item.Typename)
@@ -415,14 +418,14 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 }
 
 func (gi *githubImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, cursor githubv4.String, comment issueComment, rootVariables map[string]interface{}) error {
-	target, err := b.ResolveTargetWithMetadata(keyGithubId, parseId(comment.Id))
-	if err != nil && err != cache.ErrNoMatchingOp {
-		// real error
+	author, err := gi.ensurePerson(repo, comment.Author)
+	if err != nil {
 		return err
 	}
 
-	author, err := gi.makePerson(repo, comment.Author)
-	if err != nil {
+	target, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(comment.Id))
+	if err != nil && err != cache.ErrNoMatchingOp {
+		// real error
 		return err
 	}
 
@@ -439,7 +442,7 @@ func (gi *githubImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache
 
 	if len(comment.UserContentEdits.Nodes) == 0 {
 		if err == cache.ErrNoMatchingOp {
-			err = b.AddCommentRaw(
+			op, err := b.AddCommentRaw(
 				author,
 				comment.CreatedAt.Unix(),
 				cleanupText(string(comment.Body)),
@@ -448,7 +451,11 @@ func (gi *githubImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache
 					keyGithubId: parseId(comment.Id),
 				},
 			)
+			if err != nil {
+				return err
+			}
 
+			target, err = op.Hash()
 			if err != nil {
 				return err
 			}
@@ -472,7 +479,7 @@ func (gi *githubImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache
 				continue
 			}
 
-			err = b.AddCommentRaw(
+			op, err := b.AddCommentRaw(
 				author,
 				comment.CreatedAt.Unix(),
 				cleanupText(string(*edit.Diff)),
@@ -485,6 +492,11 @@ func (gi *githubImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache
 			if err != nil {
 				return err
 			}
+
+			target, err = op.Hash()
+			if err != nil {
+				return err
+			}
 		}
 
 		err := gi.ensureCommentEdit(repo, b, target, edit)
@@ -554,11 +566,7 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC
 		return nil
 	}
 
-	if edit.Editor == nil {
-		return fmt.Errorf("no editor")
-	}
-
-	_, err := b.ResolveTargetWithMetadata(keyGithubId, parseId(edit.Id))
+	_, err := b.ResolveOperationWithMetadata(keyGithubId, parseId(edit.Id))
 	if err == nil {
 		// already imported
 		return nil
@@ -570,9 +578,18 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC
 
 	fmt.Println("import edition")
 
-	editor, err := gi.makePerson(repo, edit.Editor)
-	if err != nil {
-		return err
+	var editor *cache.IdentityCache
+	if edit.Editor == nil {
+		// user account has been deleted, replacing it with the ghost
+		editor, err = gi.getGhost(repo)
+		if err != nil {
+			return err
+		}
+	} else {
+		editor, err = gi.ensurePerson(repo, edit.Editor)
+		if err != nil {
+			return err
+		}
 	}
 
 	switch {
@@ -581,7 +598,7 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC
 
 	case edit.DeletedAt == nil:
 		// comment edition
-		err := b.EditCommentRaw(
+		_, err := b.EditCommentRaw(
 			editor,
 			edit.CreatedAt.Unix(),
 			target,
@@ -598,8 +615,8 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC
 	return nil
 }
 
-// makePerson create a bug.Person from the Github data
-func (gi *githubImporter) makePerson(repo *cache.RepoCache, actor *actor) (*cache.IdentityCache, error) {
+// ensurePerson create a bug.Person from the Github data
+func (gi *githubImporter) ensurePerson(repo *cache.RepoCache, actor *actor) (*cache.IdentityCache, error) {
 	// When a user has been deleted, Github return a null actor, while displaying a profile named "ghost"
 	// in it's UI. So we need a special case to get it.
 	if actor == nil {
diff --git a/bridge/launchpad/import.go b/bridge/launchpad/import.go
index b70d34f0..30ec5c3f 100644
--- a/bridge/launchpad/import.go
+++ b/bridge/launchpad/import.go
@@ -23,7 +23,7 @@ func (li *launchpadImporter) Init(conf core.Configuration) error {
 const keyLaunchpadID = "launchpad-id"
 const keyLaunchpadLogin = "launchpad-login"
 
-func (li *launchpadImporter) makePerson(repo *cache.RepoCache, owner LPPerson) (*cache.IdentityCache, error) {
+func (li *launchpadImporter) ensurePerson(repo *cache.RepoCache, owner LPPerson) (*cache.IdentityCache, error) {
 	// Look first in the cache
 	i, err := repo.ResolveIdentityImmutableMetadata(keyLaunchpadLogin, owner.Login)
 	if err == nil {
@@ -67,7 +67,7 @@ func (li *launchpadImporter) ImportAll(repo *cache.RepoCache) error {
 			return err
 		}
 
-		owner, err := li.makePerson(repo, lpBug.Owner)
+		owner, err := li.ensurePerson(repo, lpBug.Owner)
 		if err != nil {
 			return err
 		}
@@ -100,7 +100,7 @@ func (li *launchpadImporter) ImportAll(repo *cache.RepoCache) error {
 		// The Launchpad API returns the bug description as the first
 		// comment, so skip it.
 		for _, lpMessage := range lpBug.Messages[1:] {
-			_, err := b.ResolveTargetWithMetadata(keyLaunchpadID, lpMessage.ID)
+			_, err := b.ResolveOperationWithMetadata(keyLaunchpadID, lpMessage.ID)
 			if err != nil && err != cache.ErrNoMatchingOp {
 				return errors.Wrapf(err, "failed to fetch comments for bug #%s", lpBugID)
 			}
@@ -113,14 +113,14 @@ func (li *launchpadImporter) ImportAll(repo *cache.RepoCache) error {
 				continue
 			}
 
-			owner, err := li.makePerson(repo, lpMessage.Owner)
+			owner, err := li.ensurePerson(repo, lpMessage.Owner)
 			if err != nil {
 				return err
 			}
 
 			// This is a new comment, we can add it.
 			createdAt, _ := time.Parse(time.RFC3339, lpMessage.CreatedAt)
-			err = b.AddCommentRaw(
+			_, err = b.AddCommentRaw(
 				owner,
 				createdAt.Unix(),
 				lpMessage.Content,