Changes re code review

Reorder password so "required chars" not at beginning
More default symbols
yaml test
concatenate all characters to use as available characters once required
chars added
Minor other changes

brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/policy/CreatePasswordSensorIntegrationTest.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.camp.brooklyn.policy;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.sensor.AttributeSensor;
+import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
+import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess;
+import org.apache.brooklyn.test.Asserts;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.Iterables;
+
+public class CreatePasswordSensorIntegrationTest extends AbstractYamlTest {
+
+    private static final Logger LOG = LoggerFactory.getLogger(CreatePasswordSensorIntegrationTest.class);
+    AttributeSensor<String> PASSWORD_1 = Sensors.newStringSensor("test.password.1", "Host name as known internally in " +
+            "the subnet where it is running (if different to host.name)");
+    AttributeSensor<String> PASSWORD_2 = Sensors.newStringSensor("test.password.2", "Host name as known internally in " +
+            "the subnet where it is running (if different to host.name)");
+
+    @Test(groups = "Integration")
+    public void testProvisioningProperties() throws Exception {
+        final Entity app = createAndStartApplication(loadYaml("EmptySoftwareProcessWithPassword.yaml"));
+
+        waitForApplicationTasks(app);
+        EmptySoftwareProcess entity = Iterables.getOnlyElement(Entities.descendants(app, EmptySoftwareProcess.class));
+
+        assertPasswordLength(entity, PASSWORD_1, 15);
+
+        assertPasswordOnlyContains(entity, PASSWORD_2, "abc");
+
+    }
+
+    private void assertPasswordOnlyContains(EmptySoftwareProcess entity, AttributeSensor<String> password, String acceptableChars) {
+        String attribute_2 = entity.getAttribute(password);
+        for (char c : attribute_2.toCharArray()) {
+            Asserts.assertTrue(acceptableChars.indexOf(c) != -1);
+        }
+    }
+
+    private void assertPasswordLength(EmptySoftwareProcess entity, AttributeSensor<String> password, int expectedLength) {
+        String attribute_1 = entity.getAttribute(password);
+        Asserts.assertEquals(attribute_1.length(), expectedLength);
+    }
+
+}

brooklyn-server/camp/camp-brooklyn/src/test/resources/EmptySoftwareProcessWithPassword.yaml

@@ -0,0 +1,17 @@
+name: example-with-CreatePasswordSensor
+description: |
+  Creates an emptyService and then attaches a password to it
+origin: https://github.com/apache/incubator-brooklyn
+location: localhost
+services:
+- type: org.apache.brooklyn.entity.software.base.EmptySoftwareProcess
+  brooklyn.initializers:
+  - type: org.apache.brooklyn.core.sensor.password.CreatePasswordSensor
+    brooklyn.config:
+      name: test.password.1
+      password.length: 15
+  - type: org.apache.brooklyn.core.sensor.password.CreatePasswordSensor
+    brooklyn.config:
+      name: test.password.2
+      password.chars: abc
+

brooklyn-server/core/src/main/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensor.java

