From 694b6413eefe56cc6097adfff73accddcfcf36a1 Mon Sep 17 00:00:00 2001 From: Steven Cooney Date: Wed, 19 Jun 2019 15:56:29 +0100 Subject: [PATCH] Remove some Logging duplication and Tidy-up Tidied up some of the logging to the build log by adding a wrapper class to do some formatting and pulling out hard-coded prefixing. Also improved the commenting through the TeamCity plugin. --- .../phabricator/AgentBuildExtension.java | 54 +++++++----- .../teamcity/phabricator/ArcanistClient.java | 35 ++++---- .../uk/xlab/teamcity/phabricator/Command.java | 27 +++++- .../teamcity/phabricator/CommandBuilder.java | 86 +++++++++++++------ .../teamcity/phabricator/CommonUtils.java | 18 +++- .../xlab/teamcity/phabricator/Constants.java | 5 ++ .../phabricator/PhabricatorPluginConfig.java | 38 ++++++-- .../logging/PhabricatorAgentLogger.java | 14 +-- .../PhabricatorBuildProgressLogger.java | 25 ++++++ .../logging/PhabricatorServerLogger.java | 14 +-- .../teamcity/phabricator/BuildTracker.java | 3 - .../PhabricatorBuildServerAdapter.java | 6 +- ...PhabricatorBuildStartContextProcessor.java | 6 ++ .../PhabricatorPluginBuildFeature.java | 5 ++ 14 files changed, 245 insertions(+), 91 deletions(-) create mode 100644 Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorBuildProgressLogger.java diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/AgentBuildExtension.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/AgentBuildExtension.java index 6a08699..5f17aa3 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/AgentBuildExtension.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/AgentBuildExtension.java @@ -10,15 +10,20 @@ import jetbrains.buildServer.agent.AgentBuildFeature; import jetbrains.buildServer.agent.AgentLifeCycleAdapter; import jetbrains.buildServer.agent.AgentLifeCycleListener; import jetbrains.buildServer.agent.AgentRunningBuild; -import jetbrains.buildServer.agent.BuildProgressLogger; import jetbrains.buildServer.util.EventDispatcher; import uk.xlab.teamcity.phabricator.logging.PhabricatorAgentLogger; +import uk.xlab.teamcity.phabricator.logging.PhabricatorBuildProgressLogger; +/** + * Extend the AgentLifeCycleAdapter to check for builds which have the + * Phabricator build feature enabled. If enabled then patch the updated code + * using arcanist making sure the build is against these changed sources. + * + */ public class AgentBuildExtension extends AgentLifeCycleAdapter { - private static final String OUTPUT_PREFIX = "Phabricator Plugin - %s"; private PhabricatorAgentLogger agentLogLogger; - private BuildProgressLogger buildLogger; + private PhabricatorBuildProgressLogger buildLogger; private PhabricatorPluginConfig phabricatorConfig; private boolean phabricatorTriggeredBuild = false; @@ -31,10 +36,18 @@ public class AgentBuildExtension extends AgentLifeCycleAdapter { phabricatorConfig.setLogger(agentLogLogger); } + /** + * From the associated parameters determine if the phabricator build feature is + * enabled and we have all the information to successfully patch changes from + * the phabricator differential revision + */ @Override public void buildStarted(@NotNull AgentRunningBuild runningBuild) { + // Reset the check for a phabricator build + phabricatorTriggeredBuild = false; + // Setup logger to print to build output - buildLogger = runningBuild.getBuildLogger(); + buildLogger = new PhabricatorBuildProgressLogger(runningBuild.getBuildLogger()); // Attempt to get the parameters set by the phabricator build feature. If non // are set then the feature is not turned on. @@ -42,7 +55,6 @@ public class AgentBuildExtension extends AgentLifeCycleAdapter { .getBuildFeaturesOfType(Constants.BUILD_FEATURE_TYPE); if (phabricatorBuildFeatureParameters.isEmpty()) { - phabricatorTriggeredBuild = false; return; } @@ -59,28 +71,26 @@ public class AgentBuildExtension extends AgentLifeCycleAdapter { // everything is present and correct for us to continue if (!phabricatorConfig.isPluginSetup()) { agentLogLogger.info("Plugin incorrectly configured"); - phabricatorTriggeredBuild = false; return; } phabricatorTriggeredBuild = true; agentLogLogger.info("Plugin ready"); - buildLogger.message(String.format(OUTPUT_PREFIX, "Active")); + buildLogger.message("Active"); - buildLogger.message(String.format(OUTPUT_PREFIX, String.format("%s: %s", Constants.PHABRICATOR_URL_SETTING, - phabricatorConfig.getPhabricatorURL().toString()))); - buildLogger.message(String.format(OUTPUT_PREFIX, String.format("%s: %s", - Constants.PHABRICATOR_ARCANIST_PATH_SETTING, phabricatorConfig.getPathToArcanist()))); - buildLogger.message(String.format(OUTPUT_PREFIX, - String.format("%s: %s", Constants.BUILD_ID, phabricatorConfig.getBuildId()))); - buildLogger.message(String.format(OUTPUT_PREFIX, - String.format("%s: %s", Constants.DIFF_ID, phabricatorConfig.getDiffId()))); - buildLogger.message(String.format(OUTPUT_PREFIX, String.format("%s: %s", Constants.HARBORMASTER_PHID, - phabricatorConfig.getPhabricatorURL().toString()))); - buildLogger.message(String.format(OUTPUT_PREFIX, - String.format("%s: %s", Constants.REVISION_ID, phabricatorConfig.getRevisionId()))); + buildLogger.logParameter(Constants.PHABRICATOR_URL_SETTING, phabricatorConfig.getPhabricatorURL().toString()); + buildLogger.logParameter(Constants.PHABRICATOR_ARCANIST_PATH_SETTING, phabricatorConfig.getPathToArcanist()); + buildLogger.logParameter(Constants.BUILD_ID, phabricatorConfig.getBuildId()); + buildLogger.logParameter(Constants.DIFF_ID, phabricatorConfig.getDiffId()); + buildLogger.logParameter(Constants.HARBORMASTER_PHID, phabricatorConfig.getHarbormasterPHID()); + buildLogger.logParameter(Constants.REVISION_ID, phabricatorConfig.getRevisionId()); } + /** + * Once the sources have been updated via the native TeamCity process check if + * the Phabricator build feature is enabled and use arcanist to patch in changes + * from a associated phabricator differential revision. + */ @Override public void sourcesUpdated(@NotNull AgentRunningBuild runningBuild) { @@ -88,7 +98,7 @@ public class AgentBuildExtension extends AgentLifeCycleAdapter { return; } - buildLogger.message(String.format(OUTPUT_PREFIX, "Attempting arc patch")); + buildLogger.message(String.format(Constants.LOGGING_PREFIX_TEMPLATE, "Attempting arc patch")); agentLogLogger.info("Attempting arc patch"); ArcanistClient arcanistClient = new ArcanistClient(phabricatorConfig.getPathToArcanist(), @@ -96,13 +106,15 @@ public class AgentBuildExtension extends AgentLifeCycleAdapter { phabricatorConfig.getConduitToken(), agentLogLogger); int patchCode = arcanistClient.patch(phabricatorConfig.getDiffId()); + agentLogLogger.info(String.format("Arc patch exited with code: %s", patchCode)); if (patchCode > 0) { runningBuild.stopBuild("Patch failed to apply. Check the agent output log for patch failure detals."); + agentLogLogger.info("Patch failed to apply, stopping build"); return; } - buildLogger.message(String.format(OUTPUT_PREFIX, "Patch completed")); + buildLogger.message(String.format(Constants.LOGGING_PREFIX_TEMPLATE, "Patch completed")); agentLogLogger.info("Patch completed"); } } diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/ArcanistClient.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/ArcanistClient.java index 5c25b66..24e10ca 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/ArcanistClient.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/ArcanistClient.java @@ -2,6 +2,10 @@ package uk.xlab.teamcity.phabricator; import uk.xlab.teamcity.phabricator.logging.PhabricatorAgentLogger; +/** + * Wrapper for arcanist and patching in changes from differential revisions + * + */ public class ArcanistClient { private final String arcPath; @@ -11,13 +15,8 @@ public class ArcanistClient { private final PhabricatorAgentLogger agentLogLogger; - public ArcanistClient( - final String pathToArcanist, - final String workingDirectory, - final String phabricatorURL, - final String conduitToken, - final PhabricatorAgentLogger logger - ) { + public ArcanistClient(final String pathToArcanist, final String workingDirectory, final String phabricatorURL, + final String conduitToken, final PhabricatorAgentLogger logger) { arcPath = pathToArcanist; workingDir = workingDirectory; conduitAPI = phabricatorURL; @@ -25,18 +24,24 @@ public class ArcanistClient { agentLogLogger = logger; } + /** + * Run "arc patch" using arcanist on the build agent to pull in changes from a + * differential revision. + * + * @param diffId The identifying diff which has the changes to patch within + * differential + * @return The exit code for "arc patch" or 1 if exceptions occur + */ public int patch(String diffId) { try { - Command commandToExecute = new CommandBuilder() - .setCommand(arcPath) - .setAction("patch") - .setWorkingDir(workingDir) - .setArg("--diff") - .setArg(diffId) + Command commandToExecute = new CommandBuilder().setCommand(arcPath).setAction("patch") + .setWorkingDir(workingDir).setArg("--diff").setArg(diffId) .setFlagWithValueEquals("--conduit-uri", conduitAPI) - .setFlagWithValueEquals("--conduit-token", token) - .build(); + .setFlagWithValueEquals("--conduit-token", token).build(); + + agentLogLogger.info(String.format("Running Command: %s", commandToExecute.toString())); + return commandToExecute.executeAndWait(); } catch (IllegalArgumentException e) { diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Command.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Command.java index c4945a3..fdf7e58 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Command.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Command.java @@ -4,6 +4,10 @@ import java.io.File; import static uk.xlab.teamcity.phabricator.CommonUtils.*; +/** + * Wrapper for executing a command on the host system + * + */ public class Command { private final File workingDir; @@ -11,17 +15,32 @@ public class Command { private ProcessBuilder processBuilder; private Process process; + /** + * Compose the command which is to be executed + * + * @param executableAndArguments An array of the command to be executed. + * @param workingDirectory The directory the command will be executed + * from. + */ public Command(final String[] executableAndArguments, final String workingDirectory) { workingDir = isNullOrEmpty(workingDirectory) ? null : new File(workingDirectory); fullCommand = executableAndArguments; - // ProcessBuilder requires the full command to be an array to prepend the - // executable to the front; + // ProcessBuilder requires the full command to be an array with the + // executable the first element; processBuilder = new ProcessBuilder(executableAndArguments); processBuilder.directory(workingDir); processBuilder.inheritIO(); } + /** + * Execute the command and wait for it to finish gathering the exit code of the + * process + * + * @return The exit code from the command which has been executed. + * @throws Exception If there is an exception when running the command throw to + * allowing the executing class to deal with error handling. + */ public int executeAndWait() throws Exception { // May well fail and throw an exception but we will handle that in the agent // plugin. @@ -29,6 +48,10 @@ public class Command { return process.waitFor(); } + /** + * Output the command and working directory for this class. It should only be + * used sparingly. + */ @Override public String toString() { if (fullCommand.length < 1) { diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommandBuilder.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommandBuilder.java index 17c828d..65e3698 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommandBuilder.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommandBuilder.java @@ -3,9 +3,12 @@ package uk.xlab.teamcity.phabricator; import static uk.xlab.teamcity.phabricator.CommonUtils.*; import java.util.ArrayList; import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +/** + * Build up the command which will be executed on the agent during relevant + * builds + * + */ public class CommandBuilder { private String command = null; @@ -13,49 +16,90 @@ public class CommandBuilder { private String workingDir = null; private List args = new ArrayList(); + /** + * Set the directory which the command should be run from. + * + * @param workingDir The directory the command should be run from. + * @return The builder instance so things can be chained. + */ public CommandBuilder setWorkingDir(String workingDir) { - if (isNullOrEmpty(workingDir)) + if (isNullOrEmpty(workingDir)) { throw new IllegalArgumentException("Need to provide valid working directory"); - else + } else { this.workingDir = workingDir; + } return this; } + /** + * Set the command/executable that will be run. + * + * @param cmd The command to be executed. + * @return The builder instance so things can be chained. + */ public CommandBuilder setCommand(String cmd) { - if (isNullOrEmpty(cmd)) + if (isNullOrEmpty(cmd)) { throw new IllegalArgumentException("Need to provide a valid command"); - else + } else { this.command = cmd; + } return this; } + /** + * Set the action the command will perform this comes immediately after the + * executable. E.g git pull where pull is the action. + * + * @param action + * @return The builder instance so things can be chained. + */ public CommandBuilder setAction(String action) { - if (isNullOrEmpty(action)) + if (isNullOrEmpty(action)) { throw new IllegalArgumentException("Need to provide a valid action"); - else + } else { this.action = action; + } return this; } + /** + * Set arguments that will be added to the command which will be executed + * + * @param arg An argument to be added to the command + * @return The builder instance so things can be chained. + */ public CommandBuilder setArg(String arg) { - if (isNullOrEmpty(arg)) + if (isNullOrEmpty(arg)) { throw new IllegalArgumentException("Need to provide a valid argument"); - else + } else { this.args.add(arg); + } return this; } + /** + * Set arguments which take the form of a key value pair. E.g token=12344 + * + * @param key The initial part of the argument + * @param value The value of the argument to be added after an the key and + * equals + * @return The builder instance so things can be chained. + */ public CommandBuilder setFlagWithValueEquals(String key, String value) { - this.args.add(String.format("%s=%s", formatFlag(key), value)); + this.args.add(String.format("%s=%s", key, value)); return this; } + /** + * Using the values set on this builder create a Command object than can be + * executed. + * + * @return Command which and be executed at a later point + */ public Command build() { if (isNullOrEmpty(this.command)) { throw new IllegalArgumentException("Must provide a command"); - } - - else { + } else { this.args.add(0, this.command); } @@ -65,18 +109,4 @@ public class CommandBuilder { return new Command(args.toArray(new String[args.size()]), this.workingDir); } - - private static String formatFlag(String flag) { - Pattern withFlag = Pattern.compile("^\\-\\-[a-zA-Z0-9-]+$"); - Pattern singleWord = Pattern.compile("^\\w$"); - Matcher m = withFlag.matcher(flag.trim()); - Matcher m1 = singleWord.matcher(flag.trim()); - if (m.matches()) { - return flag.trim(); - } else if (m1.matches()) { - return String.format("--%s", flag); - } else { - throw new IllegalArgumentException(String.format("%s is not a valid flag", flag)); - } - } } diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommonUtils.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommonUtils.java index 3e331a6..423cc76 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommonUtils.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommonUtils.java @@ -1,11 +1,27 @@ package uk.xlab.teamcity.phabricator; +/** + * A collection of helper methods that are used throughout the plugin + * + */ public final class CommonUtils { + /** + * Check if the given string is NULL or an empty string + * + * @param str String to be checked + * @return Whether the string is NULL or empty + */ public static Boolean isNullOrEmpty(String str) { return str == null || str.equals(""); } - + + /** + * Check if the given object is NULL + * + * @param obj Object to be checked for NULL + * @return Whether the object is NULL + */ public static Boolean isNull(Object obj) { return obj == null; } diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Constants.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Constants.java index 971c53b..535f764 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Constants.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Constants.java @@ -1,9 +1,14 @@ package uk.xlab.teamcity.phabricator; +/** + * A set of constants that are used throughout the server and agent plugins + * + */ public class Constants { public static final String PLUGIN_NAME = "phabricator"; public static final String PLUGIN_DISPLAY_NAME = "Phabricator Plugin"; + public static final String LOGGING_PREFIX_TEMPLATE = "Phabricator Plugin - %s"; // Build Feature Settings public static final String BUILD_FEATURE_TYPE = "phabricator-build-feature"; diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginConfig.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginConfig.java index 91f4ad0..10c2312 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginConfig.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginConfig.java @@ -48,14 +48,9 @@ public class PhabricatorPluginConfig { * @param parameters */ public void setParameters(Map parameters) { - // Clear the class variables to avoid any lingering references - phabricatorUrl = null; - conduitToken = null; - pathToArcanist = null; - buildId = null; - diffId = null; - harbormasterPHID = null; - revisionId = null; + // Clear the class variables to avoid any previous build configurations staying + // around + clearParameters(); params = parameters; @@ -109,6 +104,26 @@ public class PhabricatorPluginConfig { } } + /** + * A little method than can be used to clear all the parameters we want to set + */ + private void clearParameters() { + phabricatorUrl = null; + conduitToken = null; + pathToArcanist = null; + buildId = null; + diffId = null; + harbormasterPHID = null; + revisionId = null; + } + + /** + * Check that all the required parameters are set and we can continue to attempt + * patching changes from Arcanist + * + * @return Result of whether or not the plugin has the appropriate parameter to + * be used by an agent. + */ public boolean isPluginSetup() { if (!isNull(phabricatorUrl) && !isNull(pathToArcanist) && !isNullOrEmpty(buildId) && !isNullOrEmpty(diffId) && !isNullOrEmpty(harbormasterPHID) && !isNullOrEmpty(revisionId)) { @@ -146,6 +161,13 @@ public class PhabricatorPluginConfig { return revisionId; } + /** + * Parse the given URL for phabricator to verify it is not just a random string + * + * @param input phabricator URL + * @return Parsed URL object of the given string phabricator URL + * @throws MalformedURLException The given string is not a URL + */ private URL parsePhabricatorURL(String input) throws MalformedURLException { URL inputURL = new URL(input); return inputURL; diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorAgentLogger.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorAgentLogger.java index d735376..5cdaa18 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorAgentLogger.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorAgentLogger.java @@ -1,23 +1,27 @@ package uk.xlab.teamcity.phabricator.logging; import jetbrains.buildServer.log.Loggers; +import uk.xlab.teamcity.phabricator.Constants; +/** + * Logging class which outputs messages to TeamCity Agent Logs. Any logging from + * this class is also prefixed with this plugin's name + * + */ public final class PhabricatorAgentLogger implements PhabricatorPluginLogger { - private final String LOGGING_PREFIX_TEMPLATE = "Phabricator Plugin - %s"; - @Override public void info(String message) { - Loggers.AGENT.info(String.format(LOGGING_PREFIX_TEMPLATE, message)); + Loggers.AGENT.info(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message)); } @Override public void warn(String message, Exception e) { - Loggers.AGENT.warn(String.format(LOGGING_PREFIX_TEMPLATE, message), e); + Loggers.AGENT.warn(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message), e); } @Override public void error(String message, Exception e) { - Loggers.AGENT.error(String.format(LOGGING_PREFIX_TEMPLATE, message), e); + Loggers.AGENT.error(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message), e); } } diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorBuildProgressLogger.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorBuildProgressLogger.java new file mode 100644 index 0000000..6a5501d --- /dev/null +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorBuildProgressLogger.java @@ -0,0 +1,25 @@ +package uk.xlab.teamcity.phabricator.logging; + +import jetbrains.buildServer.agent.BuildProgressLogger; +import uk.xlab.teamcity.phabricator.Constants; + +/** + * Logging class which outputs messages to the Build Log. Any logging from this + * class is also prefixed with this plugin's name + * + */ +public class PhabricatorBuildProgressLogger { + private BuildProgressLogger buildLogger; + + public PhabricatorBuildProgressLogger(BuildProgressLogger buildProcessLogger) { + buildLogger = buildProcessLogger; + } + + public void message(String message) { + buildLogger.message(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message)); + } + + public void logParameter(String parameter, String value) { + buildLogger.message(String.format(Constants.LOGGING_PREFIX_TEMPLATE, String.format("%s: %s", parameter, value))); + } +} diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorServerLogger.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorServerLogger.java index a229542..8956b9b 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorServerLogger.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorServerLogger.java @@ -1,23 +1,27 @@ package uk.xlab.teamcity.phabricator.logging; import jetbrains.buildServer.log.Loggers; +import uk.xlab.teamcity.phabricator.Constants; +/** + * Logging class which outputs messages to TeamCity Server Logs. Any logging + * from this class is also prefixed with this plugin's name + * + */ public final class PhabricatorServerLogger implements PhabricatorPluginLogger { - private final String LOGGING_PREFIX_TEMPLATE = "Phabricator Plugin - %s"; - @Override public void info(String message) { - Loggers.SERVER.info(String.format(LOGGING_PREFIX_TEMPLATE, message)); + Loggers.SERVER.info(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message)); } @Override public void warn(String message, Exception e) { - Loggers.SERVER.warn(String.format(LOGGING_PREFIX_TEMPLATE, message), e); + Loggers.SERVER.warn(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message), e); } @Override public void error(String message, Exception e) { - Loggers.SERVER.error(String.format(LOGGING_PREFIX_TEMPLATE, message), e); + Loggers.SERVER.error(String.format(Constants.LOGGING_PREFIX_TEMPLATE, message), e); } } diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/BuildTracker.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/BuildTracker.java index b69fe12..8d29702 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/BuildTracker.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/BuildTracker.java @@ -25,8 +25,6 @@ import org.apache.http.util.EntityUtils; * Class that is run in a thread once a build has started. If the build does not * have the phabricator build feature then the tread should come to an end * otherwise wait for the build to finish and report back to phabricator - * - * @author steven.cooney * */ public class BuildTracker implements Runnable { @@ -61,7 +59,6 @@ public class BuildTracker implements Runnable { params.putAll(phabricatorBuildFeatureParameters.iterator().next().getParameters()); // Setup plugin specific configuration - // TODO: implement AppConfig as PluginConfig phabricatorConfig.setParameters(params); // Now we have set all the parameters we need to check if diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildServerAdapter.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildServerAdapter.java index c8896cd..4b649d2 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildServerAdapter.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildServerAdapter.java @@ -9,9 +9,9 @@ import jetbrains.buildServer.util.EventDispatcher; import uk.xlab.teamcity.phabricator.logging.PhabricatorServerLogger; /** - * Listen for builds been started and track their progress with BuildTracker - * - * @author steven.cooney + * Listen for builds to start by extending BuildServerAdapter. When a build + * starts spin up a thread of BuildTracker class to follow the progress of the + * build. * */ public class PhabricatorBuildServerAdapter extends BuildServerAdapter { diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildStartContextProcessor.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildStartContextProcessor.java index 3b246a6..53f9ac2 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildStartContextProcessor.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildStartContextProcessor.java @@ -5,6 +5,12 @@ import java.util.Map; import jetbrains.buildServer.serverSide.BuildStartContext; import jetbrains.buildServer.serverSide.BuildStartContextProcessor; +/** + * At the start of a build gather the parameters the agent plugin requires and + * set them in shared parameters. The agent can then read them from the shared + * parameters. + * + */ public class PhabricatorBuildStartContextProcessor implements BuildStartContextProcessor { @Override diff --git a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginBuildFeature.java b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginBuildFeature.java index 2884302..2e68e6d 100644 --- a/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginBuildFeature.java +++ b/Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginBuildFeature.java @@ -8,6 +8,11 @@ import jetbrains.buildServer.serverSide.BuildFeature; import jetbrains.buildServer.web.openapi.PluginDescriptor; import uk.xlab.teamcity.phabricator.logging.PhabricatorServerLogger; +/** + * Sets up a Build Feature for the phabricator plugin. Ultimately allowing the + * plugin to be configured through the TeamCity BuildFeature UI. + * + */ public class PhabricatorPluginBuildFeature extends BuildFeature { private final String myEditUrl;