QPID-4696: Group broker attributes on editing UI, improve attribute descriptions, add attributes units.

           Remove obsolete GroupPrincipalAccessor.
           Fix flow control attribute validation.
           Fix constant name for default group provider attribute.
           Remove redundant comments.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1464115 13f79535-47bb-0310-9956-ffa450edef68

11 files changed, 187 insertions, 244 deletions

diff --git a/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js b/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js
index 699cbcfab8..5ff208d43f 100644
--- a/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js
+++ b/java/broker-plugins/management-http/src/main/java/resources/js/qpid/common/util.js
@@ -26,6 +26,7 @@ define(["dojo/_base/xhr",
         "dojo/dom-construct",
         "dojo/dom-geometry",
         "dojo/window",
+        "dijit/TitlePane",
         "dijit/Dialog",
         "dijit/form/Form",
         "dijit/form/Button",
@@ -163,6 +164,7 @@ define(["dojo/_base/xhr",
 
               var widgets = {};
               var requiredFor ={};
+              var groups = {};
               for(var i in attributeWidgetFactories)
               {
                  var attributeWidgetFactory = attributeWidgetFactories[i];
@@ -170,7 +172,37 @@ define(["dojo/_base/xhr",
                  var name = attributeWidgetFactory.name ? attributeWidgetFactory.name : widget.name;
                  widgets[name] = widget;
                  widget.initialValue = widget.value;
-                 layout.addChild(widget);
+                 var dotPos = name.indexOf(".");
+                 if (dotPos == -1)
+                 {
+                   layout.addChild(widget);
+                 }
+                 else
+                 {
+                   var groupName = name.substring(0, dotPos);
+                   var groupFieldContainer = null;
+                   if (groups.hasOwnProperty(groupName))
+                   {
+                       groupFieldContainer = groups[groupName];
+                   }
+                   else
+                   {
+                     groupFieldContainer = new dojox.layout.TableContainer({
+                           cols: 1,
+                           "labelWidth": "290",
+                           showLabels: true,
+                           orientation: "horiz",
+                           customClass: "formLabel"
+                      });
+                     groups[groupName] = groupFieldContainer;
+                     var groupTitle = attributeWidgetFactory.groupName ? attributeWidgetFactory.groupName :
+                                         groupName.charAt(0).toUpperCase() + groupName.slice(1);
+                     var panel = new dijit.TitlePane({title: groupTitle, toggleable: false, content: groupFieldContainer.domNode});
+                     dialogContentArea.appendChild(dom.create("br"));
+                     dialogContentArea.appendChild(panel.domNode);
+                   }
+                   groupFieldContainer.addChild(widget);
+                 }
                  if (attributeWidgetFactory.hasOwnProperty("requiredFor") && !data[name])
                  {
                    requiredFor[attributeWidgetFactory.requiredFor] = widget;
@@ -249,7 +281,7 @@ define(["dojo/_base/xhr",
              form.connectChildren(true);
              setAttributesDialog.startup();
              setAttributesDialog.on("show", function(){
-               var data = geometry.position(layout.domNode);
+               var data = geometry.position(dialogContentArea);
                var maxHeight = win.getBox().h * 0.6;
                if (data.h > maxHeight)
                {
diff --git a/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/Broker.js b/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/Broker.js
index 365133b9c7..66b888c0cf 100644
--- a/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/Broker.js
+++ b/java/broker-plugins/management-http/src/main/java/resources/js/qpid/management/Broker.js
@@ -125,15 +125,6 @@ define(["dojo/_base/xhr",
                              name: "keyStorePath"});
                        }
                }, {
-                       name: "keyStoreCertAlias",
-                       createWidget: function(brokerData) {
-                           return new dijit.form.ValidationTextBox({
-                             required: false,
-                             value: brokerData.keyStoreCertAlias,
-                             label: "Keystore certificate alias:",
-                             name: "keyStoreCertAlias"});
-                       }
-               }, {
                        name: "keyStorePassword",
                        requiredFor: "keyStorePath",
                        createWidget: function(brokerData) {
@@ -146,6 +137,15 @@ define(["dojo/_base/xhr",
                              });
                        }
                }, {
+                       name: "keyStoreCertAlias",
+                       createWidget: function(brokerData) {
+                           return new dijit.form.ValidationTextBox({
+                             required: false,
+                             value: brokerData.keyStoreCertAlias,
+                             label: "Keystore certificate alias:",
+                             name: "keyStoreCertAlias"});
+                       }
+               }, {
                        name: "trustStorePath",
                        createWidget: function(brokerData)
                        {
@@ -189,7 +189,32 @@ define(["dojo/_base/xhr",
                          });
                        }
                }, {
+                       name: "statisticsReportingPeriod",
+                       createWidget: function(brokerData) {
+                         return new dijit.form.ValidationTextBox({
+                           trim: "true",
+                           regexp: "[0-9]+",
+                           invalidMessage: "Invalid value",
+                           required: false,
+                           value: brokerData.statisticsReportingPeriod,
+                           placeholder: "Time in ms",
+                           label: "Statistics reporting period (ms):",
+                           name: "statisticsReportingPeriod"
+                         });
+                       }
+               }, {
+                       name: "statisticsReportingResetEnabled",
+                       createWidget: function(brokerData)
+                       {
+                         return new dijit.form.CheckBox({
+                           required: false, checked: brokerData.statisticsReportingResetEnabled, value: "true",
+                           label: "Statistics reporting period enabled:",
+                           name: "statisticsReportingResetEnabled"
+                         });
+                       }
+               }, {
                        name: "queue.alertThresholdQueueDepthMessages",
+                       groupName: "Global Queue Defaults",
                        createWidget: function(brokerData) {
                          return new dijit.form.ValidationTextBox({
                            trim: "true",
@@ -197,8 +222,8 @@ define(["dojo/_base/xhr",
                            invalidMessage: "Invalid value",
                            required: false,
                            value: brokerData["queue.alertThresholdQueueDepthMessages"],
-                           placeholder: "Count of messages",
-                           label: "Queue depth messages alert threshold:",
+                           placeholder: "Number of messages",
+                           label: "Depth alert threshold (messages):",
                            name: "queue.alertThresholdQueueDepthMessages"
                          });
                        }
@@ -212,7 +237,7 @@ define(["dojo/_base/xhr",
                          required: false,
                          value: brokerData["queue.alertThresholdQueueDepthBytes"],
                          placeholder: "Number of bytes",
-                         label: "Queue depth bytes alert threshold:",
+                         label: "Depth alert threshold (bytes):",
                          name: "queue.alertThresholdQueueDepthBytes"
                        });
                      }
@@ -226,7 +251,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["queue.alertThresholdMessageAge"],
                            placeholder: "Time in ms",
-                           label: "Queue message age alert threshold:",
+                           label: "Message age alert threshold (ms):",
                            name: "queue.alertThresholdMessageAge"
                          });
                        }
@@ -240,7 +265,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["queue.alertThresholdMessageSize"],
                            placeholder: "Size in bytes",
-                           label: "Queue message size alert threshold:",
+                           label: "Message size alert threshold (bytes):",
                            name: "queue.alertThresholdMessageSize"
                          });
                        }
@@ -254,7 +279,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["queue.alertRepeatGap"],
                            placeholder: "Time in ms",
-                           label: "Queue alert repeat gap:",
+                           label: "Alert repeat gap (ms):",
                            name: "queue.alertRepeatGap"
                          });
                        }
@@ -267,8 +292,8 @@ define(["dojo/_base/xhr",
                            invalidMessage: "Invalid value",
                            required: false,
                            value: brokerData["queue.maximumDeliveryAttempts"],
-                           placeholder: "Count of messages",
-                           label: "Queue maximum delivery retries:",
+                           placeholder: "Number of messages",
+                           label: "Maximum delivery retries (messages):",
                            name: "queue.maximumDeliveryAttempts"
                          });
                        }
@@ -293,7 +318,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["queue.flowControlSizeBytes"],
                            placeholder: "Size in bytes",
-                           label: "Queue flow capacity:",
+                           label: "Flow control threshold (bytes):",
                            name: "queue.flowControlSizeBytes",
                          });
                        }