@@ -34,7 +34,7 @@ public class CreatePasswordSensor extends AddSensor<String> {
     public static final ConfigKey<String> ACCEPTABLE_CHARS = ConfigKeys.newStringConfigKey("password.chars", "The characters allowed in password");
 
     private Integer passwordLength;
-    private String accecptableChars;
+    private String acceptableChars;
 
     public CreatePasswordSensor(Map<String, String> params) {
         this(ConfigBag.newInstance(params));
@@ -43,16 +43,16 @@ public CreatePasswordSensor(Map<String, String> params) {
     public CreatePasswordSensor(ConfigBag params) {
         super(params);
         passwordLength = params.get(PASSWORD_LENGTH);
-        accecptableChars = params.get(ACCEPTABLE_CHARS);
+        acceptableChars = params.get(ACCEPTABLE_CHARS);
     }
 
     @Override
     public void apply(EntityLocal entity) {
         super.apply(entity);
 
-        String password = accecptableChars == null
+        String password = acceptableChars == null
                 ? Identifiers.makeRandomPassword(passwordLength)
-                : Identifiers.makeRandomPassword(passwordLength, accecptableChars);
+                : Identifiers.makeRandomPassword(passwordLength, acceptableChars);
 
         entity.sensors().set(sensor, password);
     }

brooklyn-server/core/src/test/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensorTest.java

@@ -21,39 +21,31 @@
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.location.Location;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
-import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.sensor.Sensors;
-import org.apache.brooklyn.core.test.entity.TestApplication;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.core.config.ConfigBag;
-import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableList;
 
-public class CreatePasswordSensorTest {
+public class CreatePasswordSensorTest extends BrooklynAppUnitTestSupport{
 
     final static AttributeSensor<String> SENSOR_STRING = Sensors.newStringSensor("aString");
     private EntityInternal entity;
-    private TestApplication app;
 
-    @BeforeMethod
+    @BeforeMethod(alwaysRun = true)
     public void setup() throws Exception {
-        app = TestApplication.Factory.newManagedInstanceForTests();
+        super.setUp();
 
         entity = app.createAndManageChild(EntitySpec.create(TestEntity.class)
                 .location(app.newLocalhostProvisioningLocation().obtain()));
         app.start(ImmutableList.<Location>of());
     }
 
-    @AfterMethod(alwaysRun = true)
-    public void tearDown() throws Exception {
-        if (app != null) Entities.destroyAll(app.getManagementContext());
-    }
-
     @Test
     public void testCreatePasswordCreatesAPasswordOfDefaultLength() {
 

brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java

@@ -19,8 +19,14 @@
 package org.apache.brooklyn.util.text;
 
 import java.security.SecureRandom;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Random;
+import java.util.Set;
 
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 
 public class Identifiers {
@@ -30,7 +36,7 @@ public class Identifiers {
     public static final String UPPER_CASE_ALPHA = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
     public static final String LOWER_CASE_ALPHA = "abcdefghijklmnopqrstuvwxyz";
     public static final String NUMERIC = "1234567890";
-    public static final String NON_ALPHA_NUMERIC = "!$%@#";
+    public static final String NON_ALPHA_NUMERIC = "!@$%^&*()-_=+[]{};:\\|/?,.<>~";
 
     /** @see #JAVA_GOOD_PACKAGE_OR_CLASS_REGEX */
     public static final String JAVA_GOOD_START_CHARS = UPPER_CASE_ALPHA + LOWER_CASE_ALPHA +"_";
@@ -129,18 +135,30 @@ public static String makeRandomPassword(final int length, String... passwordVali
         Preconditions.checkState(length >= passwordValidCharsPool.length);
         int l = 0;
 
-        char[] id = new char[length];
+        Character[] password = new Character[length];
 
         for(int i = 0; i < passwordValidCharsPool.length; i++){
-            id[l++] = pickRandomCharFrom(passwordValidCharsPool[i]);
+            password[l++] = pickRandomCharFrom(passwordValidCharsPool[i]);
         }
 
-        String remainingValidChars = passwordValidCharsPool[passwordValidCharsPool.length - 1];
+        String remainingValidChars = mergeCharacterSets(passwordValidCharsPool);
 
         while(l < length) {
-            id[l++] = pickRandomCharFrom(remainingValidChars);
+            password[l++] = pickRandomCharFrom(remainingValidChars);
         }
-        return new String(id);
+
+        List<Character> list = Arrays.asList(password);
+        Collections.shuffle(list);
+        return Joiner.on("").join(list);
+    }
+
+    protected static String mergeCharacterSets(String... s) {
+        Set characters = new HashSet<Character>();
+        for (String characterSet : s) {
+            characters.addAll(Arrays.asList(characterSet.split("")));
+        }
+
+        return Joiner.on("").join(characters);
     }
 
     /** creates a short identifier comfortable in java and OS's, given an input hash code

brooklyn-server/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java

@@ -102,11 +102,17 @@ public void testPassword() {
         String upper = "ABC";
         String numbers = "123";
         String symbols = "!£$";
-        String id1 = Identifiers.makeRandomPassword(4, upper, numbers, symbols, Identifiers.PASSWORD_VALID_CHARS);
+        String password = Identifiers.makeRandomPassword(4, upper, numbers, symbols, Identifiers.PASSWORD_VALID_CHARS);
 
-        Assert.assertTrue(upper.contains(id1.substring(0,1)));
-        Assert.assertTrue(numbers.contains(id1.substring(1,2)));
-        Assert.assertTrue(symbols.contains(id1.substring(2,3)));
+        Assert.assertTrue(password.matches(".*[" + upper + "].*"));
+        Assert.assertTrue(password.matches(".*[" + numbers + "].*"));
+        Assert.assertTrue(password.matches(".*[" + symbols + "].*"));
+    }
+
+    @Test
+    public void testCharMerge() {
+        String characters = Identifiers.mergeCharacterSets("abc", "bcd", "ghjj");
+        Assert.assertEquals(characters.indexOf('b'), characters.lastIndexOf('b'));
     }
 
 }