[
https://issues.apache.org/jira/browse/QPID-7279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15484446#comment-15484446
]
Alex Rudyy commented on QPID-7279:
----------------------------------
Rob,
I reviewed changes committed against this JIRA and here are my review comments:
# ExceptionHandlingFilter
## (!) Redundant exception logging is added into
ExceptionHandlingFilter.doFilter at line 86.
# BDBHAVirtualHostNodeImpl#onMaster
## (!) Store structure is not upgraded as call to
getConfigurationStore().upgradeStoreStructure() is missed in onMaster. As
result, upgrade will not occur whilst migrating HA brokers from 6.0 to 6.1.
## line 640: null check is redundant as non null initialRecords are returned
from getInitialRecords
{code}
createDefaultExchanges = initialRecords == null || initialRecords.length == 0;
{code}
# VirtualHost.setFirstOpening
## I am wondering whether it would be better to name this method
"restoreDefaultExchanges" or "recreateMandatoryEntities". As far as I
understood this method was introduced to re-create the default exchanges for
existing VH if they have been wiped out from the configuration store.I think
that name "setFirstOpening" is not expressing its purpose.
## Additionally, it seems that we do not cover the case when exchange records
are removed from the configuration store when VH is active and restarted. (For
example, when someone connected to JDBC VH database and executed delete
statements whilst VH is running.) This makes me thinking, that VH might need to
check the existence of default exchanges on every activation and recreate any
non-existing default exchange. If this approach would be taken the method
"setFirstOpening" might not be needed anymore. I think that adding checks for
default exchange and recreation of them (if required) on every VH activation
would simplify a bit an activation logic for VHNs as well.
What do you think?
# AbstractVirtualHost.onCreate
## It seems that look-up for children is redundant here as the children do not
exist yet in doCreation stage and will materialize on VH activation only. Thus,
setting _createDefaultExchanges to true would be sufficient here.
{code}
_createDefaultExchanges = getChildren(Exchange.class).isEmpty() &&
getChildren(Queue.class).isEmpty();
{code}
# DurableConfigurationStore implementations
## There is no state verification in methods 'upgradeStoreStructure '. It is
not an issue as such because upgradeStoreStructure is always called after init.
Just to be consistent with other store methods the state check can be added
into upgradeStoreStructure as well.
## Taking that state management logic is repeated through the every store
implementation I am wondering whether it would be better to introduce some sort
of ConfigurationStoreStateManager encapsulation store state operations?
> [Java Broker] with config encryption creating a JDBC VirtualHost fails
> ----------------------------------------------------------------------
>
> Key: QPID-7279
> URL: https://issues.apache.org/jira/browse/QPID-7279
> Project: Qpid
> Issue Type: Bug
> Components: Java Broker
> Reporter: Lorenz Quack
> Assignee: Rob Godfrey
> Fix For: qpid-java-6.1
>
>
> If configuration encryption is enabled on the broker creating a JDBC
> VirtualHost via the WMC fails with the following error:
> {{422 - Encrypted value is not valid Base 64 data: 'myPassword'}}
> Also we might not want to log the content of the invalid data due to its
> potential sensitive nature.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
