Merge pull request #254 from MichaelMure/import-events

Bridge: correctly emit NothingEvents

6 files changed, 48 insertions, 68 deletions

diff --git a/bridge/github/export.go b/bridge/github/export.go
index e2a185b0..2fb92636 100644
--- a/bridge/github/export.go
+++ b/bridge/github/export.go
@@ -148,8 +148,6 @@ func (ge *githubExporter) ExportAll(ctx context.Context, repo *cache.RepoCache,
 				if snapshot.HasAnyActor(allIdentitiesIds...) {
 					// try to export the bug and it associated events
 					ge.exportBug(ctx, b, since, out)
-				} else {
-					out <- core.NewExportNothing(id, "not an actor")
 				}
 			}
 		}
@@ -161,6 +159,7 @@ func (ge *githubExporter) ExportAll(ctx context.Context, repo *cache.RepoCache,
 // exportBug publish bugs and related events
 func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, since time.Time, out chan<- core.ExportResult) {
 	snapshot := b.Snapshot()
+	var bugUpdated bool
 
 	var bugGithubID string
 	var bugGithubURL string
@@ -198,13 +197,12 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			return
 		}
 
-		// ignore issue comming from other repositories
+		// ignore issue coming from other repositories
 		if owner != ge.conf[keyOwner] && project != ge.conf[keyProject] {
 			out <- core.NewExportNothing(b.Id(), fmt.Sprintf("skipping issue from url:%s", githubURL))
 			return
 		}
 
-		out <- core.NewExportNothing(b.Id(), "bug already exported")
 		// will be used to mark operation related to a bug as exported
 		bugGithubID = githubID
 		bugGithubURL = githubURL
@@ -260,24 +258,20 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 		// cache the ID of already exported or imported issues and events from Github
 		if id, ok := op.GetMetadata(metaKeyGithubId); ok {
 			ge.cachedOperationIDs[op.Id()] = id
-			out <- core.NewExportNothing(op.Id(), "already exported operation")
 			continue
 		}
 
 		opAuthor := op.GetAuthor()
 		client, err := ge.getIdentityClient(opAuthor.Id())
 		if err != nil {
-			out <- core.NewExportNothing(op.Id(), "missing operation author token")
 			continue
 		}
 
 		var id, url string
