Mauro, that looks great. Both GeoTools and GeoServer (patched branch) build cleanly. The docs build and look fine.
The only question I have is: why is it necessary to synchronise the block where resolvedLocationToOriginalLocationMap.put is called, when this is already a ConcurrentHashMap? (I can see why the containsKey is a good idea.) I am not saying this is wrong, just that I do not yet understand. If you are confident that the concurrency issues are addressed, please go ahead an issue a pull request for master. (And please make sure the GeoServer changes get merged at the same time.) Thanks again! Kind regards, Ben. On 18/02/13 23:43, Mauro Bartolomeoli wrote: > Hi all, I tried to solve the remaining issues. > > 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 added a new section (resolving) in gt-xml docs, extracting info > specific to the 3 classes moved from app-schema-resolver to core (and I > removed the same docs from gt-app-schema-resolver). > > 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 removed the extra unneeded files. > > > 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. > > > I transformed resolvedLocationToOriginalLoca__tionMap to a > ConcurrentHashMap and synchronized updating accesses to it (the download > stuff was already synchronized in SchemaCache). > > The branch is the usual one: > https://github.com/mbarto/geotools/tree/geot_4386_no_app_schema_resolver > > Thanks > Mauro > > -- > == > 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 > > ------------------------------------------------------- -- Ben Caradoc-Davies <ben.caradoc-dav...@csiro.au> Software Engineer CSIRO Earth Science and Resource Engineering Australian Resources Research Centre ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_feb _______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