@@ -307,12 +332,13 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["queue.flowResumeSizeBytes"],
                            placeholder: "Size in bytes",
-                           label: "Queue flow resume capacity:",
+                           label: "Flow resume threshold (bytes):",
                            name: "queue.flowResumeSizeBytes",
                          });
                        }
                }, {
                        name: "connection.sessionCountLimit",
+                       groupName: "Global Connection Defaults",
                        createWidget: function(brokerData)
                        {
                          return new dijit.form.NumberSpinner({
@@ -321,7 +347,7 @@ define(["dojo/_base/xhr",
                            value: brokerData["connection.sessionCountLimit"],
                            smallDelta: 1,
                            constraints: {min:1,max:65535,places:0, pattern: "#####"},
-                           label: "Connection session limit:",
+                           label: "Maximum number of sessions:",
                            name: "connection.sessionCountLimit"
                          });
                        }
@@ -335,36 +361,13 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["connection.heartBeatDelay"],
                            placeholder: "Time in ms",
-                           label: "Heart beat delay:",
+                           label: "Heart beat delay (ms):",
                            name: "connection.heartBeatDelay"
                          });
                        }
                }, {
-                       name: "statisticsReportingPeriod",
-                       createWidget: function(brokerData) {
-                         return new dijit.form.ValidationTextBox({
-                           trim: "true",
-                           regexp: "[0-9]+",
-                           invalidMessage: "Invalid value",
-                           required: false,
-                           value: brokerData.statisticsReportingPeriod,
-                           placeholder: "Time in ms",
-                           label: "Statistics reporting period:",
-                           name: "statisticsReportingPeriod"
-                         });
-                       }
-               }, {
-                       name: "statisticsReportingResetEnabled",
-                       createWidget: function(brokerData)
-                       {
-                         return new dijit.form.CheckBox({
-                           required: false, checked: brokerData.statisticsReportingResetEnabled, value: "true",
-                           label: "Statistics reporting period enabled:",
-                           name: "statisticsReportingResetEnabled"
-                         });
-                       }
-               }, {
                        name: "virtualhost.housekeepingCheckPeriod",
+                       groupName: "Global Virtual Host defaults",
                        createWidget: function(brokerData) {
                          return new dijit.form.ValidationTextBox({
                            trim: "true",
@@ -373,7 +376,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["virtualhost.housekeepingCheckPeriod"],
                            placeholder: "Time in ms",
-                           label: "House keeping check period:",
+                           label: "House keeping check period (ms):",
                            name: "virtualhost.housekeepingCheckPeriod"
                          });
                        }
@@ -387,7 +390,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["virtualhost.storeTransactionIdleTimeoutClose"],
                            placeholder: "Time in ms",
-                           label: "Idle store transaction close timeout:",
+                           label: "Idle store transaction close timeout (ms):",
                            name: "virtualhost.storeTransactionIdleTimeoutClose"
                          });
                        }
