Author: chris.laprun(a)jboss.com Date: 2010-01-11 11:53:40 -0500 (Mon, 11 Jan 2010) New Revision: 1224 Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/EndpointConfigurationInfo.java components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationProperty.java components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/EndpointConfigurationInfoTestCase.java components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationInfoTestCase.java components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationPropertyTestCase.java Log: - Have EndpointConfigurationInfo.refresh return whether a refresh actually happened. - Make RegistrationProperty validity be computed based on status instead of relying on a mix of saved state and status as was previously needed for backward compatibility. As a result, removed setInvalid method: setStatus should be used instead. - Check that endpoint refreshed before saving changes so that we don't save stuff unnecessarily. - Made it possible to set a registration value to null in RegistrationInfo. - Fixed improper check of null listener in RegistrationProperty. - Updated test cases. Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/EndpointConfigurationInfo.java =================================================================== --- components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/EndpointConfigurationInfo.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/EndpointConfigurationInfo.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,6 +1,6 @@ /* * JBoss, a division of Red Hat - * Copyright 2009, Red Hat Middleware, LLC, and individual + * Copyright 2010, Red Hat Middleware, LLC, and individual * contributors as indicated by the @authors tag. See the * copyright.txt in the distribution for a full listing of * individual contributors. @@ -154,20 +154,19 @@ return result; } - public void refresh() throws InvokerUnavailableException + public boolean refresh() throws InvokerUnavailableException { - if (isRefreshNeeded()) - { - forceRefresh(); - } + return isRefreshNeeded() && forceRefresh(); } - void forceRefresh() throws InvokerUnavailableException + boolean forceRefresh() throws InvokerUnavailableException { getService(WSRPV1ServiceDescriptionPortType.class, serviceFactory); getService(WSRPV1MarkupPortType.class, serviceFactory); getService(WSRPV1PortletManagementPortType.class, serviceFactory); getService(WSRPV1RegistrationPortType.class, serviceFactory); + + return true; } public String getRemoteHostAddress() Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java =================================================================== --- components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/ProducerInfo.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,6 +1,6 @@ /* * JBoss, a division of Red Hat - * Copyright 2009, Red Hat Middleware, LLC, and individual + * Copyright 2010, Red Hat Middleware, LLC, and individual * contributors as indicated by the @authors tag. See the * copyright.txt in the distribution for a full listing of * individual contributors. @@ -367,23 +367,27 @@ RefreshResult result = new RefreshResult(); // success by default! + boolean didJustRefresh = false; try { - persistentEndpointInfo.refresh(); + didJustRefresh = persistentEndpointInfo.refresh(); } catch (InvokerUnavailableException e) { log.debug("Couldn't refresh endpoint information, attempting a second time: " + e); // try again as refresh on a failed service factory will fail without attempting the refresh - persistentEndpointInfo.forceRefresh(); + didJustRefresh = persistentEndpointInfo.forceRefresh(); // todo: should we fail fast here? // throw new PortletInvokerException("Couldn't refresh endpoint information: " + e.getLocalizedMessage()); } finally { - // save changes to endpoint - registry.updateProducerInfo(this); + // save changes to endpoint only if we just refreshed, otherwise unneeded + if (didJustRefresh) + { + registry.updateProducerInfo(this); + } } // get the service description from the producer Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java =================================================================== --- components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationInfo.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,6 +1,6 @@ /* * JBoss, a division of Red Hat - * Copyright 2009, Red Hat Middleware, LLC, and individual + * Copyright 2010, Red Hat Middleware, LLC, and individual * contributors as indicated by the @authors tag. See the * copyright.txt in the distribution for a full listing of * individual contributors. @@ -305,7 +305,6 @@ public RegistrationProperty setRegistrationPropertyValue(String name, String value) { ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(name, "registration property name", "RegistrationInfo.setRegistrationPropertyValue"); - ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(value, "registration property value", "RegistrationInfo.setRegistrationPropertyValue"); RegistrationProperty prop = getOrCreateRegistrationPropertiesMap(true).get(name); if (prop != null) @@ -574,11 +573,11 @@ { // mark the prop as invalid RegistrationProperty prop = properties.get(name); - prop.setInvalid(Boolean.TRUE, RegistrationProperty.Status.INVALID_VALUE); + prop.setStatus(RegistrationProperty.Status.INVALID_VALUE); // do the same in the result prop = result.getRegistrationProperties().get(name); - prop.setInvalid(Boolean.TRUE, RegistrationProperty.Status.INEXISTENT); + prop.setStatus(RegistrationProperty.Status.INEXISTENT); } else { @@ -624,7 +623,7 @@ RegistrationPropertyDescription desc = WSRPUtils.convertToRegistrationPropertyDescription(description); RegistrationProperty prop = new RegistrationProperty(name, null, WSRPUtils.toString(desc.getLang()), this); prop.setDescription(desc); - prop.setInvalid(Boolean.TRUE, RegistrationProperty.Status.MISSING_VALUE); + prop.setStatus(RegistrationProperty.Status.MISSING_VALUE); result.put(name, prop); } @@ -664,7 +663,7 @@ for (Object o : persistentRegistrationProperties.values()) { RegistrationProperty prop = (RegistrationProperty)o; - prop.setInvalid(Boolean.FALSE, RegistrationProperty.Status.VALID); + prop.setStatus(RegistrationProperty.Status.VALID); } } Modified: components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationProperty.java =================================================================== --- components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationProperty.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/main/java/org/gatein/wsrp/consumer/RegistrationProperty.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,6 +1,6 @@ /* * JBoss, a division of Red Hat - * Copyright 2009, Red Hat Middleware, LLC, and individual + * Copyright 2010, Red Hat Middleware, LLC, and individual * contributors as indicated by the @authors tag. See the * copyright.txt in the distribution for a full listing of * individual contributors. @@ -24,9 +24,10 @@ package org.gatein.wsrp.consumer; import org.gatein.common.util.ParameterValidation; -import static org.gatein.wsrp.consumer.RegistrationProperty.Status.*; import org.gatein.wsrp.registration.RegistrationPropertyDescription; +import static org.gatein.wsrp.consumer.RegistrationProperty.Status.*; + /** * @author <a href="mailto:chris.laprun@jboss.com">Chris Laprun</a> * @version $Revision: 12019 $ @@ -36,13 +37,12 @@ { private Long persistentId; private RegistrationPropertyDescription persistentDescription; - private Boolean persistentInvalid; private String persistentLang; private String persistentName; private String persistentValue; + private Status status; private transient PropertyChangeListener listener; - private transient Status status; public int compareTo(RegistrationProperty o) { @@ -79,7 +79,7 @@ { ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(name, "Name", "RegistrationProperty"); ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(lang, "Lang", "RegistrationProperty"); - ParameterValidation.throwIllegalArgExceptionIfNullOrEmpty(lang, "listener", "RegistrationProperty"); + ParameterValidation.throwIllegalArgExceptionIfNull(listener, "listener"); this.persistentName = name; this.persistentLang = lang; this.listener = listener; @@ -160,37 +160,21 @@ public Boolean isInvalid() { - return persistentInvalid; - } - - void setInvalid(Boolean invalid) - { - this.persistentInvalid = invalid; - } - - public boolean isDeterminedInvalid() - { - return persistentInvalid != null && persistentInvalid && !UNCHECKED_VALUE.equals(getStatus()); - } - - public void setInvalid(Boolean invalid, Status status) - { - this.persistentInvalid = invalid; - if (!invalid) + if (UNCHECKED_VALUE.equals(status)) { - this.status = VALID; + return null; } else { - if (status == null || VALID.equals(status)) - { - throw new IllegalArgumentException("Invalid status: " + status + " for an invalid property!"); - } - - this.status = status; + return !VALID.equals(status); } } + public boolean isDeterminedInvalid() + { + return !VALID.equals(status) && !UNCHECKED_VALUE.equals(status); + } + public void setValue(String stringValue) { // only change the value if it's not the same as the old one @@ -198,7 +182,6 @@ { String oldValue = persistentValue; persistentValue = stringValue; - persistentInvalid = null; if (persistentValue == null) { status = MISSING_VALUE; Modified: components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/EndpointConfigurationInfoTestCase.java =================================================================== --- components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/EndpointConfigurationInfoTestCase.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/EndpointConfigurationInfoTestCase.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,24 +1,25 @@ /* -* JBoss, a division of Red Hat -* Copyright 2006, Red Hat Middleware, LLC, and individual contributors as indicated -* by the @authors tag. See the copyright.txt in the distribution for a -* full listing of individual contributors. -* -* This is free software; you can redistribute it and/or modify it -* under the terms of the GNU Lesser General Public License as -* published by the Free Software Foundation; either version 2.1 of -* the License, or (at your option) any later version. -* -* This software is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -* Lesser General Public License for more details. -* -* You should have received a copy of the GNU Lesser General Public -* License along with this software; if not, write to the Free -* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA -* 02110-1301 USA, or see the FSF site: http://www.fsf.org. -*/ + * JBoss, a division of Red Hat + * Copyright 2010, Red Hat Middleware, LLC, and individual + * contributors as indicated by the @authors tag. See the + * copyright.txt in the distribution for a full listing of + * individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ package org.gatein.wsrp.consumer; @@ -66,7 +67,7 @@ String bea = "http://wsrp.bea.com:7001/producer/producer?WSDL"; info.setWsdlDefinitionURL(bea); - info.refresh(); + assertTrue(info.refresh()); assertFalse(info.isRefreshNeeded()); assertTrue(info.isAvailable()); } Modified: components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationInfoTestCase.java =================================================================== --- components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationInfoTestCase.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationInfoTestCase.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,25 +1,25 @@ -/****************************************************************************** - * JBoss, a division of Red Hat * - * Copyright 2007, Red Hat Middleware, LLC, and individual * - * contributors as indicated by the @authors tag. See the * - * copyright.txt in the distribution for a full listing of * - * individual contributors. * - * * - * This is free software; you can redistribute it and/or modify it * - * under the terms of the GNU Lesser General Public License as * - * published by the Free Software Foundation; either version 2.1 of * - * the License, or (at your option) any later version. * - * * - * This software is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * - * Lesser General Public License for more details. * - * * - * You should have received a copy of the GNU Lesser General Public * - * License along with this software; if not, write to the Free * - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA * - * 02110-1301 USA, or see the FSF site: http://www.fsf.org. * - ******************************************************************************/ +/* + * JBoss, a division of Red Hat + * Copyright 2010, Red Hat Middleware, LLC, and individual + * contributors as indicated by the @authors tag. See the + * copyright.txt in the distribution for a full listing of + * individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ package org.gatein.wsrp.consumer; @@ -126,7 +126,7 @@ // specifiy that the prop is valid to simulate a successful registration (integration test, should have something // testing that in ProducerInfoTestCase) - prop.setInvalid(Boolean.FALSE, RegistrationProperty.Status.VALID); + prop.setStatus(RegistrationProperty.Status.VALID); info.setRegistrationPropertyValue("prop0", "value1"); assertTrue(info.isRefreshNeeded()); Modified: components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationPropertyTestCase.java =================================================================== --- components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationPropertyTestCase.java 2010-01-11 10:50:19 UTC (rev 1223) +++ components/wsrp/trunk/consumer/src/test/java/org/gatein/wsrp/consumer/RegistrationPropertyTestCase.java 2010-01-11 16:53:40 UTC (rev 1224) @@ -1,25 +1,25 @@ -/****************************************************************************** - * JBoss, a division of Red Hat * - * Copyright 2007, Red Hat Middleware, LLC, and individual * - * contributors as indicated by the @authors tag. See the * - * copyright.txt in the distribution for a full listing of * - * individual contributors. * - * * - * This is free software; you can redistribute it and/or modify it * - * under the terms of the GNU Lesser General Public License as * - * published by the Free Software Foundation; either version 2.1 of * - * the License, or (at your option) any later version. * - * * - * This software is distributed in the hope that it will be useful, * - * but WITHOUT ANY WARRANTY; without even the implied warranty of * - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * - * Lesser General Public License for more details. * - * * - * You should have received a copy of the GNU Lesser General Public * - * License along with this software; if not, write to the Free * - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA * - * 02110-1301 USA, or see the FSF site: http://www.fsf.org. * - ******************************************************************************/ +/* + * JBoss, a division of Red Hat + * Copyright 2010, Red Hat Middleware, LLC, and individual + * contributors as indicated by the @authors tag. See the + * copyright.txt in the distribution for a full listing of + * individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ package org.gatein.wsrp.consumer; @@ -54,6 +54,56 @@ prop = new RegistrationProperty("name", VALUE, "en", listener); } + public void testConstructor() + { + try + { + new RegistrationProperty(null, VALUE, "en", listener); + fail("Should have failed on null name"); + } + catch (Exception e) + { + } + + try + { + new RegistrationProperty("name", null, "en", listener); + } + catch (Exception e) + { + fail("Shouldn't have failed on null value"); + } + + try + { + new RegistrationProperty("name", VALUE, null, listener); + fail("Should have failed on null lang"); + } + catch (Exception e) + { + } + + try + { + new RegistrationProperty("name", VALUE, "en", null); + fail("Should have failed on null listener"); + } + catch (Exception e) + { + } + } + + public void testSetNullValue() + { + forceValid(); + + prop.setValue(null); + assertNull(prop.getValue()); + assertTrue(prop.isInvalid()); + assertTrue(prop.isDeterminedInvalid()); + assertEquals(RegistrationProperty.Status.MISSING_VALUE, prop.getStatus()); + } + public void testGetters() { assertEquals("name", prop.getName()); @@ -67,7 +117,7 @@ public void testSetValue() { - prop.setInvalid(Boolean.FALSE, RegistrationProperty.Status.VALID); + forceValid(); // we haven't changed the value, so the status shouldn't have changed prop.setValue(VALUE); @@ -81,40 +131,10 @@ assertEquals(RegistrationProperty.Status.UNCHECKED_VALUE, prop.getStatus()); } - public void testSetInvalid() + /** Force valid status so that we can check that status properly change based on state. */ + private void forceValid() { - prop.setInvalid(Boolean.FALSE, RegistrationProperty.Status.VALID); - assertEquals(Boolean.FALSE, prop.isInvalid()); - assertEquals(RegistrationProperty.Status.VALID, prop.getStatus()); - - // whatever the status, if we specifiy that the property is valid, its status should be VALID - prop.setInvalid(Boolean.FALSE, RegistrationProperty.Status.INEXISTENT); - assertEquals(Boolean.FALSE, prop.isInvalid()); - assertEquals(RegistrationProperty.Status.VALID, prop.getStatus()); - - prop.setInvalid(Boolean.FALSE, null); - assertEquals(Boolean.FALSE, prop.isInvalid()); - assertEquals(RegistrationProperty.Status.VALID, prop.getStatus()); - - try - { - prop.setInvalid(true, null); - fail("setInvalid should not accept a prop to be set invalid without a proper status"); - } - catch (IllegalArgumentException e) - { - // expected - } - - try - { - prop.setInvalid(true, RegistrationProperty.Status.VALID); - fail("setInvalid should not accept a prop to be set invalid with a VALID status"); - } - catch (IllegalArgumentException e) - { - // expected - } + prop.setStatus(RegistrationProperty.Status.VALID); } public void testPropertyChangedEvent()