From 135dd19676e8b74d505f34f84ef887940e146331 Mon Sep 17 00:00:00 2001 From: Olivier Dagenais Date: Fri, 11 Oct 2013 14:56:59 +0000 Subject: [PATCH] Implemented TODO about disposing resources when done. --- .../plugins/tfs/TeamFoundationServerScm.java | 93 +++++++++++-------- .../java/hudson/plugins/tfs/model/Server.java | 12 ++- .../tfs/model/ServerIntegrationTest.java | 8 +- .../hudson/plugins/tfs/model/ServerTest.java | 8 +- 4 files changed, 73 insertions(+), 48 deletions(-) diff --git a/src/main/java/hudson/plugins/tfs/TeamFoundationServerScm.java b/src/main/java/hudson/plugins/tfs/TeamFoundationServerScm.java index 0286547fd..51b1486ed 100644 --- a/src/main/java/hudson/plugins/tfs/TeamFoundationServerScm.java +++ b/src/main/java/hudson/plugins/tfs/TeamFoundationServerScm.java @@ -154,47 +154,50 @@ private String substituteBuildParameter(Run run, String text) { @Override public boolean checkout(AbstractBuild build, Launcher launcher, FilePath workspaceFilePath, BuildListener listener, File changelogFile) throws IOException, InterruptedException { Server server = createServer(new TfTool(getDescriptor().getTfExecutable(), launcher, listener, workspaceFilePath), build); - WorkspaceConfiguration workspaceConfiguration = new WorkspaceConfiguration(server.getUrl(), getWorkspaceName(build, Computer.currentComputer()), getProjectPath(build), getLocalPath()); - - // Check if the configuration has changed - if (build.getPreviousBuild() != null) { - BuildWorkspaceConfiguration nodeConfiguration = new BuildWorkspaceConfigurationRetriever().getLatestForNode(build.getBuiltOn(), build.getPreviousBuild()); - if ((nodeConfiguration != null) && - nodeConfiguration.workspaceExists() - && (! workspaceConfiguration.equals(nodeConfiguration))) { - listener.getLogger().println("Deleting workspace as the configuration has changed since a build was performed on this computer."); - new RemoveWorkspaceAction(workspaceConfiguration.getWorkspaceName()).remove(server); - nodeConfiguration.setWorkspaceWasRemoved(); - nodeConfiguration.save(); - } - } - - build.addAction(workspaceConfiguration); - CheckoutAction action = new CheckoutAction(workspaceConfiguration.getWorkspaceName(), workspaceConfiguration.getProjectPath(), workspaceConfiguration.getWorkfolder(), isUseUpdate()); try { - List list = action.checkout(server, workspaceFilePath, (build.getPreviousBuild() != null ? build.getPreviousBuild().getTimestamp() : null), build.getTimestamp()); - ChangeSetWriter writer = new ChangeSetWriter(); - writer.write(list, changelogFile); - } catch (ParseException pe) { - listener.fatalError(pe.getMessage()); - throw new AbortException(); - } - - try { - setWorkspaceChangesetVersion(null); - String projectPath = workspaceConfiguration.getProjectPath(); - Project project = server.getProject(projectPath); - // TODO: even better would be to call this first, then use the changeset when calling checkout - int buildChangeset = project.getRemoteChangesetVersion(build.getTimestamp()); - setWorkspaceChangesetVersion(Integer.toString(buildChangeset, 10)); + WorkspaceConfiguration workspaceConfiguration = new WorkspaceConfiguration(server.getUrl(), getWorkspaceName(build, Computer.currentComputer()), getProjectPath(build), getLocalPath()); - // by adding this action, we prevent calcRevisionsFromBuild() from being called - build.addAction(new TFSRevisionState(buildChangeset, projectPath)); - } catch (ParseException pe) { - listener.fatalError(pe.getMessage()); - throw new AbortException(); + // Check if the configuration has changed + if (build.getPreviousBuild() != null) { + BuildWorkspaceConfiguration nodeConfiguration = new BuildWorkspaceConfigurationRetriever().getLatestForNode(build.getBuiltOn(), build.getPreviousBuild()); + if ((nodeConfiguration != null) && + nodeConfiguration.workspaceExists() + && (! workspaceConfiguration.equals(nodeConfiguration))) { + listener.getLogger().println("Deleting workspace as the configuration has changed since a build was performed on this computer."); + new RemoveWorkspaceAction(workspaceConfiguration.getWorkspaceName()).remove(server); + nodeConfiguration.setWorkspaceWasRemoved(); + nodeConfiguration.save(); + } + } + + build.addAction(workspaceConfiguration); + CheckoutAction action = new CheckoutAction(workspaceConfiguration.getWorkspaceName(), workspaceConfiguration.getProjectPath(), workspaceConfiguration.getWorkfolder(), isUseUpdate()); + try { + List list = action.checkout(server, workspaceFilePath, (build.getPreviousBuild() != null ? build.getPreviousBuild().getTimestamp() : null), build.getTimestamp()); + ChangeSetWriter writer = new ChangeSetWriter(); + writer.write(list, changelogFile); + } catch (ParseException pe) { + listener.fatalError(pe.getMessage()); + throw new AbortException(); + } + + try { + setWorkspaceChangesetVersion(null); + String projectPath = workspaceConfiguration.getProjectPath(); + Project project = server.getProject(projectPath); + // TODO: even better would be to call this first, then use the changeset when calling checkout + int buildChangeset = project.getRemoteChangesetVersion(build.getTimestamp()); + setWorkspaceChangesetVersion(Integer.toString(buildChangeset, 10)); + + // by adding this action, we prevent calcRevisionsFromBuild() from being called + build.addAction(new TFSRevisionState(buildChangeset, projectPath)); + } catch (ParseException pe) { + listener.fatalError(pe.getMessage()); + throw new AbortException(); + } + } finally { + server.close(); } - return true; } @@ -217,6 +220,8 @@ public boolean pollChanges(AbstractProject hudsonProject, Launcher launcher, Fil } catch (ParseException pe) { listener.fatalError(pe.getMessage()); throw new AbortException(); + } finally { + server.close(); } } } @@ -255,9 +260,13 @@ public boolean processWorkspaceBeforeDeletion(AbstractProject project, Fil LogTaskListener listener = new LogTaskListener(logger, Level.INFO); Launcher launcher = node.createLauncher(listener); Server server = createServer(new TfTool(getDescriptor().getTfExecutable(), launcher, listener, workspace), lastRun); - if (new RemoveWorkspaceAction(configuration.getWorkspaceName()).remove(server)) { - configuration.setWorkspaceWasRemoved(); - configuration.save(); + try { + if (new RemoveWorkspaceAction(configuration.getWorkspaceName()).remove(server)) { + configuration.setWorkspaceWasRemoved(); + configuration.save(); + } + } finally { + server.close(); } } return true; @@ -463,6 +472,8 @@ protected PollingResult compareRemoteRevisionWith( } catch (ParseException pe) { listener.fatalError(pe.getMessage()); throw new AbortException(); + } finally { + server.close(); } } } diff --git a/src/main/java/hudson/plugins/tfs/model/Server.java b/src/main/java/hudson/plugins/tfs/model/Server.java index 31331bcd6..7a35789b4 100644 --- a/src/main/java/hudson/plugins/tfs/model/Server.java +++ b/src/main/java/hudson/plugins/tfs/model/Server.java @@ -20,8 +20,9 @@ import com.microsoft.tfs.core.httpclient.UsernamePasswordCredentials; import com.microsoft.tfs.core.util.CredentialsUtils; import com.microsoft.tfs.core.util.URIUtils; +import com.microsoft.tfs.util.Closable; -public class Server implements ServerConfigurationProvider { +public class Server implements ServerConfigurationProvider, Closable { private static final String nativeFolderPropertyName = "com.microsoft.tfs.jni.native.base-directory"; private final String url; @@ -51,8 +52,6 @@ else if (username != null && password != null) { if (credentials != null) { ensureNativeLibrariesConfigured(); - // TODO: TFSTeamProjectCollection implements Closeable - // and should be disposed as soon as no longer needed. this.tpc = new TFSTeamProjectCollection(uri, credentials); } else { @@ -118,4 +117,11 @@ public String getUserPassword() { public String getLocalHostname() throws IOException, InterruptedException { return tool.getHostname(); } + + public synchronized void close() { + if (this.tpc != null) { + this.tpc.close(); + } + + } } diff --git a/src/test/java/hudson/plugins/tfs/model/ServerIntegrationTest.java b/src/test/java/hudson/plugins/tfs/model/ServerIntegrationTest.java index 57695229c..5743c86bc 100644 --- a/src/test/java/hudson/plugins/tfs/model/ServerIntegrationTest.java +++ b/src/test/java/hudson/plugins/tfs/model/ServerIntegrationTest.java @@ -19,7 +19,11 @@ public class ServerIntegrationTest { */ public void canFindTfsSdkNativeLibraries() { final Server server = new Server(null, "http://tfs.invalid:8080/tfs", "username", "password"); - final TFSTeamProjectCollection tpc = server.getTeamProjectCollection(); - tpc.getVersionControlClient(); + try { + final TFSTeamProjectCollection tpc = server.getTeamProjectCollection(); + tpc.getVersionControlClient(); + } finally { + server.close(); + } } } diff --git a/src/test/java/hudson/plugins/tfs/model/ServerTest.java b/src/test/java/hudson/plugins/tfs/model/ServerTest.java index 115ed7ddd..213824a4b 100644 --- a/src/test/java/hudson/plugins/tfs/model/ServerTest.java +++ b/src/test/java/hudson/plugins/tfs/model/ServerTest.java @@ -45,7 +45,11 @@ public void assertGetProjectWithDifferentProjectPathReturnsNotSameInstance() { public void assertLocalHostnameIsRetrievedFromTfTool() throws Exception { when(tool.getHostname()).thenReturn("thehostname"); Server server = new Server(tool, "url", null, null); - assertEquals("Hostname was incorrect", "thehostname", server.getLocalHostname()); - verify(tool).getHostname(); + try { + assertEquals("Hostname was incorrect", "thehostname", server.getLocalHostname()); + verify(tool).getHostname(); + } finally { + server.close(); + } } }