@@ -401,7 +404,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["virtualhost.storeTransactionIdleTimeoutWarn"],
                            placeholder: "Time in ms",
-                           label: "Idle store transaction warn timeout:",
+                           label: "Idle store transaction warn timeout (ms):",
                            name: "virtualhost.storeTransactionIdleTimeoutWarn"
                          });
                        }
@@ -415,7 +418,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["virtualhost.storeTransactionOpenTimeoutClose"],
                            placeholder: "Time in ms",
-                           label: "Open store transaction close timeout:",
+                           label: "Open store transaction close timeout (ms):",
                            name: "virtualhost.storeTransactionOpenTimeoutClose"
                          });
                        }
@@ -429,7 +432,7 @@ define(["dojo/_base/xhr",
                            required: false,
                            value: brokerData["virtualhost.storeTransactionOpenTimeoutWarn"],
                            placeholder: "Time in ms",
-                           label: "Open store transaction warn timeout:",
+                           label: "Open store transaction warn timeout (ms):",
                            name: "virtualhost.storeTransactionOpenTimeoutWarn"
                          });
                        }
@@ -530,7 +533,6 @@ define(["dojo/_base/xhr",
 
                              util.flattenStatistics( that.brokerData);
 
-                             that.showReadOnlyAttributes();
                              that.updateHeader();
 
                              var gridProperties = {
@@ -655,6 +657,7 @@ define(["dojo/_base/xhr",
 
            BrokerUpdater.prototype.updateHeader = function()
            {
+               showReadOnlyAttributes();
                var brokerData = this.brokerData;
                for(var i in this.attributes)
                {
@@ -715,7 +718,7 @@ define(["dojo/_base/xhr",
                dojo.byId("brokerAttribute.operatingSystem").innerHTML = brokerData.operatingSystem;
                dojo.byId("brokerAttribute.platform").innerHTML = brokerData.platform;
                dojo.byId("brokerAttribute.productVersion").innerHTML = brokerData.productVersion;
-               dojo.byId("brokerAttribute.managementVersion").innerHTML = brokerData.managementVersion;
+               dojo.byId("brokerAttribute.modelVersion").innerHTML = brokerData.managementVersion;
                dojo.byId("brokerAttribute.storeType").innerHTML = brokerData.storeType;
                dojo.byId("brokerAttribute.storeVersion").innerHTML = brokerData.storeVersion;
                dojo.byId("brokerAttribute.storePath").innerHTML = brokerData.storePath;
diff --git a/java/broker-plugins/management-http/src/main/java/resources/showBroker.html b/java/broker-plugins/management-http/src/main/java/resources/showBroker.html
index 0cac138cb1..887ca4e736 100644
--- a/java/broker-plugins/management-http/src/main/java/resources/showBroker.html
+++ b/java/broker-plugins/management-http/src/main/java/resources/showBroker.html
@@ -37,9 +37,9 @@
                 <div class="formLabel-labelCell" style="float:left; width: 250px;">Broker version:</div>
                 <div id="brokerAttribute.productVersion" style="float:left;"></div>
             </div>
-            <div id="brokerAttribute.managementVersion.container" style="clear:both">
-                <div class="formLabel-labelCell" style="float:left; width: 250px;">Broker management version:</div>
-                <div id="brokerAttribute.managementVersion" style="float:left;"></div>
+            <div id="brokerAttribute.modelVersion.container" style="clear:both">
+                <div class="formLabel-labelCell" style="float:left; width: 250px;">Broker model version:</div>
+                <div id="brokerAttribute.modelVersion" style="float:left;"></div>
             </div>
             <div id="brokerAttribute.storeType.container" style="clear:both">
                 <div class="formLabel-labelCell" style="float:left; width: 250px;">Broker store type:</div>
@@ -72,7 +72,7 @@
             <div id="brokerAttribute.keyStorePath.container" style="display: none; clear:both">
                 <div class="formLabel-labelCell" style="float:left; width: 250px;">Path to keystore:</div>
                 <div id="brokerAttribute.keyStorePath" style="float:left;"></div><br/>
-                <div class="formLabel-labelCell" style="float:left; width: 250px;">Keystore alias:</div>
+                <div class="formLabel-labelCell" style="float:left; width: 250px;">Keystore certificate alias:</div>
                 <div id="brokerAttribute.keyStoreCertAlias" style="float:left;"></div>
             </div>
             <div id="brokerAttribute.trustStorePath.container" style="display: none; clear:both">
@@ -95,75 +95,75 @@
             <br/>
             <div data-dojo-type="dijit.TitlePane" data-dojo-props="title: 'Global Queue Defaults',  open: true">
                 <div id="brokerAttribute.queue.alertThresholdQueueDepthMessages.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue depth messages alert threshold:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Depth messages alert threshold:</div>
                     <div id="brokerAttribute.queue.alertThresholdQueueDepthMessages" style="float:left;"></div>
                 </div>
                 <div id="brokerAttribute.queue.alertThresholdQueueDepthBytes.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue depth bytes alert threshold:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Depth bytes alert threshold:</div>
                     <div id="brokerAttribute.queue.alertThresholdQueueDepthBytes" style="float:left;"></div> bytes
                 </div>
                 <div id="brokerAttribute.queue.alertThresholdMessageAge.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue message age alert threshold:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Message age alert threshold:</div>
                     <div id="brokerAttribute.queue.alertThresholdMessageAge" style="float:left;"></div> ms
                 </div>
                 <div id="brokerAttribute.queue.alertThresholdMessageSize.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue message size alert threshold:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Message size alert threshold:</div>
                     <div id="brokerAttribute.queue.alertThresholdMessageSize" style="float:left;"></div> bytes
                 </div>
                 <div id="brokerAttribute.queue.alertRepeatGap.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue alert repeat gap:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Alert repeat gap:</div>
                     <div id="brokerAttribute.queue.alertRepeatGap" style="float:left;"></div> ms
                 </div>
                 <div id="brokerAttribute.queue.maximumDeliveryAttempts.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue maximum delivery retries:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Maximum delivery retries:</div>
                     <div id="brokerAttribute.queue.maximumDeliveryAttempts" style="float:left;"></div>
                 </div>
                 <div id="brokerAttribute.queue.deadLetterQueueEnabled.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Dead letter queue enabled:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Dead letter queue enabled:</div>
                     <div id="brokerAttribute.queue.deadLetterQueueEnabled" style="float:left;"></div>
                 </div>
                 <div id="brokerAttribute.queue.flowControlSizeBytes.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue flow capacity:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Flow control threshold:</div>
                     <div id="brokerAttribute.queue.flowControlSizeBytes" style="float:left;"></div> bytes
                 </div>
                 <div id="brokerAttribute.queue.flowResumeSizeBytes.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Queue flow resume capacity:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Flow resume threshold:</div>
                     <div id="brokerAttribute.queue.flowResumeSizeBytes" style="float:left;"></div> bytes
                 </div>
                 <div style="clear:both"></div>
             </div>
             <br/>
-            <div data-dojo-type="dijit.TitlePane" data-dojo-props="title: 'Global connection defaults',  open: true">
+            <div data-dojo-type="dijit.TitlePane" data-dojo-props="title: 'Global Connection Defaults',  open: true">
                 <div id="brokerAttribute.connection.sessionCountLimit.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Connection session limit:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Maximum number of sessions:</div>
                     <div id="brokerAttribute.connection.sessionCountLimit" style="float:left;"></div>
                 </div>
                 <div id="brokerAttribute.connection.heartBeatDelay.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Heart beat delay:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Heart beat delay:</div>
                     <div id="brokerAttribute.connection.heartBeatDelay" style="float:left;"></div> ms
                 </div>
                 <div style="clear:both"></div>
             </div>
             <br/>
-            <div data-dojo-type="dijit.TitlePane" data-dojo-props="title: 'Global virtual host defaults',  open: true">
+            <div data-dojo-type="dijit.TitlePane" data-dojo-props="title: 'Global Virtual Host Defaults',  open: true">
                 <div id="brokerAttribute.virtualhost.housekeepingCheckPeriod.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">House keeping check period:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">House keeping check period:</div>
                     <div id="brokerAttribute.virtualhost.housekeepingCheckPeriod" style="float:left;"></div>ms
                 </div>
                 <div id="brokerAttribute.virtualhost.storeTransactionIdleTimeoutClose.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Idle store transaction close timeout:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Idle store transaction close timeout:</div>
                     <div id="brokerAttribute.virtualhost.storeTransactionIdleTimeoutClose" style="float:left;"></div>ms
                 </div>
                 <div id="brokerAttribute.virtualhost.storeTransactionIdleTimeoutWarn.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Idle store transaction warn timeout:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Idle store transaction warn timeout:</div>
                     <div id="brokerAttribute.virtualhost.storeTransactionIdleTimeoutWarn" style="float:left;"></div>ms
                 </div>
                 <div id="brokerAttribute.virtualhost.storeTransactionOpenTimeoutClose.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Open store transaction close timeout:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Open store transaction close timeout:</div>
                     <div id="brokerAttribute.virtualhost.storeTransactionOpenTimeoutClose" style="float:left;"></div>ms
                 </div>
                 <div id="brokerAttribute.virtualhost.storeTransactionOpenTimeoutWarn.container" style="clear:both">
-                    <div class="formLabel-labelCell" style="float:left; width: 250px;">Open store transaction warn timeout:</div>
+                    <div class="formLabel-labelCell" style="float:left; width: 240px;">Open store transaction warn timeout:</div>
                     <div id="brokerAttribute.virtualhost.storeTransactionOpenTimeoutWarn" style="float:left;"></div>ms
                 </div>
                 <div style="clear:both"></div>
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
index 594ef7520a..f788923b3a 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
@@ -57,7 +57,6 @@ import org.apache.qpid.server.security.auth.UsernamePrincipal;
 import org.apache.qpid.server.security.auth.database.PrincipalDatabase;
 import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
 import org.apache.qpid.server.security.auth.manager.PrincipalDatabaseAuthenticationManager;
-import org.apache.qpid.server.security.group.GroupPrincipalAccessor;
 import org.apache.qpid.server.security.SecurityManager;
 
 public abstract class AuthenticationProviderAdapter<T extends AuthenticationManager> extends AbstractAdapter implements AuthenticationProvider
@@ -252,7 +251,7 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
     @Override
     public SubjectCreator getSubjectCreator()
     {
-        return new SubjectCreator(_authManager, new GroupPrincipalAccessor(_broker.getGroupProviders()));
+        return new SubjectCreator(_authManager, _broker.getGroupProviders());
     }
 
     @Override
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
index ec5a0402b4..e968d91e79 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
@@ -135,7 +135,7 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
     public static final long DEFAULT_STORE_TRANSACTION_OPEN_TIMEOUT_WARN = 0l;
     private static final String DEFAULT_KEY_STORE_NAME = "defaultKeyStore";
     private static final String DEFAULT_TRUST_STORE_NAME = "defaultTrustStore";
-    private static final String DEFAULT_GROUP_PROFIDER_NAME = "defaultGroupProvider";
+    private static final String DEFAULT_GROUP_PROVIDER_NAME = "defaultGroupProvider";
     private static final String DEFAULT_PEER_STORE_NAME = "defaultPeerStore";
 
     private static final String DUMMY_PASSWORD_MASK = "********";
@@ -238,13 +238,13 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         {
             GroupManager groupManager = new FileGroupManager(groupFile);
             UUID groupProviderId = UUIDGenerator.generateBrokerChildUUID(GroupProvider.class.getSimpleName(),
-                    DEFAULT_GROUP_PROFIDER_NAME);
+                    DEFAULT_GROUP_PROVIDER_NAME);
             GroupProviderAdapter groupProviderAdapter = new GroupProviderAdapter(groupProviderId, groupManager, this);
-            _groupProviders.put(DEFAULT_GROUP_PROFIDER_NAME, groupProviderAdapter);
+            _groupProviders.put(DEFAULT_GROUP_PROVIDER_NAME, groupProviderAdapter);
         }
         else
         {
-            _groupProviders.remove(DEFAULT_GROUP_PROFIDER_NAME);
+            _groupProviders.remove(DEFAULT_GROUP_PROVIDER_NAME);
         }
     }
 
@@ -1097,12 +1097,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
                 }
             }
         }
-
-        // the calls below are not thread safe but they should be fine in a management mode
-        // as there will be no user connected
-        // The new keystore/trustore/peerstore will be only used with new ports
-        // At the moment we cannot restart ports with new keystore/trustore/peerstore
-
         if (keyStoreChanged)
         {
             createKeyStore();
@@ -1155,16 +1149,20 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
             }
         }
         Long queueFlowControlSize = (Long) convertedAttributes.get(QUEUE_FLOW_CONTROL_SIZE_BYTES);
-        if (queueFlowControlSize != null && queueFlowControlSize > 0)
+        Long queueFlowControlResumeSize = (Long) convertedAttributes.get(QUEUE_FLOW_CONTROL_RESUME_SIZE_BYTES);
+        if (queueFlowControlSize != null || queueFlowControlResumeSize != null )
         {
-            Long queueFlowControlResumeSize = (Long) convertedAttributes.get(QUEUE_FLOW_CONTROL_RESUME_SIZE_BYTES);
+            if (queueFlowControlSize == null)
+            {
+                queueFlowControlSize = (Long)getAttribute(QUEUE_FLOW_CONTROL_SIZE_BYTES);
+            }
             if (queueFlowControlResumeSize == null)
             {
-                throw new IllegalConfigurationException("Flow control resume size attribute is not specified with flow control size attribute");
+                queueFlowControlResumeSize = (Long)getAttribute(QUEUE_FLOW_CONTROL_RESUME_SIZE_BYTES);
             }
-            if (queueFlowControlResumeSize >= queueFlowControlSize)
+            if (queueFlowControlResumeSize > queueFlowControlSize)
             {
-                throw new IllegalConfigurationException("Flow control resume size should be less then flow control size");
+                throw new IllegalConfigurationException("Flow resume size can't be greater than flow control size");
             }
         }
         for (String attributeName : POSITIVE_NUMERIC_ATTRIBUTES)
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
index 0fa834bc28..9ad58f9670 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
@@ -538,8 +538,6 @@ public class GroupProviderAdapter extends AbstractAdapter implements
             return true;
         }
         // TODO: DELETE state is ignored for now
-        // in case if we need to delete group provider, then we need AuthenticationProvider to be a change listener of it
-        // in order to remove deleted group provider from its group provider list
         return false;
     }
 
diff --git a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
index 59a2a50a24..4250de17a7 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java
@@ -73,11 +73,6 @@ public class PortAdapter extends AbstractAdapter implements Port
     private AuthenticationProvider _authenticationProvider;
     private AtomicReference<State> _state;
 
-    /*
-     * TODO register PortAceptor as a listener. For supporting multiple
-     * protocols on the same port we need to introduce a special entity like
-     * PortAceptor which will be responsible for port binding/unbinding
-     */
     public PortAdapter(UUID id, Broker broker, Map<String, Object> attributes, Map<String, Object> defaults, TaskExecutor taskExecutor)
     {
         super(id, defaults, MapValueConverter.convert(attributes, ATTRIBUTE_TYPES), taskExecutor);
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/SubjectCreator.java b/java/broker/src/main/java/org/apache/qpid/server/security/SubjectCreator.java
index 8138745486..213f19dc5c 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/security/SubjectCreator.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/SubjectCreator.java
@@ -21,17 +21,21 @@
 package org.apache.qpid.server.security;
 
 import java.security.Principal;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
 
 import javax.security.auth.Subject;
 import javax.security.sasl.SaslException;
 import javax.security.sasl.SaslServer;
 
+import org.apache.qpid.server.model.GroupProvider;
 import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 import org.apache.qpid.server.security.auth.AuthenticationResult;
 import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus;
 import org.apache.qpid.server.security.auth.SubjectAuthenticationResult;
 import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
-import org.apache.qpid.server.security.group.GroupPrincipalAccessor;
 
 /**
  * Creates a {@link Subject} formed by the {@link Principal}'s returned from:
@@ -48,12 +52,12 @@ import org.apache.qpid.server.security.group.GroupPrincipalAccessor;
 public class SubjectCreator
 {
     private AuthenticationManager _authenticationManager;
-    private GroupPrincipalAccessor _groupAccessor;
+    private Collection<GroupProvider> _groupProviders;
 
-    public SubjectCreator(AuthenticationManager authenticationManager, GroupPrincipalAccessor groupAccessor)
+    public SubjectCreator(AuthenticationManager authenticationManager, Collection<GroupProvider> groupProviders)
     {
         _authenticationManager = authenticationManager;
-        _groupAccessor = groupAccessor;
+        _groupProviders = groupProviders;
     }
 
    /**
@@ -112,7 +116,7 @@ public class SubjectCreator
             final Subject authenticationSubject = new Subject();
 
             authenticationSubject.getPrincipals().addAll(authenticationResult.getPrincipals());
-            authenticationSubject.getPrincipals().addAll(_groupAccessor.getGroupPrincipals(username));
+            authenticationSubject.getPrincipals().addAll(getGroupPrincipals(username));
 
             authenticationSubject.setReadOnly();
 
@@ -129,9 +133,24 @@ public class SubjectCreator
         Subject authenticationSubject = new Subject();
 
         authenticationSubject.getPrincipals().add(new AuthenticatedPrincipal(username));
-        authenticationSubject.getPrincipals().addAll(_groupAccessor.getGroupPrincipals(username));
+        authenticationSubject.getPrincipals().addAll(getGroupPrincipals(username));
         authenticationSubject.setReadOnly();
 
         return authenticationSubject;
     }
+
+    public Set<Principal> getGroupPrincipals(String username)
+    {
+        Set<Principal> principals = new HashSet<Principal>();
+        for (GroupProvider groupProvider : _groupProviders)
+        {
+            Set<Principal> groups = groupProvider.getGroupPrincipalsForUser(username);
+            if (groups != null)
+            {
+                principals.addAll(groups);
+            }
+        }
+
+        return Collections.unmodifiableSet(principals);
+    }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java b/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java
deleted file mode 100644
index 1b8cdc91bc..0000000000
--- a/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupPrincipalAccessor.java
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * 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.qpid.server.security.group;
-
-import java.security.Principal;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
-
-import org.apache.qpid.server.model.GroupProvider;
-
-public class GroupPrincipalAccessor
-{
-    private final Collection<GroupProvider> _groupProviders;
-
-    public GroupPrincipalAccessor(Collection<GroupProvider> groupProviders)
-    {
-        _groupProviders = groupProviders;
-    }
-
-    public Set<Principal> getGroupPrincipals(String username)
-    {
-        Set<Principal> principals = new HashSet<Principal>();
-        for (GroupProvider groupProvider : _groupProviders)
-        {
-            Set<Principal> groups = groupProvider.getGroupPrincipalsForUser(username);
-            if (groups != null)
-            {
-                principals.addAll(groups);
-            }
-        }
-
-        return Collections.unmodifiableSet(principals);
-    }
-}
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/SubjectCreatorTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/SubjectCreatorTest.java
index b1bc9bea68..9edd345360 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/security/SubjectCreatorTest.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/security/SubjectCreatorTest.java
@@ -23,19 +23,21 @@ import static org.mockito.Mockito.when;
 
 import java.security.Principal;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.Set;
 
 import javax.security.auth.Subject;
 import javax.security.sasl.SaslServer;
 
 import junit.framework.TestCase;
 
+import org.apache.qpid.server.model.GroupProvider;
 import org.apache.qpid.server.security.auth.AuthenticatedPrincipal;
 import org.apache.qpid.server.security.auth.AuthenticationResult;
 import org.apache.qpid.server.security.auth.AuthenticationResult.AuthenticationStatus;
 import org.apache.qpid.server.security.auth.SubjectAuthenticationResult;
 import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
-import org.apache.qpid.server.security.group.GroupPrincipalAccessor;
 
 public class SubjectCreatorTest extends TestCase
 {
@@ -43,13 +45,15 @@ public class SubjectCreatorTest extends TestCase
     private static final String PASSWORD = "password";
 
     private AuthenticationManager _authenticationManager = mock(AuthenticationManager.class);
-    private GroupPrincipalAccessor _groupPrincipalAccessor = mock(GroupPrincipalAccessor.class);
-    private SubjectCreator _subjectCreator = new SubjectCreator(_authenticationManager, _groupPrincipalAccessor);
+
+    private GroupProvider _groupManager1 = mock(GroupProvider.class);
+    private GroupProvider _groupManager2 = mock(GroupProvider.class);
 
     private Principal _userPrincipal = mock(Principal.class);
     private Principal _group1 = mock(Principal.class);
     private Principal _group2 = mock(Principal.class);
 
+    private SubjectCreator _subjectCreator;
     private AuthenticationResult _authenticationResult;
     private SaslServer _testSaslServer = mock(SaslServer.class);
     private byte[] _saslResponseBytes = PASSWORD.getBytes();
@@ -57,11 +61,12 @@ public class SubjectCreatorTest extends TestCase
     @Override
     public void setUp()
     {
+        when(_groupManager1.getGroupPrincipalsForUser(USERNAME)).thenReturn(Collections.singleton(_group1));
+        when(_groupManager2.getGroupPrincipalsForUser(USERNAME)).thenReturn(Collections.singleton(_group2));
+
+        _subjectCreator = new SubjectCreator(_authenticationManager, new HashSet<GroupProvider>(Arrays.asList(_groupManager1, _groupManager2)));
         _authenticationResult = new AuthenticationResult(_userPrincipal);
         when(_authenticationManager.authenticate(USERNAME, PASSWORD)).thenReturn(_authenticationResult);
-
-        when(_groupPrincipalAccessor.getGroupPrincipals(USERNAME))
-            .thenReturn(new HashSet<Principal>(Arrays.asList(_group1, _group2)));
     }
 
     public void testAuthenticateUsernameAndPasswordReturnsSubjectWithUserAndGroupPrincipals()
@@ -135,4 +140,30 @@ public class SubjectCreatorTest extends TestCase
         assertSame(expectedStatus, subjectAuthenticationResult.getStatus());
         assertNull(subjectAuthenticationResult.getSubject());
     }
+
+    public void testGetGroupPrincipals()
+    {
+        getAndAssertGroupPrincipals(_group1, _group2);
+    }
+
+    public void testGetGroupPrincipalsWhenAGroupManagerReturnsNull()
+    {
+        when(_groupManager1.getGroupPrincipalsForUser(USERNAME)).thenReturn(null);
+
+        getAndAssertGroupPrincipals(_group2);
+    }
+
+    public void testGetGroupPrincipalsWhenAGroupManagerReturnsEmptySet()
+    {
+        when(_groupManager2.getGroupPrincipalsForUser(USERNAME)).thenReturn(new HashSet<Principal>());
+
+        getAndAssertGroupPrincipals(_group1);
+    }
+
+    private void getAndAssertGroupPrincipals(Principal... expectedGroups)
+    {
+        Set<Principal> actualGroupPrincipals = _subjectCreator.getGroupPrincipals(USERNAME);
+        Set<Principal> expectedGroupPrincipals = new HashSet<Principal>(Arrays.asList(expectedGroups));
+        assertEquals(expectedGroupPrincipals, actualGroupPrincipals);
+    }
 }
diff --git a/java/broker/src/test/java/org/apache/qpid/server/security/group/GroupPrincipalAccessorTest.java b/java/broker/src/test/java/org/apache/qpid/server/security/group/GroupPrincipalAccessorTest.java
deleted file mode 100644
index e58a1a01f8..0000000000
--- a/java/broker/src/test/java/org/apache/qpid/server/security/group/GroupPrincipalAccessorTest.java
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * 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.qpid.server.security.group;
-
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import java.security.Principal;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
-
-import junit.framework.TestCase;
-
-import org.apache.qpid.server.model.GroupProvider;
-
-public class GroupPrincipalAccessorTest extends TestCase
-{
-    private static final String USERNAME = "username";
-
-    private GroupProvider _groupManager1 = mock(GroupProvider.class);
-    private GroupProvider _groupManager2 = mock(GroupProvider.class);
-
-    private Principal _group1 = mock(Principal.class);
-    private Principal _group2 = mock(Principal.class);
-
-    @Override
-    public void setUp()
-    {
-        when(_groupManager1.getGroupPrincipalsForUser(USERNAME)).thenReturn(Collections.singleton(_group1));
-        when(_groupManager2.getGroupPrincipalsForUser(USERNAME)).thenReturn(Collections.singleton(_group2));
-    }
-
-    public void testGetGroupPrincipals()
-    {
-        getAndAssertGroupPrincipals(_group1, _group2);
-    }
-
-    public void testGetGroupPrincipalsWhenAGroupManagerReturnsNull()
-    {
-        when(_groupManager1.getGroupPrincipalsForUser(USERNAME)).thenReturn(null);
-
-        getAndAssertGroupPrincipals(_group2);
-    }
-
-    public void testGetGroupPrincipalsWhenAGroupManagerReturnsEmptySet()
-    {
-        when(_groupManager2.getGroupPrincipalsForUser(USERNAME)).thenReturn(new HashSet<Principal>());
-
-        getAndAssertGroupPrincipals(_group1);
-    }
-
-    private void getAndAssertGroupPrincipals(Principal... expectedGroups)
-    {
-        GroupPrincipalAccessor groupPrincipalAccessor = new GroupPrincipalAccessor(Arrays.asList(_groupManager1, _groupManager2));
-
-        Set<Principal> actualGroupPrincipals = groupPrincipalAccessor.getGroupPrincipals(USERNAME);
-
-        Set<Principal> expectedGroupPrincipals = new HashSet<Principal>(Arrays.asList(expectedGroups));
-
-        assertEquals(expectedGroupPrincipals, actualGroupPrincipals);
-    }
-}