From d5b2c0ea6ee32c86433cfe310374275594184d06 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Fri, 3 May 2013 11:21:16 +0000 Subject: QPID-4803: Ensure the modelVersion and storeVersion attributes are saved to the configuration store and validated at startup git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1478732 13f79535-47bb-0310-9956-ffa450edef68 --- .../configuration/startup/BrokerRecoverer.java | 44 +++++++++++++- .../store/MemoryConfigurationEntryStore.java | 20 ++++++- .../java/org/apache/qpid/server/model/Model.java | 1 + .../qpid/server/model/adapter/BrokerAdapter.java | 29 ++++----- .../broker/src/main/resources/initial-config.json | 1 + .../BrokerConfigurationStoreCreatorTest.java | 7 ++- .../configuration/startup/BrokerRecovererTest.java | 70 +++++++++++++++++++++- .../store/JsonConfigurationEntryStoreTest.java | 68 ++++++++++++++++++++- qpid/java/systests/etc/config-systests.json | 12 +--- 9 files changed, 218 insertions(+), 34 deletions(-) (limited to 'qpid/java') diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java index 35c96bc993..4b7b9e3254 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import org.apache.qpid.server.BrokerOptions; import org.apache.qpid.server.configuration.ConfigurationEntry; @@ -39,6 +40,7 @@ import org.apache.qpid.server.model.AuthenticationProvider; import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.KeyStore; +import org.apache.qpid.server.model.Model; import org.apache.qpid.server.model.TrustStore; import org.apache.qpid.server.model.adapter.AccessControlProviderFactory; import org.apache.qpid.server.model.adapter.AuthenticationProviderFactory; @@ -46,10 +48,13 @@ import org.apache.qpid.server.model.adapter.BrokerAdapter; import org.apache.qpid.server.model.adapter.GroupProviderFactory; import org.apache.qpid.server.model.adapter.PortFactory; import org.apache.qpid.server.stats.StatisticsGatherer; +import org.apache.qpid.server.util.MapValueConverter; import org.apache.qpid.server.virtualhost.VirtualHostRegistry; public class BrokerRecoverer implements ConfiguredObjectRecoverer { + private static final Pattern MODEL_VERSION_PATTERN = Pattern.compile("^\\d+\\.\\d+$"); + private final StatisticsGatherer _statisticsGatherer; private final VirtualHostRegistry _virtualHostRegistry; private final LogRecorder _logRecorder; @@ -80,8 +85,14 @@ public class BrokerRecoverer implements ConfiguredObjectRecoverer @Override public Broker create(RecovererProvider recovererProvider, ConfigurationEntry entry, ConfiguredObject... parents) { + Map attributes = entry.getAttributes(); + validateAttributes(attributes); + + Map attributesCopy = new HashMap(attributes); + attributesCopy.put(Broker.MODEL_VERSION, Model.MODEL_VERSION); + StoreConfigurationChangeListener storeChangeListener = new StoreConfigurationChangeListener(entry.getStore()); - BrokerAdapter broker = new BrokerAdapter(entry.getId(), entry.getAttributes(), _statisticsGatherer, _virtualHostRegistry, + BrokerAdapter broker = new BrokerAdapter(entry.getId(), attributesCopy, _statisticsGatherer, _virtualHostRegistry, _logRecorder, _rootMessageLogger, _authenticationProviderFactory, _groupProviderFactory, _accessControlProviderFactory, _portFactory, _taskExecutor, entry.getStore(), _brokerOptions); @@ -117,6 +128,37 @@ public class BrokerRecoverer implements ConfiguredObjectRecoverer return broker; } + private void validateAttributes(Map attributes) + { + String modelVersion = null; + if (attributes.containsKey(Broker.MODEL_VERSION)) + { + modelVersion = MapValueConverter.getStringAttribute(Broker.MODEL_VERSION, attributes, null); + } + + if (modelVersion == null) + { + throw new IllegalConfigurationException("Broker " + Broker.MODEL_VERSION + " must be specified"); + } + + if (!MODEL_VERSION_PATTERN.matcher(modelVersion).matches()) + { + throw new IllegalConfigurationException("Broker " + Broker.MODEL_VERSION + " is specified in incorrect format: " + + modelVersion); + } + + int versionSeparatorPosition = modelVersion.indexOf("."); + String majorVersionPart = modelVersion.substring(0, versionSeparatorPosition); + int majorModelVersion = Integer.parseInt(majorVersionPart); + int minorModelVersion = Integer.parseInt(modelVersion.substring(versionSeparatorPosition + 1)); + + if (majorModelVersion != Model.MODEL_MAJOR_VERSION || minorModelVersion > Model.MODEL_MINOR_VERSION) + { + throw new IllegalConfigurationException("The model version '" + modelVersion + + "' in configuration is incompatible with the broker model version '" + Model.MODEL_VERSION + "'"); + } + } + private void recoverType(RecovererProvider recovererProvider, StoreConfigurationChangeListener storeChangeListener, BrokerAdapter broker, diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/MemoryConfigurationEntryStore.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/MemoryConfigurationEntryStore.java index 24e0e3bbff..2b9c5ad290 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/MemoryConfigurationEntryStore.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/MemoryConfigurationEntryStore.java @@ -66,7 +66,7 @@ public class MemoryConfigurationEntryStore implements ConfigurationEntryStore private static final String ID = "id"; private static final String TYPE = "@type"; - private static final int STORE_VERSION = 1; + static final int STORE_VERSION = 1; private final ObjectMapper _objectMapper; private final Map _entries; @@ -268,6 +268,24 @@ public class MemoryConfigurationEntryStore implements ConfigurationEntryStore { is = url.openStream(); JsonNode node = loadJsonNodes(is, _objectMapper); + + int storeVersion = 0; + JsonNode storeVersionNode = node.get(Broker.STORE_VERSION); + if (storeVersionNode == null || storeVersionNode.isNull()) + { + throw new IllegalConfigurationException("Broker " + Broker.STORE_VERSION + " attribute must be specified"); + } + else + { + storeVersion = storeVersionNode.getIntValue(); + } + + if (storeVersion != STORE_VERSION) + { + throw new IllegalConfigurationException("The data of version " + storeVersion + + " can not be loaded by store of version " + STORE_VERSION); + } + ConfigurationEntry brokerEntry = toEntry(node, Broker.class, _entries); _rootId = brokerEntry.getId(); } diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Model.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Model.java index bccb6b48ee..dab92c50fa 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Model.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Model.java @@ -34,6 +34,7 @@ public class Model */ public static final int MODEL_MAJOR_VERSION = 1; public static final int MODEL_MINOR_VERSION = 0; + public static final String MODEL_VERSION = MODEL_MAJOR_VERSION + "." + MODEL_MINOR_VERSION; private static final Model MODEL_INSTANCE = new Model(); diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java index 3a94cf22f2..adc30eb944 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java @@ -100,6 +100,8 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat put(VIRTUALHOST_STORE_TRANSACTION_IDLE_TIMEOUT_WARN, Long.class); put(VIRTUALHOST_STORE_TRANSACTION_OPEN_TIMEOUT_CLOSE, Long.class); put(VIRTUALHOST_STORE_TRANSACTION_OPEN_TIMEOUT_WARN, Long.class); + put(MODEL_VERSION, String.class); + put(STORE_VERSION, String.class); }}); public static final int DEFAULT_STATISTICS_REPORTING_PERIOD = 0; @@ -775,7 +777,7 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat } else if (MODEL_VERSION.equals(name)) { - return Model.MODEL_MAJOR_VERSION + "." + Model.MODEL_MINOR_VERSION; + return Model.MODEL_VERSION; } else if (STORE_VERSION.equals(name)) { @@ -1132,23 +1134,22 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat Map convertedAttributes = MapValueConverter.convert(attributes, ATTRIBUTE_TYPES); validateAttributes(convertedAttributes); - Collection names = AVAILABLE_ATTRIBUTES; - for (String name : names) - { - if (convertedAttributes.containsKey(name)) - { - Object desired = convertedAttributes.get(name); - Object expected = getAttribute(name); - if (changeAttribute(name, expected, desired)) - { - attributeSet(name, expected, desired); - } - } - } + super.changeAttributes(convertedAttributes); } private void validateAttributes(Map convertedAttributes) { + if (convertedAttributes.containsKey(MODEL_VERSION) && !Model.MODEL_VERSION.equals(convertedAttributes.get(MODEL_VERSION))) + { + throw new IllegalConfigurationException("Cannot change the model version"); + } + + if (convertedAttributes.containsKey(STORE_VERSION) + && !new Integer(_brokerStore.getVersion()).equals(convertedAttributes.get(STORE_VERSION))) + { + throw new IllegalConfigurationException("Cannot change the store version"); + } + String defaultVirtualHost = (String) convertedAttributes.get(DEFAULT_VIRTUAL_HOST); if (defaultVirtualHost != null) { diff --git a/qpid/java/broker/src/main/resources/initial-config.json b/qpid/java/broker/src/main/resources/initial-config.json index e510c34178..02fe942f55 100644 --- a/qpid/java/broker/src/main/resources/initial-config.json +++ b/qpid/java/broker/src/main/resources/initial-config.json @@ -21,6 +21,7 @@ { "name": "Broker", "storeVersion": 1, + "modelVersion": "1.0", "defaultVirtualHost" : "default", "authenticationproviders" : [ { "name" : "passwordFile", diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/BrokerConfigurationStoreCreatorTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/BrokerConfigurationStoreCreatorTest.java index 8fc7d99246..a7772ffd10 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/BrokerConfigurationStoreCreatorTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/BrokerConfigurationStoreCreatorTest.java @@ -31,6 +31,7 @@ import java.util.UUID; import org.apache.qpid.server.BrokerOptions; import org.apache.qpid.server.configuration.store.JsonConfigurationEntryStore; import org.apache.qpid.server.model.Broker; +import org.apache.qpid.server.model.Model; import org.apache.qpid.test.utils.QpidTestCase; import org.apache.qpid.test.utils.TestFileUtils; import org.apache.qpid.util.FileUtils; @@ -104,7 +105,9 @@ public class BrokerConfigurationStoreCreatorTest extends QpidTestCase Map brokerObjectMap = new HashMap(); UUID testBrokerId = UUID.randomUUID(); brokerObjectMap.put(Broker.ID, testBrokerId); - brokerObjectMap.put("name", testBrokerName); + brokerObjectMap.put(Broker.NAME, testBrokerName); + brokerObjectMap.put(Broker.MODEL_VERSION, Model.MODEL_VERSION); + brokerObjectMap.put(Broker.STORE_VERSION, 1); StringWriter sw = new StringWriter(); objectMapper.writeValue(sw, brokerObjectMap); @@ -122,7 +125,7 @@ public class BrokerConfigurationStoreCreatorTest extends QpidTestCase assertEquals("Unexpected root id", testBrokerId, entry.getId()); Map attributes = entry.getAttributes(); assertNotNull("Unexpected attributes: " + attributes, attributes); - assertEquals("Unexpected attributes size: " + attributes.size(), 1, attributes.size()); + assertEquals("Unexpected attributes size: " + attributes.size(), 3, attributes.size()); assertEquals("Unexpected attribute name: " + attributes.get("name"), testBrokerName, attributes.get(Broker.NAME)); Set childrenIds = entry.getChildrenIds(); assertTrue("Unexpected children: " + childrenIds, childrenIds.isEmpty()); diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java index 758eb62809..589f0fc5af 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java @@ -36,6 +36,7 @@ import junit.framework.TestCase; import org.apache.qpid.server.BrokerOptions; import org.apache.qpid.server.configuration.ConfigurationEntry; import org.apache.qpid.server.configuration.ConfiguredObjectRecoverer; +import org.apache.qpid.server.configuration.IllegalConfigurationException; import org.apache.qpid.server.configuration.RecovererProvider; import org.apache.qpid.server.logging.LogRecorder; import org.apache.qpid.server.logging.RootMessageLogger; @@ -44,6 +45,7 @@ import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.GroupProvider; import org.apache.qpid.server.model.KeyStore; +import org.apache.qpid.server.model.Model; import org.apache.qpid.server.model.Plugin; import org.apache.qpid.server.model.Port; import org.apache.qpid.server.model.TrustStore; @@ -77,6 +79,7 @@ public class BrokerRecovererTest extends TestCase mock(StatisticsGatherer.class), mock(VirtualHostRegistry.class), mock(LogRecorder.class), mock(RootMessageLogger.class), mock(TaskExecutor.class), mock(BrokerOptions.class)); when(_brokerEntry.getId()).thenReturn(_brokerId); when(_brokerEntry.getChildren()).thenReturn(_brokerEntryChildren); + when(_brokerEntry.getAttributes()).thenReturn(Collections.singletonMap(Broker.MODEL_VERSION, Model.MODEL_VERSION)); //Add a base AuthenticationProvider for all tests _authenticationProvider1 = mock(AuthenticationProvider.class); @@ -104,6 +107,7 @@ public class BrokerRecovererTest extends TestCase attributes.put(Broker.CONNECTION_HEART_BEAT_DELAY, 2000); attributes.put(Broker.STATISTICS_REPORTING_PERIOD, 4000); attributes.put(Broker.STATISTICS_REPORTING_RESET_ENABLED, true); + attributes.put(Broker.MODEL_VERSION, Model.MODEL_VERSION); Map entryAttributes = new HashMap(); for (Map.Entry attribute : attributes.entrySet()) @@ -191,9 +195,6 @@ public class BrokerRecovererTest extends TestCase ConfigurationEntry authenticationProviderEntry2 = mock(ConfigurationEntry.class); _brokerEntryChildren.put(AuthenticationProvider.class.getSimpleName(), Arrays.asList(_authenticationProviderEntry1, authenticationProviderEntry2)); - Map brokerAtttributes = new HashMap(); - when(_brokerEntry.getAttributes()).thenReturn(brokerAtttributes); - //Add a couple ports ConfigurationEntry portEntry1 = mock(ConfigurationEntry.class); Port port1 = mock(Port.class); @@ -288,6 +289,69 @@ public class BrokerRecovererTest extends TestCase assertEquals(Collections.singleton(trustStore), new HashSet(broker.getChildren(TrustStore.class))); } + public void testModelVersionValidationForIncompatibleMajorVersion() throws Exception + { + Map brokerAttributes = new HashMap(); + String[] incompatibleVersions = {Integer.MAX_VALUE + "." + 0, "0.0"}; + for (String incompatibleVersion : incompatibleVersions) + { + brokerAttributes.put(Broker.MODEL_VERSION, incompatibleVersion); + when(_brokerEntry.getAttributes()).thenReturn(brokerAttributes); + + try + { + _brokerRecoverer.create(null, _brokerEntry); + fail("The broker creation should fail due to unsupported model version"); + } + catch (IllegalConfigurationException e) + { + assertEquals("The model version '" + incompatibleVersion + + "' in configuration is incompatible with the broker model version '" + Model.MODEL_VERSION + "'", e.getMessage()); + } + } + } + + + public void testModelVersionValidationForIncompatibleMinorVersion() throws Exception + { + Map brokerAttributes = new HashMap(); + String incompatibleVersion = Model.MODEL_MAJOR_VERSION + "." + Integer.MAX_VALUE; + brokerAttributes.put(Broker.MODEL_VERSION, incompatibleVersion); + when(_brokerEntry.getAttributes()).thenReturn(brokerAttributes); + + try + { + _brokerRecoverer.create(null, _brokerEntry); + fail("The broker creation should fail due to unsupported model version"); + } + catch (IllegalConfigurationException e) + { + assertEquals("The model version '" + incompatibleVersion + + "' in configuration is incompatible with the broker model version '" + Model.MODEL_VERSION + "'", e.getMessage()); + } + } + + public void testIncorrectModelVersion() throws Exception + { + Map brokerAttributes = new HashMap(); + String[] versions = { Integer.MAX_VALUE + "_" + 0, "", null }; + for (String modelVersion : versions) + { + brokerAttributes.put(Broker.MODEL_VERSION, modelVersion); + when(_brokerEntry.getAttributes()).thenReturn(brokerAttributes); + + try + { + _brokerRecoverer.create(null, _brokerEntry); + fail("The broker creation should fail due to unsupported model version"); + } + catch (IllegalConfigurationException e) + { + // pass + } + } + } + private String convertToString(Object attributeValue) { return String.valueOf(attributeValue); diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStoreTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStoreTest.java index 9ee93a345f..f328211253 100644 --- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStoreTest.java +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/store/JsonConfigurationEntryStoreTest.java @@ -48,11 +48,20 @@ public class JsonConfigurationEntryStoreTest extends ConfigurationEntryStoreTest private File createStoreFile(UUID brokerId, Map brokerAttributes) throws IOException, JsonGenerationException, JsonMappingException + { + return createStoreFile(brokerId, brokerAttributes, true); + } + + private File createStoreFile(UUID brokerId, Map brokerAttributes, boolean setVersion) throws IOException, + JsonGenerationException, JsonMappingException { Map brokerObjectMap = new HashMap(); brokerObjectMap.put(Broker.ID, brokerId); - brokerObjectMap.put("@type", Broker.class.getSimpleName()); - brokerObjectMap.put("storeVersion", 1); + if (setVersion) + { + brokerObjectMap.put(Broker.STORE_VERSION, MemoryConfigurationEntryStore.STORE_VERSION); + } + brokerObjectMap.put(Broker.NAME, getTestName()); brokerObjectMap.putAll(brokerAttributes); StringWriter sw = new StringWriter(); @@ -124,7 +133,6 @@ public class JsonConfigurationEntryStoreTest extends ConfigurationEntryStoreTest { UUID brokerId = UUID.randomUUID(); Map brokerAttributes = new HashMap(); - brokerAttributes.put(Broker.NAME, getTestName()); File initialStoreFile = createStoreFile(brokerId, brokerAttributes); JsonConfigurationEntryStore initialStore = new JsonConfigurationEntryStore(initialStoreFile.getAbsolutePath(), null, false, Collections.emptyMap()); @@ -151,4 +159,58 @@ public class JsonConfigurationEntryStoreTest extends ConfigurationEntryStoreTest { assertEquals("Unexpected type", "json", getStore().getType()); } + + public void testUnsupportedStoreVersion() throws Exception + { + UUID brokerId = UUID.randomUUID(); + Map brokerAttributes = new HashMap(); + int[] storeVersions = {Integer.MAX_VALUE, 0}; + for (int storeVersion : storeVersions) + { + brokerAttributes.put(Broker.STORE_VERSION, storeVersion); + File storeFile = null; + try + { + storeFile = createStoreFile(brokerId, brokerAttributes); + new JsonConfigurationEntryStore(storeFile.getAbsolutePath(), null, false, Collections.emptyMap()); + fail("The store creation should fail due to unsupported store version"); + } + catch (IllegalConfigurationException e) + { + assertEquals("The data of version " + storeVersion + + " can not be loaded by store of version " + MemoryConfigurationEntryStore.STORE_VERSION, e.getMessage()); + } + finally + { + if (storeFile != null) + { + storeFile.delete(); + } + } + } + } + + public void testStoreVersionNotSpecified() throws Exception + { + UUID brokerId = UUID.randomUUID(); + Map brokerAttributes = new HashMap(); + File storeFile = null; + try + { + storeFile = createStoreFile(brokerId, brokerAttributes, false); + new JsonConfigurationEntryStore(storeFile.getAbsolutePath(), null, false, Collections.emptyMap()); + fail("The store creation should fail due to unspecified store version"); + } + catch (IllegalConfigurationException e) + { + assertEquals("Broker " + Broker.STORE_VERSION + " attribute must be specified", e.getMessage()); + } + finally + { + if (storeFile != null) + { + storeFile.delete(); + } + } + } } diff --git a/qpid/java/systests/etc/config-systests.json b/qpid/java/systests/etc/config-systests.json index c47744c47c..12a8a5c5a6 100644 --- a/qpid/java/systests/etc/config-systests.json +++ b/qpid/java/systests/etc/config-systests.json @@ -21,6 +21,8 @@ { "name": "Broker", "defaultVirtualHost" : "test", + "storeVersion": 1, + "modelVersion": "1.0", "authenticationproviders" : [ { "name" : "plain", "type" : "PlainPasswordFile", @@ -59,14 +61,4 @@ "name" : "test", "configPath" : "${broker.virtualhosts-config}" } ] - /* -, - "plugins" : [ { - "pluginType" : "MANAGEMENT-HTTP", - "name" : "httpManagement" - }, { - "pluginType" : "MANAGEMENT-JMX", - "name" : "jmxManagement" - } ] - */ } -- cgit v1.2.1