Hey,
implementing an alternate catalog facade, found something weird.
CatalogImplTest.testProxyListBehaviour does the following:
List<StyleInfo> styles = catalog.getStyles();
Collections.sort( styles, new Comparator<StyleInfo>(){...});
Problem is two fold:
- the returned list is said to be immutable. The javadocs for
ProxyList say "An unmodifiable list proxy in which each element in the
list is wrapped in a proxy of its own". Yet the collection is being
sorted, hence changed.
- The internal state of the DefaultCatalogFacade is being changed as
a side effect. It's internal list of styles is being returned as is,
wrapped on a ProxyList, I suppose relying on the fact that the wrapper
should make it immutable.
So, breaking the encapsulation of DefaultCatalogFacade.styles may lead
to unexpected issues. Either a ConcurrentModificationException or
phantom reads when multiple threads use it.
It is not clear whether the returned list shall be immutable by
contract or not. I would say yes. I'm pretty sure I've seen a test
that ensured ProxyList is, but don't seem to be able to find it now.
In any case, immutable is good.
So what do we do?
a- return a defensive copy on getStyles()
b- return an actually immutable list? (Note javadoc for all the
methods returning a list say "The resulting list should not be used to
add or remove styles, the {@link #add(XXXInfo)} and {@link
#remove(XXXInfo)} are used for that purpose.
c- both?
Cheers,
Gabriel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
