revi-archive/Teamcity-Phabricator-Plugin
1
0
Fork 0
Code Activity

Remove some Logging duplication and Tidy-up

Browse source 
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.
This commit is contained in:
Steven Cooney 2019-06-19 15:56:29 +01:00
parent 3564d14927
commit 694b6413ee
14 changed files with 245 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

54
Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/AgentBuildExtension.java
View file

@ -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");
}
}

35
Teamcity-Phabricator-Plugin/phabricator-plugin-agent/src/main/java/uk/xlab/teamcity/phabricator/ArcanistClient.java
View file

@ -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) {

27
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Command.java
View file

@ -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) {

86
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommandBuilder.java
View file

@ -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<String> args = new ArrayList<String>();
/**
* 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));
}
}
}

18
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/CommonUtils.java
View file

@ -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;
}

5
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/Constants.java
View file

@ -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";

38
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginConfig.java
View file

@ -48,14 +48,9 @@ public class PhabricatorPluginConfig {
* @param parameters
*/
public void setParameters(Map<String, String> 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;

14
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorAgentLogger.java
View file

@ -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);
}
}

25
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorBuildProgressLogger.java Normal file
View file

@ -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)));
}
}

14
Teamcity-Phabricator-Plugin/phabricator-plugin-common/src/main/java/uk/xlab/teamcity/phabricator/logging/PhabricatorServerLogger.java
View file

@ -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);
}
}

3
Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/BuildTracker.java
View file

@ -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

6
Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildServerAdapter.java
View file

@ -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 {

6
Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorBuildStartContextProcessor.java
View file

@ -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

5
Teamcity-Phabricator-Plugin/phabricator-plugin-server/src/main/java/uk/xlab/teamcity/phabricator/PhabricatorPluginBuildFeature.java
View file

@ -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;