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
diff --git a/brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/policy/CreatePasswordSensorIntegrationTest.java b/brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/policy/CreatePasswordSensorIntegrationTest.java
new file mode 100644
index 0000000..360b705
--- /dev/null
+++ b/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);
+    }
+
+}
diff --git a/brooklyn-server/camp/camp-brooklyn/src/test/resources/EmptySoftwareProcessWithPassword.yaml b/brooklyn-server/camp/camp-brooklyn/src/test/resources/EmptySoftwareProcessWithPassword.yaml
new file mode 100644
index 0000000..2b6acb2
--- /dev/null
+++ b/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
+
diff --git a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensor.java b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensor.java
index 0352071..7b7a908 100644
--- a/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensor.java
+++ b/brooklyn-server/core/src/main/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensor.java
@@ -34,7 +34,7 @@
     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(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);
     }
diff --git a/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensorTest.java b/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensorTest.java
index 5223ce5..0dc1126 100644
--- a/brooklyn-server/core/src/test/java/org/apache/brooklyn/core/sensor/password/CreatePasswordSensorTest.java
+++ b/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() {
 
diff --git a/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java b/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java
index 25fb588..bea6208 100644
--- a/brooklyn-server/utils/common/src/main/java/org/apache/brooklyn/util/text/Identifiers.java
+++ b/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 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 @@
         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
diff --git a/brooklyn-server/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java b/brooklyn-server/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java
index 37c6443..e21bb81 100644
--- a/brooklyn-server/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java
+++ b/brooklyn-server/utils/common/src/test/java/org/apache/brooklyn/util/text/IdentifiersTest.java
@@ -102,11 +102,17 @@
         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'));
     }
     
 }