opendev/gerrit
Code Issues Proposed changes

TestAccount: rename getEmailAddress() to getNameEmail()

Browse Source 
The getEmailAddress() returns "name <login@host>" while an email
address is often interpreted as "login@host"

Change-Id: Icf4d825d1618dea547f546d83af724bfe210da9d
This commit is contained in:
Han-Wen Nienhuys
2020-02-25 17:22:45 +01:00
parent 813f3aa62a
commit 419eaec4cf
14 changed files with 47 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -1282,7 +1282,7 @@ public abstract class AbstractDaemonTest {
}
protected void assertNotifyCc(TestAccount expected) {
assertNotifyCc(expected.getEmailAddress());
assertNotifyCc(expected.getNameEmail());
}
protected void assertNotifyCc(String expectedEmail, String expectedFullname) {
@@ -1302,7 +1302,7 @@ public abstract class AbstractDaemonTest {
protected void assertNotifyBcc(TestAccount expected) {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(expected.getEmailAddress());
assertThat(m.rcpt()).containsExactly(expected.getNameEmail());
assertThat(m.headers().get("To").isEmpty()).isTrue();
assertThat(m.headers().get("Cc").isEmpty()).isTrue();
}

2
java/com/google/gerrit/acceptance/TestAccount.java
View File

@@ -79,7 +79,7 @@ public abstract class TestAccount {
.toString();
}
public Address getEmailAddress() {
public Address getNameEmail() {
// Address is weird enough that it's safer and clearer to create a new instance in a
// non-abstract method rather than, say, having an abstract emailAddress() as part of this
// AutoValue class. Specifically:

1
java/com/google/gerrit/mail/Address.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.mail;
import com.google.gerrit.common.Nullable;
/** Represents an address (name + email) in an email message. */
public class Address {
public static Address parse(String in) {
final int lt = in.indexOf('<');

8
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -857,7 +857,7 @@ public class AccountIT extends AbstractDaemonTest {
gApi.changes().id(r.getChangeId()).abandon();
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt()).containsExactly(user2.getEmailAddress());
assertThat(messages.get(0).rcpt()).containsExactly(user2.getNameEmail());
accountIndexedCounter.assertNoReindex();
}
}
@@ -883,7 +883,7 @@ public class AccountIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message message = messages.get(0);
assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertMailReplyTo(message, admin.email());
accountIndexedCounter.assertNoReindex();
}
@@ -1840,7 +1840,7 @@ public class AccountIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
Message message = sender.getMessages().get(0);
assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("new SSH keys have been added");
// Delete key
@@ -1852,7 +1852,7 @@ public class AccountIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
message = sender.getMessages().get(0);
assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
assertThat(message.rcpt()).containsExactly(user.getNameEmail());
assertThat(message.body()).contains("SSH keys have been deleted");
}
}

12
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -1518,7 +1518,7 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.from().getName()).isEqualTo("Administrator (Code Review)");
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("I'd like you to do a code review");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertMailReplyTo(m, admin.email());
@@ -1587,7 +1587,7 @@ public class ChangeIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -1771,7 +1771,7 @@ public class ChangeIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
@@ -2162,7 +2162,7 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test
@@ -2411,7 +2411,7 @@ public class ChangeIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message msg = messages.get(0);
assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");
@@ -4527,7 +4527,7 @@ public class ChangeIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt()).containsExactly(user.getEmailAddress());
assertThat(messages.get(0).rcpt()).containsExactly(user.getNameEmail());
}
@Test

