Author: julien(a)jboss.com Date: 2007-02-01 19:28:54 -0500 (Thu, 01 Feb 2007) New Revision: 6144 Modified: trunk/core/build.xml trunk/core/src/main/org/jboss/portal/core/impl/model/instance/PersistentInstanceContainer.java trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java trunk/core/src/resources/portal-core-sar/portal-aop.xml Log: JBPORTAL-1222 : Incorrect behaviour of InstanceContainer.createDefinition in case a definition already exists Modified: trunk/core/build.xml =================================================================== --- trunk/core/build.xml 2007-02-01 23:45:02 UTC (rev 6143) +++ trunk/core/build.xml 2007-02-02 00:28:54 UTC (rev 6144) @@ -549,7 +549,6 @@ </x-sysproperty> <x-test> -<!-- <zest todir="${test.reports}" name="org.jboss.portal.test.core.model.portal.PortalObjectContainerTestCase" outfile="TEST-PortalObjectContainerTestCase"> <parameter name="CacheNaturalId" value="true"/> @@ -578,23 +577,18 @@ <parameter name="CloneOnCreate" value="false"/> <parameter name="CacheNaturalId" value="true"/> </zest> ---> <zest todir="${test.reports}" name="org.jboss.portal.test.core.state.ProducerTestCase" outfile="TEST-ProducerTestCase"> </zest> -<!-- <zest todir="${test.reports}" name="org.jboss.portal.test.core.state.RegistrationPersistenceManagerTestCase" outfile="TEST-RegistrationPersistenceManagerTestCase"> </zest> ---> -<!-- <test todir="${test.reports}" name="org.jboss.portal.test.core.deployment.JBossApplicationMetaDataFactoryTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.core.model.portal.PortalObjectPermissionTestCase"/> <test todir="${test.reports}" name="org.jboss.portal.test.core.model.portal.PortalObjectIdTestCase"/> ---> </x-test> <x-classpath> <pathelement location="${build.lib}/portal-core-lib.jar"/> Modified: trunk/core/src/main/org/jboss/portal/core/impl/model/instance/PersistentInstanceContainer.java =================================================================== --- trunk/core/src/main/org/jboss/portal/core/impl/model/instance/PersistentInstanceContainer.java 2007-02-01 23:45:02 UTC (rev 6143) +++ trunk/core/src/main/org/jboss/portal/core/impl/model/instance/PersistentInstanceContainer.java 2007-02-02 00:28:54 UTC (rev 6144) @@ -252,11 +252,16 @@ public InstanceDefinition createDefinition(String id, String portletId) throws DuplicateInstanceException, IllegalArgumentException, PortletInvokerException { - return createDefinition(id, portletId, cloneOnCreate); + return internalCreateDefinition(id, portletId, cloneOnCreate); } public InstanceDefinition createDefinition(String id, String portletId, boolean clone) throws DuplicateInstanceException, PortletInvokerException { + return internalCreateDefinition(id, portletId, clone); + } + + private InstanceDefinition internalCreateDefinition(String id, String portletId, boolean clone) throws DuplicateInstanceException, PortletInvokerException + { if (id == null) { throw new IllegalArgumentException("id cannot be null"); @@ -269,23 +274,30 @@ log.debug("Creating instance " + id + " of portlet " + portletId); } - // + // Create the portlet context we'll use PortletContext portletContext = PortletContext.createPortletContext(portletId); // Check that the portlet exist before creating an instance of it portletInvoker.getPortlet(portletContext); + // Prevent duplicate + Session session = ctx.getCurrentSession(); + if (lookup(session, id) != null) + { + throw new DuplicateInstanceException("An instance with id " + id + " already exist"); + } + // InstanceDefinitionImpl instance; try { - // Create the persistent instance which may raise a constraint violation exception if it already exist instance = new InstanceDefinitionImpl(ctx, id, portletId); - Session session = ctx.getCurrentSession(); session.persist(instance); } catch (ConstraintViolationException e) { + // May raise a constraint violation exception if it is has been inserted between the lookup + // and the insert and the isolation level is not serializable throw new DuplicateInstanceException("An instance with id " + id + " already exist"); } @@ -308,7 +320,6 @@ instance.setModifiable(true); // - Session session = ctx.getCurrentSession(); session.update(instance); } Modified: trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java =================================================================== --- trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java 2007-02-01 23:45:02 UTC (rev 6143) +++ trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java 2007-02-02 00:28:54 UTC (rev 6144) @@ -50,6 +50,7 @@ import org.jboss.portal.core.impl.portlet.state.PersistentPortletStatePersistenceManager; import org.jboss.portal.core.model.instance.Instance; import org.jboss.portal.core.model.instance.InstanceDefinition; +import org.jboss.portal.core.model.instance.DuplicateInstanceException; import org.jboss.portal.portlet.NoSuchPortletException; import org.jboss.portal.portlet.Portlet; import org.jboss.portal.portlet.PortletContext; @@ -763,6 +764,7 @@ { portletContainer.addPortlet("MyPortlet", new TestPortletSupport()); + // TransactionAssert.beginTransaction(); instanceContainer.createDefinition("MyInstance", "MyPortlet"); TransactionAssert.commitTransaction(); @@ -786,6 +788,28 @@ TransactionAssert.commitTransaction(); } + public void testCreateDefinitionThrowsDuplicateInstanceException() throws PortletInvokerException, DuplicateInstanceException + { + portletContainer.addPortlet("MyPortlet", new TestPortletSupport()); + + // + TransactionAssert.beginTransaction(); + instanceContainer.createDefinition("MyInstance", "MyPortlet"); + TransactionAssert.commitTransaction(); + + TransactionAssert.beginTransaction(); + try + { + instanceContainer.createDefinition("MyInstance", "MyPortlet"); + fail("Was expecting a NoSuchPortletException"); + } + catch (DuplicateInstanceException expected) + { + // Duplicate instance exception does not mark the transaction as rollback + TransactionAssert.commitTransaction(); + } + } + // //// /**Tests the authorization of portal objects */ //// public void testInstanceAuthorization() throws Exception Modified: trunk/core/src/resources/portal-core-sar/portal-aop.xml =================================================================== --- trunk/core/src/resources/portal-core-sar/portal-aop.xml 2007-02-01 23:45:02 UTC (rev 6143) +++ trunk/core/src/resources/portal-core-sar/portal-aop.xml 2007-02-02 00:28:54 UTC (rev 6144) @@ -100,6 +100,7 @@ </method> <method name="createDefinition"> <trans-attribute>Required</trans-attribute> + <application-exceptions>org.jboss.portal.core.model.instance.DuplicateInstanceException</application-exceptions> </method> <method name="destroyDefinition"> <trans-attribute>Required</trans-attribute>