Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: removal of Credential argument in newApi() methods #3713

Merged
merged 7 commits into from
Mar 17, 2023
Merged

chore: removal of Credential argument in newApi() methods #3713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9991811
feat: initial removal of Credential argument in newApi() methods
diegomarquezp Feb 22, 2023
bafad26
chore: move account resolution to new class
diegomarquezp Feb 22, 2023
9ad0f16
chore: correct precond check, correct exception logging
diegomarquezp Feb 23, 2023
f874744
chore: do not throw exception when checking for credentials
diegomarquezp Mar 16, 2023
9fca091
fix: compilation errors
diegomarquezp Mar 16, 2023
29d0eee
fix: compilation errors in tests
diegomarquezp Mar 16, 2023
9a70835
Merge branch 'gcloud-cli-creds' into new-api-factory-creds
diegomarquezp Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions ...i.test/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/GcpProjectQueryJobTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ public void setUp() throws ProjectRepositoryException {
assertNotNull(Display.getCurrent());
when(projectSelector.getDisplay()).thenReturn(Display.getCurrent());

queryJob = new GcpProjectQueryJob(credential, projectRepository, projectSelector,
queryJob = new GcpProjectQueryJob(projectRepository, projectSelector,
dataBindingContext, isLatestQueryJob);

when(projectSelector.isDisposed()).thenReturn(false);
when(projectRepository.getProjects(credential)).thenReturn(projects);
when(projectRepository.getProjects()).thenReturn(projects);
when(isLatestQueryJob.test(queryJob)).thenReturn(true);
}

Expand All @@ -74,7 +74,7 @@ public void tearDown() {

@Test(expected = NullPointerException.class)
public void testNullCredential() {
new GcpProjectQueryJob(null /* credential */, projectRepository, projectSelector,
new GcpProjectQueryJob(projectRepository, projectSelector,
dataBindingContext, isLatestQueryJob);
}

Expand All @@ -83,7 +83,7 @@ public void testRun_setsProjects() throws InterruptedException, ProjectRepositor
queryJob.schedule();
queryJob.join();

verify(projectRepository).getProjects(credential);
verify(projectRepository).getProjects();
verify(isLatestQueryJob).test(queryJob);
verify(projectSelector).isDisposed();
verify(projectSelector).setProjects(projects);
Expand All @@ -96,7 +96,7 @@ public void testRun_abandonIfDisposed() throws InterruptedException, ProjectRepo
queryJob.schedule();
queryJob.join();

verify(projectRepository).getProjects(credential);
verify(projectRepository).getProjects();
verify(projectSelector, never()).setProjects(projects);
}

Expand All @@ -108,24 +108,21 @@ public void testRun_abandonIfNotLatestJob()
queryJob.schedule();
queryJob.join();

verify(projectRepository).getProjects(credential);
verify(projectRepository).getProjects();
verify(projectSelector, never()).setProjects(projects);
}

@Test
public void testRun_abandonStaleJob() throws InterruptedException, ProjectRepositoryException {
// Prepare another concurrent query job.
Credential staleCredential = mock(Credential.class);

List<GcpProject> anotherProjectList = new ArrayList<>();
anotherProjectList.add(null); // so not equals to projects

ProjectRepository projectRepository2 = mock(ProjectRepository.class);
when(projectRepository2.getProjects(staleCredential)).thenReturn(anotherProjectList);
when(projectRepository2.getProjects()).thenReturn(anotherProjectList);

// This second job is stale, i.e., it was fired, but user has selected another credential.
Predicate<Job> notLatest = job -> false;
Job staleJob = new GcpProjectQueryJob(staleCredential, projectRepository2,
Job staleJob = new GcpProjectQueryJob(projectRepository2,
projectSelector, dataBindingContext, notLatest);

queryJob.schedule();
Expand All @@ -134,8 +131,8 @@ public void testRun_abandonStaleJob() throws InterruptedException, ProjectReposi
staleJob.schedule();
staleJob.join();

verify(projectRepository).getProjects(credential);
verify(projectRepository2).getProjects(staleCredential);
verify(projectRepository).getProjects();
verify(projectRepository2).getProjects();

verify(projectSelector).setProjects(projects);
verify(projectSelector, never()).setProjects(anotherProjectList);
Expand Down
24 changes: 12 additions & 12 deletions ...le/cloud/tools/eclipse/appengine/deploy/ui/internal/AppEngineApplicationQueryJobTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ public void tearDown() {
@Test
public void testRun_projectHasNoApplication()
throws ProjectRepositoryException, InterruptedException {
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);
assertNull(project.getAppEngine());

queryJob.schedule();
queryJob.join();

verify(projectRepository).getAppEngineApplication(credential, "projectId");
verify(projectRepository).getAppEngineApplication("projectId");
verify(isLatestQueryJob).test(queryJob);
verify(projectSelector).isDisposed();
verify(projectSelection).isEmpty();
Expand All @@ -107,7 +107,7 @@ public void testRun_projectHasNoApplication()
@Test
public void testRun_accountDoesNotHavePermission()
throws ProjectRepositoryException, InterruptedException {
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenThrow(new ApplicationPermissionsException("testException", null));

queryJob.schedule();
Expand All @@ -125,7 +125,7 @@ public void testRun_accountDoesNotHavePermission()
public void testRun_projectHasApplication()
throws ProjectRepositoryException, InterruptedException {
AppEngine appEngine = AppEngine.withId("unique-id");
when(projectRepository.getAppEngineApplication(credential, "projectId")).thenReturn(appEngine);
when(projectRepository.getAppEngineApplication("projectId")).thenReturn(appEngine);

queryJob.schedule();
queryJob.join();
Expand All @@ -139,7 +139,7 @@ public void testRun_projectHasApplication()

@Test
public void testRun_queryError() throws ProjectRepositoryException, InterruptedException {
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenThrow(new ProjectRepositoryException("testException"));

queryJob.schedule();
Expand All @@ -155,7 +155,7 @@ public void testRun_queryError() throws ProjectRepositoryException, InterruptedE
@Test
public void testRun_abandonIfDisposed() throws InterruptedException, ProjectRepositoryException {
when(projectSelector.isDisposed()).thenReturn(true);
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);

queryJob.schedule();
Expand All @@ -169,7 +169,7 @@ public void testRun_abandonIfDisposed() throws InterruptedException, ProjectRepo
public void testRun_abandonIfNotLatestJob()
throws InterruptedException, ProjectRepositoryException {
when(isLatestQueryJob.test(queryJob)).thenReturn(false);
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);

queryJob.schedule();
Expand All @@ -182,7 +182,7 @@ public void testRun_abandonIfNotLatestJob()
@Test
public void testRun_abandonIfProjectSelectorHasNoSelection()
throws ProjectRepositoryException, InterruptedException {
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);
when(projectSelection.isEmpty()).thenReturn(true);

Expand All @@ -195,15 +195,15 @@ public void testRun_abandonIfProjectSelectorHasNoSelection()

@Test
public void testRun_abandonStaleJob() throws InterruptedException, ProjectRepositoryException {
when(projectRepository.getAppEngineApplication(credential, "projectId"))
when(projectRepository.getAppEngineApplication("projectId"))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);

// Prepare another concurrent query job.
Credential staleCredential = mock(Credential.class);

GcpProject staleProject = new GcpProject("name", "staleProjectId");
ProjectRepository projectRepository2 = mock(ProjectRepository.class);
when(projectRepository2.getAppEngineApplication(staleCredential, "staleProjectId"))
when(projectRepository2.getAppEngineApplication("staleProjectId"))
.thenThrow(new ProjectRepositoryException("testException"));

// This second job is stale, i.e., it was fired, but user has selected another credential.
Expand All @@ -217,8 +217,8 @@ public void testRun_abandonStaleJob() throws InterruptedException, ProjectReposi
staleJob.schedule();
staleJob.join();

verify(projectRepository).getAppEngineApplication(credential, "projectId");
verify(projectRepository2).getAppEngineApplication(staleCredential, "staleProjectId");
verify(projectRepository).getAppEngineApplication("projectId");
verify(projectRepository2).getAppEngineApplication("staleProjectId");

verify(projectSelector).setStatusLink(EXPECTED_MESSAGE_WHEN_NO_APPLICATION, EXPECTED_LINK);
verify(projectSelector, never()).setStatusLink(EXPECTED_MESSAGE_WHEN_EXCEPTION, null);
Expand Down
19 changes: 9 additions & 10 deletions ...ols/eclipse/appengine/deploy/ui/internal/ProjectSelectorSelectionChangedListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -99,7 +98,7 @@ public void testSelectionChanged_emptySelection() {
public void testSelectionChanged_repositoryException()
throws ProjectRepositoryException, InterruptedException {
initSelectionAndAccountSelector();
when(projectRepository.getAppEngineApplication(any(Credential.class), anyString()))
when(projectRepository.getAppEngineApplication(anyString()))
.thenThrow(new ProjectRepositoryException("testException"));

listener.selectionChanged(event);
Expand All @@ -112,7 +111,7 @@ public void testSelectionChanged_repositoryException()
public void testSelectionChanged_noAppEngineApplication()
throws ProjectRepositoryException, InterruptedException {
initSelectionAndAccountSelector();
when(projectRepository.getAppEngineApplication(any(Credential.class), anyString()))
when(projectRepository.getAppEngineApplication(anyString()))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);

listener.selectionChanged(event);
Expand All @@ -125,7 +124,7 @@ public void testSelectionChanged_noAppEngineApplication()
public void testSelectionChanged_hasAppEngineApplication()
throws ProjectRepositoryException, InterruptedException {
initSelectionAndAccountSelector();
when(projectRepository.getAppEngineApplication(any(Credential.class), anyString()))
when(projectRepository.getAppEngineApplication(anyString()))
.thenReturn(AppEngine.withId("id"));

listener.selectionChanged(event);
Expand All @@ -141,7 +140,7 @@ public void testSelectionChanged_doNotRunQueryJobIfCached() throws ProjectReposi

listener.selectionChanged(event);
assertNull(listener.latestQueryJob);
verify(projectRepository, never()).getAppEngineApplication(any(Credential.class), anyString());
verify(projectRepository, never()).getAppEngineApplication(anyString());
verify(projectSelector).clearStatusLink();
}

Expand All @@ -154,16 +153,16 @@ public void testSelectionChanged_whenCachedResultIsNoAppEngineApplication()

listener.selectionChanged(event);
assertNull(listener.latestQueryJob);
verify(projectRepository, never()).getAppEngineApplication(any(Credential.class), anyString());
verify(projectRepository, never()).getAppEngineApplication(anyString());
verify(projectSelector).setStatusLink(EXPECTED_MESSAGE_WHEN_NO_APPLICATION, EXPECTED_LINK);
}

@Test
public void testSelectionChanged_changeSelectedProject()
throws ProjectRepositoryException, InterruptedException {
when(projectRepository.getAppEngineApplication(any(Credential.class), eq("oldProjectId")))
when(projectRepository.getAppEngineApplication(eq("oldProjectId")))
.thenThrow(new ProjectRepositoryException("testException"));
when(projectRepository.getAppEngineApplication(any(Credential.class), eq("projectId")))
when(projectRepository.getAppEngineApplication(eq("projectId")))
.thenReturn(AppEngine.NO_APPENGINE_APPLICATION);

initSelectionAndAccountSelector(new GcpProject("oldProjectName", "oldProjectId"));
Expand All @@ -181,8 +180,8 @@ public void testSelectionChanged_changeSelectedProject()
assertNotEquals(oldJob, newJob);
newJob.join();

verify(projectRepository).getAppEngineApplication(any(Credential.class), eq("oldProjectId"));
verify(projectRepository).getAppEngineApplication(any(Credential.class), eq("projectId"));
verify(projectRepository).getAppEngineApplication(eq("oldProjectId"));
verify(projectRepository).getAppEngineApplication(eq("projectId"));
verify(projectSelector).setStatusLink(EXPECTED_MESSAGE_WHEN_NO_APPLICATION, EXPECTED_LINK);
}

Expand Down
22 changes: 11 additions & 11 deletions ...c/com/google/cloud/tools/eclipse/appengine/deploy/ui/AppEngineDeployPreferencesPanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,12 +406,12 @@ private void createProjectIdSection(IGoogleApiFactory apiFactory) {
refreshProjectsButton.addSelectionListener(new SelectionAdapter() {
@Override
public void widgetSelected(SelectionEvent event) {
refreshProjectsForSelectedCredential();
refreshProjectsForSelectedCredential(apiFactory);
}
});

accountSelector.addSelectionListener(
new RefreshProjectOnAccountSelection(refreshProjectsButton));
new RefreshProjectOnAccountSelection(refreshProjectsButton, apiFactory));

projectSelector.addSelectionChangedListener(
new ProjectSelectorSelectionChangedListener(accountSelector,
Expand Down Expand Up @@ -506,15 +506,14 @@ private Composite createBucketSection(Composite parent) {
@VisibleForTesting
public Job latestGcpProjectQueryJob; // Must be updated/accessed in the UI context.

private void refreshProjectsForSelectedCredential() {
private void refreshProjectsForSelectedCredential(IGoogleApiFactory apiFactory) {
projectSelector.setProjects(Collections.<GcpProject>emptyList());
latestGcpProjectQueryJob = null;

Credential selectedCredential = accountSelector.getSelectedCredential();
if (selectedCredential != null) {
if (apiFactory.hasCredentialsSet()) {
Predicate<Job> isLatestQueryJob = job -> job == latestGcpProjectQueryJob;
latestGcpProjectQueryJob = new GcpProjectQueryJob(selectedCredential,
projectRepository, projectSelector, bindingContext, isLatestQueryJob);
latestGcpProjectQueryJob = new GcpProjectQueryJob(projectRepository,
projectSelector, bindingContext, isLatestQueryJob);
latestGcpProjectQueryJob.schedule();

}
Expand All @@ -523,14 +522,16 @@ private void refreshProjectsForSelectedCredential() {
private final class RefreshProjectOnAccountSelection implements Runnable {

private final Button refreshProjectsButton;
private final IGoogleApiFactory apiFactory;

public RefreshProjectOnAccountSelection(Button refreshProjectsButton) {
public RefreshProjectOnAccountSelection(Button refreshProjectsButton, IGoogleApiFactory apiFactory) {
this.refreshProjectsButton = refreshProjectsButton;
this.apiFactory = apiFactory;
}

@Override
public void run() {
refreshProjectsForSelectedCredential();
refreshProjectsForSelectedCredential(apiFactory);
refreshProjectsButton.setEnabled(accountSelector.getSelectedCredential() != null);
}
}
Expand All @@ -549,8 +550,7 @@ public Object convert(Object fromObject) {

Preconditions.checkArgument(fromObject instanceof String);
try {
return projectRepository.getProject(accountSelector.getSelectedCredential(),
(String) fromObject);
return projectRepository.getProject((String) fromObject);
} catch (ProjectRepositoryException ex) {
return null;
}
Expand Down
6 changes: 2 additions & 4 deletions ....deploy.ui/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/GcpProjectQueryJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
*/
public class GcpProjectQueryJob extends Job {

private final Credential credential;
private final ProjectRepository projectRepository;
private final ProjectSelector projectSelector;
private final DataBindingContext dataBindingContext;
Expand All @@ -52,11 +51,10 @@ public class GcpProjectQueryJob extends Job {
* which determines if the job should update {@link ProjectSelector} or die silently. This
* predicate is executed in the UI context
*/
GcpProjectQueryJob(Credential credential, ProjectRepository projectRepository,
GcpProjectQueryJob(ProjectRepository projectRepository,
ProjectSelector projectSelector, DataBindingContext dataBindingContext,
Predicate<Job> isLatestQueryJob) {
super("Google Cloud Platform Projects Query Job");
this.credential = Preconditions.checkNotNull(credential);
this.projectRepository = Preconditions.checkNotNull(projectRepository);
this.projectSelector = Preconditions.checkNotNull(projectSelector);
this.dataBindingContext = Preconditions.checkNotNull(dataBindingContext);
Expand All @@ -68,7 +66,7 @@ public class GcpProjectQueryJob extends Job {
protected IStatus run(IProgressMonitor monitor) {
try {
final Job thisJob = this;
final List<GcpProject> projects = projectRepository.getProjects(credential);
final List<GcpProject> projects = projectRepository.getProjects();

// The selector may have been disposed (i.e., dialog closed); check it in the UI thread.
display.syncExec(() -> {
Expand Down
2 changes: 1 addition & 1 deletion ...google/cloud/tools/eclipse/appengine/deploy/ui/internal/AppEngineApplicationQueryJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public AppEngineApplicationQueryJob(GcpProject project, Credential credential,
@Override
protected IStatus run(IProgressMonitor monitor) {
try {
AppEngine appEngine = projectRepository.getAppEngineApplication(credential, project.getId());
AppEngine appEngine = projectRepository.getAppEngineApplication(project.getId());
project.setAppEngine(appEngine);

if (appEngine == AppEngine.NO_APPENGINE_APPLICATION) {
Expand Down
Loading