From 7955eb623a70a36492d76010824b54652ee25332 Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Mon, 30 Jan 2006 01:45:32 +0000 Subject: [PATCH] Fixed AR-359: Property values never persisted as Serializable unless absolutely necessary. Bumped schema version up to 4. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@2246 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../messages/patch-service.properties | 3 +- .../alfresco/patch/patch-services-context.xml | 13 +- config/alfresco/version.properties | 2 +- .../repo/admin/patch/impl/GuestUserPatch.java | 7 + .../impl/NodePropertySerializablePatch.java | 135 ++++++++++++++++++ .../alfresco/repo/domain/PropertyValue.java | 99 +++++++++++-- .../repo/domain/hibernate/Node.hbm.xml | 10 ++ .../repo/node/BaseNodeServiceTest.java | 2 +- 8 files changed, 257 insertions(+), 14 deletions(-) create mode 100644 source/java/org/alfresco/repo/admin/patch/impl/NodePropertySerializablePatch.java diff --git a/config/alfresco/messages/patch-service.properties b/config/alfresco/messages/patch-service.properties index acbd9d1c2f..46bb07ce49 100644 --- a/config/alfresco/messages/patch-service.properties +++ b/config/alfresco/messages/patch-service.properties @@ -16,4 +16,5 @@ patch.updatePermissionData.result=Created the following permission reference nam patch.guestUser.description=Add the guest user, guest home space; and fix permissions on company home, guest home and guest person. patch.guestUser.result=Added guest user and fixed permissions. - +patch.fixNodeSerializableValues.description=Ensure that property values are not stored as Serializable if at all possible +patch.fixNodeSerializableValues.result=Fixed {0} node property serialized values diff --git a/config/alfresco/patch/patch-services-context.xml b/config/alfresco/patch/patch-services-context.xml index 8e739cf6ed..ba3b4c2eca 100644 --- a/config/alfresco/patch/patch-services-context.xml +++ b/config/alfresco/patch/patch-services-context.xml @@ -106,7 +106,6 @@ - patch.guestUser patch.guestUser.description @@ -132,7 +131,17 @@ - + + + patch.fixNodeSerializableValues + patch.fixNodeSerializableValues.description + 0 + 3 + 4 + + + + diff --git a/config/alfresco/version.properties b/config/alfresco/version.properties index 0306053284..7041232b36 100644 --- a/config/alfresco/version.properties +++ b/config/alfresco/version.properties @@ -15,4 +15,4 @@ version.edition=Open Source # Schema number -version.schema=3 +version.schema=4 diff --git a/source/java/org/alfresco/repo/admin/patch/impl/GuestUserPatch.java b/source/java/org/alfresco/repo/admin/patch/impl/GuestUserPatch.java index a457dec1e8..43e82e1c12 100644 --- a/source/java/org/alfresco/repo/admin/patch/impl/GuestUserPatch.java +++ b/source/java/org/alfresco/repo/admin/patch/impl/GuestUserPatch.java @@ -36,6 +36,13 @@ import org.alfresco.service.cmr.security.PersonService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; +/** + * Ensures that the guest user homespace exists.
+ * A guest user homespace is now created during bootstrap. It is required for guest user + * access, but in older databases will not exist. + * + * @author Andy Hind + */ public class GuestUserPatch extends AbstractPatch { private static final String MSG_SUCCESS = "patch.guestUser.result"; diff --git a/source/java/org/alfresco/repo/admin/patch/impl/NodePropertySerializablePatch.java b/source/java/org/alfresco/repo/admin/patch/impl/NodePropertySerializablePatch.java new file mode 100644 index 0000000000..828346c2d0 --- /dev/null +++ b/source/java/org/alfresco/repo/admin/patch/impl/NodePropertySerializablePatch.java @@ -0,0 +1,135 @@ +/* + * Copyright (C) 2005 Alfresco, Inc. + * + * Licensed under the Mozilla Public License version 1.1 + * with a permitted attribution clause. You may obtain a + * copy of the License at + * + * http://www.alfresco.org/legal/license.txt + * + * 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.alfresco.repo.admin.patch.impl; + +import java.io.Serializable; +import java.util.Iterator; +import java.util.Map; + +import org.alfresco.i18n.I18NUtil; +import org.alfresco.repo.admin.patch.AbstractPatch; +import org.alfresco.repo.domain.Node; +import org.alfresco.repo.domain.PropertyValue; +import org.alfresco.service.cmr.dictionary.DataTypeDefinition; +import org.alfresco.service.namespace.QName; +import org.hibernate.Query; +import org.hibernate.Session; +import org.hibernate.SessionFactory; +import org.springframework.orm.hibernate3.HibernateCallback; +import org.springframework.orm.hibernate3.support.HibernateDaoSupport; + +/** + * Certain content models make extensive use of the d:any datatype, which has led + * to storage of simple types as serialized instances. + * This patch ensures that all previously serializable values are stored in their + * more native form in the database.
+ * e.g. If a property was d:any and a string was written ("ABC"), + * then the value was stored in serializable_value. Instead, the newer code stores + * the value in string_value. None of the retrieval code is affected, but the values + * are made visible to queries, in addition to reducing the size of the node_properties + * table. This patch ensures that previously-stored values are changed to conform + * to the new storage mechanism. + *

+ * JIRA: {@link http://www.alfresco.org/jira/browse/AR-359 AR-359} + * + * @see org.alfresco.repo.domain.PropertyValue + * + * @author Derek Hulley + */ +public class NodePropertySerializablePatch extends AbstractPatch +{ + private static final String MSG_SUCCESS = "patch.fixNodeSerializableValues.result"; + + private HibernateHelper helper; + + public NodePropertySerializablePatch() + { + helper = new HibernateHelper(); + } + + public void setSessionFactory(SessionFactory sessionFactory) + { + this.helper.setSessionFactory(sessionFactory); + } + + @Override + protected String applyInternal() throws Exception + { + int updatedEntries = helper.fixSerializableProperties(); + + // build the result message + String msg = I18NUtil.getMessage(MSG_SUCCESS, updatedEntries); + // done + return msg; + } + + private static class HibernateHelper extends HibernateDaoSupport + { + private static final String QUERY_GET_NODES = "node.patch.GetNodesWithPersistedSerializableProperties"; + + public int fixSerializableProperties() + { + HibernateCallback callback = new HibernateCallback() + { + @SuppressWarnings("unchecked") + public Object doInHibernate(Session session) + { + Query query = session.getNamedQuery(HibernateHelper.QUERY_GET_NODES); + Iterator iterator = query.iterate(); + // iterate over the nodes + int count = 0; + while (iterator.hasNext()) + { + Node node = iterator.next(); + // retrieve the node properties + Map properties = node.getProperties(); + // check each property + for (Map.Entry entry : properties.entrySet()) + { + PropertyValue propertyValue = entry.getValue(); + if (propertyValue.getSerializableValue() == null) + { + // the property was not persisted as a serializable - nothing to do + continue; + } + else if (propertyValue.isMultiValued()) + { + // this is a persisted collection - nothing to do + continue; + } + else if (!"SERIALIZABLE".equals(propertyValue.getActualType())) + { + // only handle actual types that were pushed in as any old type + continue; + } + // make sure that this value is persisted correctly + Serializable value = propertyValue.getSerializableValue(); + // put it back + PropertyValue newPropertyValue = new PropertyValue(DataTypeDefinition.ANY, value); + entry.setValue(newPropertyValue); + count++; + } + } + return new Integer(count); + } + }; + Integer updateCount = (Integer) getHibernateTemplate().execute(callback); + // done + return updateCount.intValue(); + } + } +} diff --git a/source/java/org/alfresco/repo/domain/PropertyValue.java b/source/java/org/alfresco/repo/domain/PropertyValue.java index 0b28cea323..56706871ed 100644 --- a/source/java/org/alfresco/repo/domain/PropertyValue.java +++ b/source/java/org/alfresco/repo/domain/PropertyValue.java @@ -69,7 +69,7 @@ public class PropertyValue implements Cloneable, Serializable INTEGER { @Override - protected ValueType getPersistedType() + protected ValueType getPersistedType(Serializable value) { return ValueType.LONG; } @@ -106,6 +106,23 @@ public class PropertyValue implements Cloneable, Serializable }, STRING { + /** + * Strings longer than the maximum of 1024 characters will be serialized. + */ + @Override + protected ValueType getPersistedType(Serializable value) + { + if (value instanceof String) + { + String valueStr = (String) value; + if (valueStr.length() > 1024) + { + return ValueType.SERIALIZABLE; + } + } + return ValueType.STRING; + } + @Override Serializable convert(Serializable value) { @@ -115,7 +132,7 @@ public class PropertyValue implements Cloneable, Serializable DATE { @Override - protected ValueType getPersistedType() + protected ValueType getPersistedType(Serializable value) { return ValueType.STRING; } @@ -128,6 +145,66 @@ public class PropertyValue implements Cloneable, Serializable }, SERIALIZABLE { + /** + * As Serializable persistence is the last resort, being both + * more verbose as well as unusable in queries, the value is used to + * determine a more appropriate persistent type. This method defers to + * the other, more basic types' version of this method to fulfill the request. + */ + @Override + protected ValueType getPersistedType(Serializable value) + { + if (value == null) + { + throw new IllegalArgumentException("Persisted type cannot be determined by null value"); + } + // check the value to determine the most appropriate type to defer to + if (value instanceof Boolean) + { + return ValueType.BOOLEAN.getPersistedType(value); + } + else if ((value instanceof Integer) || (value instanceof Long)) + { + return ValueType.LONG.getPersistedType(value); + } + else if (value instanceof Float) + { + return ValueType.FLOAT.getPersistedType(value); + } + else if (value instanceof Double) + { + return ValueType.DOUBLE.getPersistedType(value); + } + else if (value instanceof String) + { + return ValueType.STRING.getPersistedType(value); + } + else if (value instanceof Date) + { + return ValueType.DATE.getPersistedType(value); + } + else if (value instanceof ContentData) + { + return ValueType.CONTENT.getPersistedType(value); + } + else if (value instanceof NodeRef) + { + return ValueType.NODEREF.getPersistedType(value); + } + else if (value instanceof QName) + { + return ValueType.QNAME.getPersistedType(value); + } + else if (value instanceof Path) + { + return ValueType.PATH.getPersistedType(value); + } + else + { + return this; + } + } + @Override Serializable convert(Serializable value) { @@ -137,7 +214,7 @@ public class PropertyValue implements Cloneable, Serializable CONTENT { @Override - protected ValueType getPersistedType() + protected ValueType getPersistedType(Serializable value) { return ValueType.STRING; } @@ -151,7 +228,7 @@ public class PropertyValue implements Cloneable, Serializable NODEREF { @Override - protected ValueType getPersistedType() + protected ValueType getPersistedType(Serializable value) { return ValueType.STRING; } @@ -165,7 +242,7 @@ public class PropertyValue implements Cloneable, Serializable QNAME { @Override - protected ValueType getPersistedType() + protected ValueType getPersistedType(Serializable value) { return ValueType.STRING; } @@ -179,7 +256,7 @@ public class PropertyValue implements Cloneable, Serializable PATH { @Override - protected ValueType getPersistedType() + protected ValueType getPersistedType(Serializable value) { return ValueType.SERIALIZABLE; } @@ -191,8 +268,12 @@ public class PropertyValue implements Cloneable, Serializable } }; - /** override if the type gets persisted in a different format */ - protected ValueType getPersistedType() + /** + * Override if the type gets persisted in a different format + * + * @param value the actual value that is to be persisted. May not be null. + */ + protected ValueType getPersistedType(Serializable value) { return this; } @@ -302,7 +383,7 @@ public class PropertyValue implements Cloneable, Serializable { // get the persisted type ValueType valueType = makeValueType(typeQName); - ValueType persistedValueType = valueType.getPersistedType(); + ValueType persistedValueType = valueType.getPersistedType(value); // convert to the persistent type value = persistedValueType.convert(value); setPersistedValue(persistedValueType, value); diff --git a/source/java/org/alfresco/repo/domain/hibernate/Node.hbm.xml b/source/java/org/alfresco/repo/domain/hibernate/Node.hbm.xml index 0a43cd27da..452d5f6051 100644 --- a/source/java/org/alfresco/repo/domain/hibernate/Node.hbm.xml +++ b/source/java/org/alfresco/repo/domain/hibernate/Node.hbm.xml @@ -330,4 +330,14 @@ status.changeTxnId = :changeTxnId + + select distinct + node + from + org.alfresco.repo.domain.hibernate.NodeImpl as node + where + node.properties.serializableValue is not null and + node.properties.multiValued = false + + diff --git a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java index 97b8ee1c4a..a22d619c46 100644 --- a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java @@ -927,7 +927,7 @@ public abstract class BaseNodeServiceTest extends BaseSpringTest rootNodeRef, ASSOC_TYPE_QNAME_TEST_CHILDREN, QName.createQName("pathA"), - ContentModel.TYPE_CONTAINER, + TYPE_QNAME_TEST_MANY_PROPERTIES, properties).getChildRef(); // persist