2013/2/14 Ben Caradoc-Davies <ben.caradoc-dav...@csiro.au>

> Nice work, Mauro. You are getting closer.
>
> Thanks, it's a hard work, but someone had to do it :-)


> There are still some issues to be addressed.
>
> The good news is that all tests pass (including SchemaCacheOnlineTest,
> which requires ~/.geotools/schema-resolver.**properties to enable it).
> Furthermore, I have made the changes required in GeoServer in a branch;
> with these changes all GeoServer tests pass as well:
> https://github.com/**bencaradocdavies/geoserver/**tree/geot-4386<https://github.com/bencaradocdavies/geoserver/tree/geot-4386>
> This branch also removes xml-commons-resolver from the app-schema plugin,
> as it will now be shipped with the core GeoServer bundle (an extra 84 kB
> [don't tell Andrea]).
>

Cool, I'll have a look at it.

>
> Please update the javadocs in gt-app-schema-resolver and the user manual,
> which still mention the old class names. You should probably move the
> resolver section in the manual to xml, or wherever you think it makes most
> sense. for your convenience, I have attached a listing of all mentions of
> the old class names.
>
> I think you have copied too much of the test resources to gt-xml (and thus
> too much in review.apt). I think the top-level xml files and the catalog
> stuff is only used by the tests that are still left in
> gt-app-schema-resolver.  Please remove the duplication; you might then need
> no review.apt changes at all.
>

Ok, I will do it.


>
> I am not sure about your synchronisation. SchemaResolver maintains
> resolvedLocationToOriginalLoca**tionMap and I see no reason why you
> cannot have multiple file resolved locations (e.g. the temporary file
> created when concurrent downloads occur) for a single HTTP URI; the
> resolver will fix your relative imports for you (that is its job). I still
> see no synchronisation on resolvedLocationToOriginalLoca**tionMap; what
> happens if two threads try to update this map concurrently? (Bad Things
> Happen.) Can this ever happen? Is a SchemaResolver ever shared between
> threads? But if not, then how will SchemaCache download synchronisation
> occur (monitors are on instances)? I wonder if all synchronisation should
> be handled at a higher level. As this is going into gt-xml, I would like to
> defer to Justin's considerable expertise in this area.
>

Currently my use case is using this stuff from the Geotools wms client,
which in turn is used by Geoserver WMS cascading feature, so I think it
will definitely be used by many threads (tipically during GetCapabilities
requests).
I will have a look at the resolvedLocationToOriginalLoca**tionMap stuff,
which I missed.

>
> Justin: there are two things that might require synchronisation:
> (1) downloads of files into the cache
> (2) updates of SchemaResolver.**resolvedLocationToOriginalLoca**tionMap
> Can you please advise?


> Mauro, if this is only going to be used by a single-threaded client, it
> might be simpler to document the SchemaResolver as non-thread-safe until
> all resolutions have been performed at least once. A multithreaded
> application could maintain a single SchemaResolver and synchronise access
> to it (this is what I mean by higher-level synchronisation).
>

The alternative I see here is moving synchronization to gt-xml
SchemaFactory, that's where I use SchemaResolver.
Opinions?


-- 
==
Our support, Your Success! Visit http://opensdi.geo-solutions.it for more
information.
==

Dott. Mauro Bartolomeoli
@mauro_bart
Senior Software Engineer

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:     +39 0584 1660272

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to