Nice work, Mauro. You are getting closer.

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
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]).

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.

I am not sure about your synchronisation. SchemaResolver maintains resolvedLocationToOriginalLocationMap 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 resolvedLocationToOriginalLocationMap; 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.

Justin: there are two things that might require synchronisation:
(1) downloads of files into the cache
(2) updates of SchemaResolver.resolvedLocationToOriginalLocationMap
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).

Thanks again.

Kind regards,
Ben.

On 14/02/13 01:08, Mauro Bartolomeoli wrote:
Hi guys, I tried to integrate all your suggestions with a new commit on
this branch:
https://github.com/mbarto/geotools/tree/geot_4386_no_app_schema_resolver.

Basically what I did is:
  * removed classes migrated to gt-xml (SchemaResolver, SchemaCache and
SchemaCatalog) from app-schema-resolver, introducing a dependency to
gt-xml in the app-schema and gt-complex extension modules
  * copied text from app-schema review.apt to gt-xml (maybe I copied too
much, please Ben have a look at the file and tell me if I need to
correct that)
  * introduced synchronization in SchemaCache (to avoid usage of
partially downloaded files in cache)

Please, have a look at the new commit and tell me if this is enough for
a pull request.

Thanks
Mauro Bartolomeoli

--
==
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
geotools(geot_4386_no_app_schema_resolver)$ find . -regextype posix-egrep 
-regex ".*\\.(java|rst)" -exec grep -H -n -E 
"AppSchema(Cache|Catalog|Resolver)" {} \;
./docs/user/extension/app-schema.rst:164:This module will resolve any resource 
type, but it is designed to aid relative imports between XML Schemas; to do 
this it keeps a reverse-lookup table to convert resolved locations back to 
their original locations, facilitating correct determination of relative 
imports and includes. To ensure that this works, use a single instance of 
``AppSchemaResolver`` to resolve a schema and all its dependencies.
./docs/user/extension/app-schema.rst:166:Optional ``AppSchemaResolver`` 
constructors arguments allow configuration of permitted resolution methods.
./docs/user/extension/app-schema.rst:176:    AppSchemaResolver resolver = new 
AppSchemaResolver(AppSchemaCatalog.build(DataUtilities.fileToURL(new 
File("/path/to/catalog.xml"))));
./docs/user/extension/app-schema.rst:184:    AppSchemaResolver resolver = new 
AppSchemaResolver();
./docs/user/extension/app-schema.rst:194:    AppSchemaResolver resolver = new 
AppSchemaResolver(new AppSchemaCache(new File("/path/to/app-schema-cache"), 
true));
./docs/user/extension/app-schema.rst:202:Once you have configured your 
``AppSchemaResolver``, you can use it to build an ``AppSchemaConfiguration`` 
that you can use to configure the GeoTools ``Encoder``::
./docs/user/extension/app-schema.rst:236:The Application Schema Packages 
collection in ``app-schema-packages`` contains GML application schemas that 
have been packaged into Maven artifacts to support offline testing. These are 
manually published to the ``osgeo`` Maven repository. Configuring your Maven 
project to depend on one of these packages will cause ``AppSchemaResolver`` to 
resolve references to these schemas on the classpath.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaXSD.java:31:
 * {@link XSD} that uses {@link AppSchemaResolver} to locate schema resources 
in a catalog, on the
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaLocationResolver.java:24:
 * A {@link SchemaLocationResolver} that uses {@link AppSchemaResolver} to 
locate schema resources
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:43:
 * A class to perform XML schema validation against schemas found using an 
{@link AppSchemaResolver}
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:79:
     * {@link AppSchemaResolver#getSimpleHttpResourcePath(java.net.URI)}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:87:
     * using an {@link AppSchemaResolver}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:98:
     * using an {@link AppSchemaResolver} with a {@link AppSchemaCatalog}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:101:
     *            AppSchemaCatalog
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:194:
     * {@link AppSchemaResolver#getSimpleHttpResourcePath(java.net.URI)}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:202:
     * using an {@link AppSchemaResolver}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:213:
     * using an {@link AppSchemaResolver} with a {@link AppSchemaCatalog}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:216:
     *            AppSchemaCatalog
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:226:
     * {@link AppSchemaResolver#getSimpleHttpResourcePath(java.net.URI)}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:235:
     *            AppSchemaCatalog to aide local schema resolution or null
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:257:
     * {@link AppSchemaResolver#getSimpleHttpResourcePath(java.net.URI)}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:266:
     *            AppSchemaCatalog to aide local schema resolution or null
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:319:
     * {@link AppSchemaResolver#getSimpleHttpResourcePath(java.net.URI)}.
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:328:
     *            AppSchemaCatalog file to aide local schema resolution or null
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaValidator.java:337:
     * An {@link EntityResolver2} that uses the enclosing instance's {@link 
AppSchemaResolver} to look up XML entities (that is, XML schemas).
./modules/extension/app-schema/app-schema-resolver/src/main/java/org/geotools/xml/AppSchemaConfiguration.java:23:
 * XML encoder {@link Configuration} that uses {@link AppSchemaResolver} to 
obtain schemas.

------------------------------------------------------------------------------
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