10
javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@@ -545,7 +545,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
r.assertOkStatus();
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.ALL);
@@ -553,7 +553,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
m = sender.getMessages().get(0);
assertThat(m.rcpt())
.containsExactly(user.getEmailAddress(), user2.getEmailAddress(), user3.getEmailAddress());
.containsExactly(user.getNameEmail(), user2.getNameEmail(), user3.getNameEmail());
sender.clear();
r = pushTo(pushSpec + ",notify=" + NotifyHandling.NONE + ",notify-to=" + user3.email());
@@ -1116,7 +1116,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
.containsExactly(user.getEmailAddress(), user2.getEmailAddress());
.containsExactly(user.getNameEmail(), user2.getNameEmail());
}
@Test
@@ -1141,7 +1141,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt())
.containsExactly(user.getEmailAddress(), user2.getEmailAddress());
.containsExactly(user.getNameEmail(), user2.getNameEmail());
}
/**
@@ -1838,7 +1838,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
@Test
public void pushWithEmailInFooter() throws Exception {
pushWithReviewerInFooter(user.getEmailAddress().toString(), user);
pushWithReviewerInFooter(user.getNameEmail().toString(), user);
}
@Test

2
javatests/com/google/gerrit/acceptance/rest/change/AssigneeIT.java
View File

@@ -61,7 +61,7 @@ public class AssigneeIT extends AbstractDaemonTest {
assertThat(sender.getMessages()).hasSize(1);
Message m = sender.getMessages().get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
}
@Test

2
javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
View File

@@ -209,7 +209,7 @@ public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
assertThat(messages.get(0).rcpt())
.containsExactly(Address.parse(addInput.reviewer), user.getEmailAddress());
.containsExactly(Address.parse(addInput.reviewer), user.getNameEmail());
sender.clear();
}
}

18
javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
View File

@@ -150,7 +150,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
}
@@ -188,7 +188,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
Message m = messages.get(0);
List<Address> expectedAddresses = new ArrayList<>(firstUsers.size());
for (TestAccount u : firstUsers) {
expectedAddresses.add(u.getEmailAddress());
expectedAddresses.add(u.getNameEmail());
}
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
@@ -216,9 +216,9 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
m = messages.get(0);
expectedAddresses = new ArrayList<>(4);
for (int i = 0; i < 3; i++) {
expectedAddresses.add(users.get(users.size() - i - 1).getEmailAddress());
expectedAddresses.add(users.get(users.size() - i - 1).getNameEmail());
}
expectedAddresses.add(reviewer.getEmailAddress());
expectedAddresses.add(reviewer.getNameEmail());
assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
}
@@ -398,13 +398,13 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
assertThat(messages).hasSize(2);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has posted comments on this change.");
assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
assertThat(m.body()).contains("Patch Set 1: Code-Review+2");
m = messages.get(1);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress(), observer.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail(), observer.getNameEmail());
assertThat(m.body()).contains("Hello " + user.fullName() + ",\n");
assertThat(m.body()).contains("I'd like you to do a code review.");
}
@@ -635,7 +635,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
sender.clear();
gApi.changes().id(r.getChangeId()).current().review(reviewInput);
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -652,7 +652,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
sender.clear();
gApi.changes().id(r.getChangeId()).addReviewer(addReviewer);
assertThat(sender.getMessages()).hasSize(1);
assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getEmailAddress());
assertThat(sender.getMessages().get(0).rcpt()).containsExactly(userToNotify.getNameEmail());
}
@Test
@@ -829,7 +829,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
}
private void assertThatUserIsOnlyReviewer(String changeId) throws Exception {
AccountInfo userInfo = new AccountInfo(user.fullName(), user.getEmailAddress().getEmail());
AccountInfo userInfo = new AccountInfo(user.fullName(), user.getNameEmail().getEmail());
userInfo._accountId = user.id().get();
userInfo.username = user.username();
assertThat(gApi.changes().id(changeId).get().reviewers)

6
javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

@@ -216,7 +216,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains(admin.fullName() + " has uploaded this change for review.");
// check that watcher is not notified if notify=NONE
@@ -303,7 +303,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
assertThat(author).email().isEqualTo(input.author.email);
assertThat(author).name().isEqualTo(input.author.name);
GitPerson committer = rApi.commit(false).committer;
assertThat(committer).email().isEqualTo(admin.getEmailAddress().getEmail());
assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
}
@Test
@@ -463,7 +463,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
GitPerson author = rApi.commit(false).author;
assertThat(author).email().isEqualTo(in.author.email);
GitPerson committer = rApi.commit(false).committer;
assertThat(committer).email().isEqualTo(admin.getEmailAddress().getEmail());
assertThat(committer).email().isEqualTo(admin.getNameEmail().getEmail());
}
@Test

2
javatests/com/google/gerrit/acceptance/rest/change/DeleteVoteIT.java
View File

@@ -73,7 +73,7 @@ public class DeleteVoteIT extends AbstractDaemonTest {
List<FakeEmailSender.Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
FakeEmailSender.Message msg = messages.get(0);
assertThat(msg.rcpt()).containsExactly(user.getEmailAddress());
assertThat(msg.rcpt()).containsExactly(user.getNameEmail());
assertThat(msg.body()).contains(admin.fullName() + " has removed a vote from this change.");
assertThat(msg.body())
.contains("Removed Code-Review+1 by " + user.fullName() + " <" + user.email() + ">\n");

4
javatests/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
View File

@@ -38,8 +38,8 @@ public class AbstractMailIT extends AbstractDaemonTest {
MailMessage.Builder messageBuilderWithDefaultFields() {
MailMessage.Builder b = MailMessage.builder();
b.id("some id");
b.from(user.getEmailAddress());
b.addTo(user.getEmailAddress()); // Not evaluated
b.from(user.getNameEmail());
b.addTo(user.getNameEmail()); // Not evaluated
b.subject("");
b.dateReceived(Instant.now());
return b;

4
javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
View File

@@ -237,7 +237,7 @@ public class MailProcessorIT extends AbstractMailIT {
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
.from(user.getEmailAddress())
.from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
@@ -259,7 +259,7 @@ public class MailProcessorIT extends AbstractMailIT {
newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
.from(user.getEmailAddress())
.from(user.getNameEmail())
.textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();

18
javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
View File

@@ -241,7 +241,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -273,7 +273,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -295,7 +295,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -322,7 +322,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -357,7 +357,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -385,7 +385,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -407,7 +407,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
messages = sender.getMessages();
assertThat(messages).hasSize(1);
m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user2.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user2.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER_USER2\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}
@@ -434,7 +434,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(user.getEmailAddress());
assertThat(m.rcpt()).containsExactly(user.getNameEmail());
assertThat(m.body()).contains("Change subject: Document multiprimary setup\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
sender.clear();
@@ -547,7 +547,7 @@ public class ProjectWatchIT extends AbstractDaemonTest {
List<Message> messages = sender.getMessages();
assertThat(messages).hasSize(1);
Message m = messages.get(0);
assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getEmailAddress());
assertThat(m.rcpt()).containsExactly(userThatCanViewPrivateChanges.getNameEmail());
assertThat(m.body()).contains("Change subject: TRIGGER\n");
assertThat(m.body()).contains("Gerrit-PatchSet: 1\n");
}