fixes per code review in pull request

jenkinsci · Aug 10, 2016 · dd9a0bd · dd9a0bd
1 parent d65e0fe
commit dd9a0bd
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 10 deletions.
diff --git a/src/main/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsBuildWrapper.java b/src/main/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsBuildWrapper.java
@@ -295,7 +295,6 @@ public Object clone() {
         }
 
         @Override
-        @CheckForNull
         public boolean equals(Object obj) {
             if(obj == null) {
                 return false;
@@ -310,6 +309,7 @@ public boolean equals(Object obj) {
             return true;
         }
 
+        @CheckForNull
         public String getRegex() {
             return regex;
         }

diff --git a/src/main/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsConfig.java b/src/main/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsConfig.java
@@ -83,7 +83,7 @@ public class MaskPasswordsConfig {
     /**
      * Users can define regexes at the global level to mask in jobs.
      *
-     * <p>Never ever use this attribute directly: Use {@link #getGlobalVarPasswordPairsList} to avoid
+     * <p>Never ever use this attribute directly: Use {@link #getGlobalVarMaskRegexes} to avoid
      * potential NPEs.</p>
      *
      * @since 2.9
@@ -116,6 +116,7 @@ public MaskPasswordsConfig() {
     public void addGlobalVarPasswordPair(VarPasswordPair varPasswordPair) {
         // blank values are forbidden
         if(StringUtils.isBlank(varPasswordPair.getVar()) || StringUtils.isBlank(varPasswordPair.getPassword())) {
+            LOGGER.fine("addGlobalVarPasswordPair NOT adding pair with null var or password");
             return;
         }
         getGlobalVarPasswordPairsList().add(varPasswordPair);
@@ -132,6 +133,7 @@ public void addGlobalVarPasswordPair(VarPasswordPair varPasswordPair) {
     public void addGlobalVarMaskRegex(VarMaskRegex varMaskRegex) {
         // blank values are forbidden
         if(StringUtils.isBlank(varMaskRegex.getRegex())) {
+            LOGGER.fine("addGlobalVarMaskRegex NOT adding null regex");
             return;
         }
         getGlobalVarMaskRegexesList().add(varMaskRegex);

diff --git a/...ain/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsConsoleLogFilter.java b/...ain/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsConsoleLogFilter.java
@@ -1,8 +1,7 @@
 /*
  * The MIT License
  *
- * Copyright (c) 2010-2012, Manufacture Francaise des Pneumatiques Michelin,
- * Romain Seguy
+ * Copyright (c) 2016 Cox Automotive, Inc./Manheim, Jason Antman.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -67,7 +66,7 @@
  * GLOBAL Console Log Filter that alters the console so that passwords don't
  * get displayed.
  *
- * @author Jason Antman <jason@jasonantman.com>
+ * @author Jason Antman jason@jasonantman.com
  */
 @Extension
 public class MaskPasswordsConsoleLogFilter extends ConsoleLogFilter {

diff --git a/src/main/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsOutputStream.java b/src/main/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsOutputStream.java
@@ -30,6 +30,7 @@
 import java.io.OutputStream;
 import java.util.Collection;
 import java.util.regex.Pattern;
+import javax.annotation.CheckForNull;
 import org.apache.commons.lang.StringUtils;
 
 /**
@@ -48,8 +49,9 @@ public class MaskPasswordsOutputStream extends LineTransformationOutputStream {
      * @param logger The output stream to which this {@link MaskPasswordsOutputStream}
      *               will write to
      * @param passwords A collection of {@link String}s to be masked
+     * @param regexes A collection of Regular Expression {@link String}s to be masked
      */
-    public MaskPasswordsOutputStream(OutputStream logger, Collection<String> passwords, Collection<String> regexes) {
+    public MaskPasswordsOutputStream(OutputStream logger, @CheckForNull Collection<String> passwords, @CheckForNull Collection<String> regexes) {
         this.logger = logger;
 
 
@@ -93,6 +95,15 @@ public MaskPasswordsOutputStream(OutputStream logger, Collection<String> passwor
         }
     }
 
+    /**
+     * @param logger The output stream to which this {@link MaskPasswordsOutputStream}
+     *               will write to
+     * @param passwords A collection of {@link String}s to be masked
+     */
+    public MaskPasswordsOutputStream(OutputStream logger, @CheckForNull Collection<String> passwords) {
+        this(logger, passwords, null);
+    }
+
     @Override
     protected void eol(byte[] bytes, int len) throws IOException {
         String line = new String(bytes, 0, len);

diff --git a/src/test/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsWorkflowTest.java b/src/test/java/com/michelin/cio/hudson/plugins/maskpasswords/MaskPasswordsWorkflowTest.java
@@ -70,19 +70,34 @@ public void configRoundTrip() throws Exception {
             @Override
             public void evaluate() throws Throwable {
                 MaskPasswordsBuildWrapper bw1 = new MaskPasswordsBuildWrapper(
-                  Collections.singletonList(new MaskPasswordsBuildWrapper.VarPasswordPair("PASSWORD", "s3cr3t")),
-                  Collections.singletonList(new MaskPasswordsBuildWrapper.VarMaskRegex("foobar"))
+                  Collections.singletonList(new MaskPasswordsBuildWrapper.VarPasswordPair("PASSWORD", "s3cr3t"))
                 );
                 CoreWrapperStep step1 = new CoreWrapperStep(bw1);
                 CoreWrapperStep step2 = new StepConfigTester(story.j).configRoundTrip(step1);
                 MaskPasswordsBuildWrapper bw2 = (MaskPasswordsBuildWrapper) step2.getDelegate();
                 List<MaskPasswordsBuildWrapper.VarPasswordPair> pairs = bw2.getVarPasswordPairs();
-                List<MaskPasswordsBuildWrapper.VarMaskRegex> regexes = bw2.getVarMaskRegexes();
                 assertEquals(1, pairs.size());
-                assertEquals(1, regexes.size());
                 MaskPasswordsBuildWrapper.VarPasswordPair pair = pairs.get(0);
                 assertEquals("PASSWORD", pair.getVar());
                 assertEquals("s3cr3t", pair.getPassword());
+            }
+        });
+    }
+
+    @Test
+    public void regexConfigRoundTrip() throws Exception {
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                MaskPasswordsBuildWrapper bw1 = new MaskPasswordsBuildWrapper(
+                  null,
+                  Collections.singletonList(new MaskPasswordsBuildWrapper.VarMaskRegex("foobar"))
+                );
+                CoreWrapperStep step1 = new CoreWrapperStep(bw1);
+                CoreWrapperStep step2 = new StepConfigTester(story.j).configRoundTrip(step1);
+                MaskPasswordsBuildWrapper bw2 = (MaskPasswordsBuildWrapper) step2.getDelegate();
+                List<MaskPasswordsBuildWrapper.VarMaskRegex> regexes = bw2.getVarMaskRegexes();
+                assertEquals(1, regexes.size());
                 MaskPasswordsBuildWrapper.VarMaskRegex regex = regexes.get(0);
                 assertEquals("foobar", regex.getRegex());
             }