-		switch op.(type) {
+		switch op := op.(type) {
 		case *bug.AddCommentOperation:
-			opr := op.(*bug.AddCommentOperation)
-
 			// send operation to github
-			id, url, err = addCommentGithubIssue(ctx, client, bugGithubID, opr.Message)
+			id, url, err = addCommentGithubIssue(ctx, client, bugGithubID, op.Message)
 			if err != nil {
 				err := errors.Wrap(err, "adding comment")
 				out <- core.NewExportError(err, b.Id())
@@ -290,14 +284,11 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			ge.cachedOperationIDs[op.Id()] = id
 
 		case *bug.EditCommentOperation:
-
-			opr := op.(*bug.EditCommentOperation)
-
 			// Since github doesn't consider the issue body as a comment
-			if opr.Target == createOp.Id() {
+			if op.Target == createOp.Id() {
 
 				// case bug creation operation: we need to edit the Github issue
-				if err := updateGithubIssueBody(ctx, client, bugGithubID, opr.Message); err != nil {
+				if err := updateGithubIssueBody(ctx, client, bugGithubID, op.Message); err != nil {
 					err := errors.Wrap(err, "editing issue")
 					out <- core.NewExportError(err, b.Id())
 					return
@@ -311,12 +302,12 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			} else {
 
 				// case comment edition operation: we need to edit the Github comment
-				commentID, ok := ge.cachedOperationIDs[opr.Target]
+				commentID, ok := ge.cachedOperationIDs[op.Target]
 				if !ok {
 					panic("unexpected error: comment id not found")
 				}
 
-				eid, eurl, err := editCommentGithubIssue(ctx, client, commentID, opr.Message)
+				eid, eurl, err := editCommentGithubIssue(ctx, client, commentID, op.Message)
 				if err != nil {
 					err := errors.Wrap(err, "editing comment")
 					out <- core.NewExportError(err, b.Id())
@@ -331,8 +322,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			}
 
 		case *bug.SetStatusOperation:
-			opr := op.(*bug.SetStatusOperation)
-			if err := updateGithubIssueStatus(ctx, client, bugGithubID, opr.Status); err != nil {
+			if err := updateGithubIssueStatus(ctx, client, bugGithubID, op.Status); err != nil {
 				err := errors.Wrap(err, "editing status")
 				out <- core.NewExportError(err, b.Id())
 				return
@@ -344,8 +334,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			url = bugGithubURL
 
 		case *bug.SetTitleOperation:
-			opr := op.(*bug.SetTitleOperation)
-			if err := updateGithubIssueTitle(ctx, client, bugGithubID, opr.Title); err != nil {
+			if err := updateGithubIssueTitle(ctx, client, bugGithubID, op.Title); err != nil {
 				err := errors.Wrap(err, "editing title")
 				out <- core.NewExportError(err, b.Id())
 				return
@@ -357,8 +346,7 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			url = bugGithubURL
 
 		case *bug.LabelChangeOperation:
-			opr := op.(*bug.LabelChangeOperation)
-			if err := ge.updateGithubIssueLabels(ctx, client, bugGithubID, opr.Added, opr.Removed); err != nil {
+			if err := ge.updateGithubIssueLabels(ctx, client, bugGithubID, op.Added, op.Removed); err != nil {
 				err := errors.Wrap(err, "updating labels")
 				out <- core.NewExportError(err, b.Id())
 				return
@@ -386,6 +374,12 @@ func (ge *githubExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			out <- core.NewExportError(err, b.Id())
 			return
 		}
+
+		bugUpdated = true
+	}
+
+	if !bugUpdated {
+		out <- core.NewExportNothing(b.Id(), "nothing has been exported")
 	}
 }
 
diff --git a/bridge/github/import.go b/bridge/github/import.go
index d3d56cbc..86444057 100644
--- a/bridge/github/import.go
+++ b/bridge/github/import.go
@@ -60,15 +60,18 @@ func (gi *githubImporter) ImportAll(ctx context.Context, repo *cache.RepoCache,
 			// loop over timeline items
 			for gi.iterator.NextTimelineItem() {
 				item := gi.iterator.TimelineItemValue()
-				if err := gi.ensureTimelineItem(repo, b, item); err != nil {
-					err := fmt.Errorf("timeline item creation: %v", err)
+				err := gi.ensureTimelineItem(repo, b, item)
+				if err != nil {
+					err = fmt.Errorf("timeline item creation: %v", err)
 					out <- core.NewImportError(err, "")
 					return
 				}
 			}
 
-			// commit bug state
-			if err := b.CommitAsNeeded(); err != nil {
+			if !b.NeedCommit() {
+				out <- core.NewImportNothing(b.Id(), "no imported operation")
+			} else if err := b.Commit(); err != nil {
+				// commit bug state
 				err = fmt.Errorf("bug commit: %v", err)
 				out <- core.NewImportError(err, "")
 				return
@@ -128,16 +131,12 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline
 
 			// importing a new bug
 			gi.out <- core.NewImportBug(b.Id())
-		} else {
-			gi.out <- core.NewImportNothing("", "bug already imported")
 		}
-
 	} else {
 		// create bug from given issueEdits
 		for i, edit := range issueEdits {
 			if i == 0 && b != nil {
 				// The first edit in the github result is the issue creation itself, we already have that
-				gi.out <- core.NewImportNothing("", "bug already imported")
 				continue
 			}
 
@@ -165,7 +164,6 @@ func (gi *githubImporter) ensureIssue(repo *cache.RepoCache, issue issueTimeline
 				if err != nil {
 					return nil, err
 				}
-
 				// importing a new bug
 				gi.out <- core.NewImportBug(b.Id())
 				continue
@@ -202,13 +200,12 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 		if err != nil {
 			return fmt.Errorf("timeline comment creation: %v", err)
 		}
+		return nil
 
 	case "LabeledEvent":
 		id := parseId(item.LabeledEvent.Id)
 		_, err := b.ResolveOperationWithMetadata(metaKeyGithubId, id)
 		if err == nil {
-			reason := fmt.Sprintf("operation already imported: %v", item.Typename)
-			gi.out <- core.NewImportNothing("", reason)
 			return nil
 		}
 
@@ -239,8 +236,6 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 		id := parseId(item.UnlabeledEvent.Id)
 		_, err := b.ResolveOperationWithMetadata(metaKeyGithubId, id)
 		if err == nil {
-			reason := fmt.Sprintf("operation already imported: %v", item.Typename)
-			gi.out <- core.NewImportNothing("", reason)
 			return nil
 		}
 		if err != cache.ErrNoMatchingOp {
@@ -274,8 +269,6 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 			return err
 		}
 		if err == nil {
-			reason := fmt.Sprintf("operation already imported: %v", item.Typename)
-			gi.out <- core.NewImportNothing("", reason)
 			return nil
 		}
 		author, err := gi.ensurePerson(repo, item.ClosedEvent.Actor)
@@ -302,8 +295,6 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 			return err
 		}
 		if err == nil {
-			reason := fmt.Sprintf("operation already imported: %v", item.Typename)
-			gi.out <- core.NewImportNothing("", reason)
 			return nil
 		}
 		author, err := gi.ensurePerson(repo, item.ReopenedEvent.Actor)
@@ -330,8 +321,6 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 			return err
 		}
 		if err == nil {
-			reason := fmt.Sprintf("operation already imported: %v", item.Typename)
-			gi.out <- core.NewImportNothing("", reason)
 			return nil
 		}
 		author, err := gi.ensurePerson(repo, item.RenamedTitleEvent.Actor)
@@ -350,10 +339,6 @@ func (gi *githubImporter) ensureTimelineItem(repo *cache.RepoCache, b *cache.Bug
 
 		gi.out <- core.NewImportTitleEdition(op.Id())
 		return nil
-
-	default:
-		reason := fmt.Sprintf("ignoring timeline type: %v", item.Typename)
-		gi.out <- core.NewImportNothing("", reason)
 	}
 
 	return nil
@@ -367,9 +352,7 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.
 	}
 
 	targetOpID, err := b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(item.Id))
-	if err == nil {
-		gi.out <- core.NewImportNothing("", "comment already imported")
-	} else if err != cache.ErrNoMatchingOp {
+	if err != nil && err != cache.ErrNoMatchingOp {
 		// real error
 		return err
 	}
@@ -398,13 +381,13 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.
 			}
 
 			gi.out <- core.NewImportComment(op.Id())
+			return nil
 		}
 
 	} else {
 		for i, edit := range edits {
 			if i == 0 && targetOpID != "" {
 				// The first edit in the github result is the comment creation itself, we already have that
-				gi.out <- core.NewImportNothing("", "comment already imported")
 				continue
 			}
 
@@ -434,6 +417,7 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.
 				if err != nil {
 					return err
 				}
+				gi.out <- core.NewImportComment(op.Id())
 
 				// set target for the nexr edit now that the comment is created
 				targetOpID = op.Id()
@@ -452,7 +436,6 @@ func (gi *githubImporter) ensureTimelineComment(repo *cache.RepoCache, b *cache.
 func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugCache, target entity.Id, edit userContentEdit) error {
 	_, err := b.ResolveOperationWithMetadata(metaKeyGithubId, parseId(edit.Id))
 	if err == nil {
-		gi.out <- core.NewImportNothing(b.Id(), "edition already imported")
 		return nil
 	}
 	if err != cache.ErrNoMatchingOp {
@@ -468,7 +451,7 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC
 	switch {
 	case edit.DeletedAt != nil:
 		// comment deletion, not supported yet
-		gi.out <- core.NewImportNothing(b.Id(), "comment deletion is not supported yet")
+		return nil
 
 	case edit.DeletedAt == nil:
 
@@ -493,8 +476,8 @@ func (gi *githubImporter) ensureCommentEdit(repo *cache.RepoCache, b *cache.BugC
 		}
 
 		gi.out <- core.NewImportCommentEdition(op.Id())
+		return nil
 	}
-
 	return nil
 }
 
diff --git a/bridge/gitlab/export.go b/bridge/gitlab/export.go
index 44778b44..7c00e39d 100644
--- a/bridge/gitlab/export.go
+++ b/bridge/gitlab/export.go
@@ -117,8 +117,6 @@ func (ge *gitlabExporter) ExportAll(ctx context.Context, repo *cache.RepoCache,
 				if snapshot.HasAnyActor(allIdentitiesIds...) {
 					// try to export the bug and it associated events
 					ge.exportBug(ctx, b, since, out)
-				} else {
-					out <- core.NewExportNothing(id, "not an actor")
 				}
 			}
 		}
@@ -131,6 +129,7 @@ func (ge *gitlabExporter) ExportAll(ctx context.Context, repo *cache.RepoCache,
 func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, since time.Time, out chan<- core.ExportResult) {
 	snapshot := b.Snapshot()
 
+	var bugUpdated bool
 	var err error
 	var bugGitlabID int
 	var bugGitlabIDString string
@@ -166,8 +165,6 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			return
 		}
 
-		out <- core.NewExportNothing(b.Id(), "bug already exported")
-
 		// will be used to mark operation related to a bug as exported
 		bugGitlabIDString = gitlabID
 		bugGitlabID, err = strconv.Atoi(bugGitlabIDString)
@@ -237,14 +234,12 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 		// cache the ID of already exported or imported issues and events from Gitlab
 		if id, ok := op.GetMetadata(metaKeyGitlabId); ok {
 			ge.cachedOperationIDs[op.Id().String()] = id
-			out <- core.NewExportNothing(op.Id(), "already exported operation")
 			continue
 		}
 
 		opAuthor := op.GetAuthor()
 		client, err := ge.getIdentityClient(opAuthor.Id())
 		if err != nil {
-			out <- core.NewExportNothing(op.Id(), "missing operation author token")
 			continue
 		}
 
@@ -371,6 +366,12 @@ func (ge *gitlabExporter) exportBug(ctx context.Context, b *cache.BugCache, sinc
 			out <- core.NewExportError(err, b.Id())
 			return
 		}
+
+		bugUpdated = true
+	}
+
+	if !bugUpdated {
+		out <- core.NewExportNothing(b.Id(), "nothing has been exported")
 	}
 }
 
diff --git a/bridge/gitlab/import.go b/bridge/gitlab/import.go
index e92067af..92e9952e 100644
--- a/bridge/gitlab/import.go
+++ b/bridge/gitlab/import.go
@@ -73,8 +73,10 @@ func (gi *gitlabImporter) ImportAll(ctx context.Context, repo *cache.RepoCache,
 				}
 			}
 
-			// commit bug state
-			if err := b.CommitAsNeeded(); err != nil {
+			if !b.NeedCommit() {
+				out <- core.NewImportNothing(b.Id(), "no imported operation")
+			} else if err := b.Commit(); err != nil {
+				// commit bug state
 				err := fmt.Errorf("bug commit: %v", err)
 				out <- core.NewImportError(err, "")
 				return
@@ -99,7 +101,6 @@ func (gi *gitlabImporter) ensureIssue(repo *cache.RepoCache, issue *gitlab.Issue
 	// resolve bug
 	b, err := repo.ResolveBugCreateMetadata(metaKeyGitlabUrl, issue.WebURL)
 	if err == nil {
-		gi.out <- core.NewImportNothing("", "bug already imported")
 		return b, nil
 	}
 	if err != bug.ErrBugNotExist {
@@ -299,8 +300,6 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n
 		NOTE_MENTIONED_IN_ISSUE,
 		NOTE_MENTIONED_IN_MERGE_REQUEST:
 
-		reason := fmt.Sprintf("unsupported note type: %s", noteType.String())
-		gi.out <- core.NewImportNothing("", reason)
 		return nil
 
 	default:
diff --git a/bridge/launchpad/import.go b/bridge/launchpad/import.go
index 178945b6..59fc5c5f 100644
--- a/bridge/launchpad/import.go
+++ b/bridge/launchpad/import.go
@@ -103,8 +103,6 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach
 
 				/* Handle messages */
 				if len(lpBug.Messages) == 0 {
-					err := fmt.Sprintf("bug doesn't have any comments")
-					out <- core.NewImportNothing(entity.Id(lpBugID), err)
 					return
 				}
 
@@ -149,8 +147,9 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach
 					out <- core.NewImportComment(op.Id())
 				}
 
-				err = b.CommitAsNeeded()
-				if err != nil {
+				if !b.NeedCommit() {
+					out <- core.NewImportNothing(b.Id(), "no imported operation")
+				} else if err := b.Commit(); err != nil {
 					out <- core.NewImportError(err, "")
 					return
 				}
diff --git a/cache/bug_cache.go b/cache/bug_cache.go
index 6a220f49..6026190f 100644
--- a/cache/bug_cache.go
+++ b/cache/bug_cache.go
@@ -265,3 +265,7 @@ func (c *BugCache) CommitAsNeeded() error {
 	}
 	return c.notifyUpdated()
 }
+
+func (c *BugCache) NeedCommit() bool {
+	return c.bug.NeedCommit()
+}