Should be fine Stefano, especially with that javadoc warning. I sometimes
name method like that as "internalMappings()" (assuming the by Names is
obvious from the type signature).

I am a bit concerned about "weird failures" in unrelated modules since I am
trying to catch up and merge all the pull requests that are coming in.

--
Jody Garnett

On 10 August 2015 at 14:32, Stefano Costa <[email protected]>
wrote:

> Hi,
> pull request with the fix is here:
> https://github.com/geotools/geotools/pull/932
>
> All unit tests in the app-schema extension pass, I couldn't try a full
> build because of some weird failures in other unrelated modules.
>
> The fix contains a small API change: I renamed the getMappingByElement()
> method to getMappingByNameOrElement(), for the sake of clarity. The
> method is public so this is technically an API change, but its JavaDocs
> state that it is public "just for testing purposes": should I assume
> it's safe to break the API in this case? If not, I can always go back to
> the old name.
> Also, the fix made the getMappingByName() method useless and I was
> tempted to remove it, since it is also public "just for testing
> purposes", but in the end I decided not to: should I go ahead and remove
> it?
>
> Any advice is appreciated.
>
> Thanks,
> --S
>
> Il giorno lun, 10/08/2015 alle 19.53 +0200, Stefano Costa ha scritto:
> > Hi all,
> > being officially on vacation, I finally found some spare time to look
> > into this :-)
> >
> > I was able to write a unit test showing the issue:
> >
> > @Test
> > public void testGetNamesAndFeatureSources() throws Exception {
> >     /*
> >      * Initiate data accesses and make sure they have the mappings
> >      */
> >     final Map dsParams = new HashMap();
> >     URL url = getClass().getResource(schemaBase + "GeologicUnit.xml");
> >     assertNotNull(url);
> >     dsParams.put("dbtype", "app-schema");
> >     dsParams.put("url", url.toExternalForm());
> >
> >     DataAccess guDataStore = DataAccessFinder.getDataStore(dsParams);
> >     assertNotNull(guDataStore);
> >
> >     for (Name name: guDataStore.getNames()) {
> >         FeatureSource  fs = guDataStore.getFeatureSource(name);
> >         assertNotNull(fs);
> >     }
> > }
> >
> > I added this method to GeologicUnitTest.java (also had to add an @After
> > method to cleanup the data access registry after each test, but this is
> > not relevant to the discussion) and the following exception is thrown
> > when the test runs:
> >
> > org.geotools.data.DataSourceException: myGeologicUnit not found.
> > Available: [urn:cgi:xmlns:CGI:GeoSciML:2.0:GeologicUnit]
> >       at
> >
> org.geotools.data.complex.AppSchemaDataAccess.getMappingByElement(AppSchemaDataAccess.java:210)
> >       at
> >
> org.geotools.data.complex.AppSchemaDataAccess.getFeatureSource(AppSchemaDataAccess.java:634)
> >       at
> >
> org.geotools.data.complex.GeologicUnitTest.testGetNamesAndFeatureSources(GeologicUnitTest.java:210)
> >       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> >
> > Problem is, the mapping configuration in GeologicUnit.xml specifies a
> > mappingName for the GeologicUnit feature type (myGeologicUnit), which is
> > returned by getNames(), but getFeatureSource() only looks up by element
> > and getFeatureSource("myGeologicUnit") fails.
> >
> > I'll open a JIRA ticket and try to come up with a fix.
> >
> > --S
> >
> > Il giorno dom, 09/08/2015 alle 10.29 +0200, Andrea Aime ha scritto:
> > > On Fri, Aug 7, 2015 at 9:57 PM, Ben Caradoc-Davies <[email protected]>
> > > wrote:
> > >         Thanks for the confirmation. My guess is that this is
> > >         something needed
> > >         for multiply mapped types or feature chaining and that
> > >         getFeatureSourceByName could have an even more longwinded
> > >         method name
> > >         like getFeatureSourceByElementName.
> > >
> > >
> > > By looking at the javadocs, it seems AppSchemaDataAccess is not
> > > abiding to the contract here:
> > >
> > >
> > > /**
> > >      * Names of the available Resources.
> > >      * <p>
> > >      * For additional information please see getInfo( Name ) and
> > > getSchema( Name ).
> > >      * </p>
> > >      * @return Names of the available contents.
> > >      * @throws IOException
> > >      */
> > >     List<Name> getNames() throws IOException;
> > >
> > >     /**
> > >
> > >      * Access to the named resource.
> > >      * <p>
> > >      * The level of access is represented by the instance of the
> > > FeatureSource
> > >      * being returned.
> > >      * <p>
> > >      * Formally:
> > >      * <ul>
> > >      * <li>FeatureSource - read-only access
> > >      * <li>FeatureStore - read-write access
> > >      * <li>FetureLocking - concurrency control
> > >      * <ul>
> > >      * Additional interfaces may be supported by the implementation
> > > you are using.
> > >      * @param typeName
> > >      * @return Access to the named resource being made available
> > >      * @throws IOException
> > >      */
> > >     FeatureSource<T,F> getFeatureSource(Name typeName) throws
> > > IOException;
> > >
> > >
> > > The way it looks, one should be able to call getNames() and then
> > > safely
> > > getFeatureSource, there is no indication that a named resource may
> > > not be available for data access.
> > >
> > >
> > > Cheers
> > > Andrea
> > >
> > >
> > > --
> > > ==
> > > GeoServer Professional Services from the experts! Visit
> > > http://goo.gl/it488V for more information.
> > > ==
> > >
> > >
> > >
> > > Ing. Andrea Aime
> > >
> > > @geowolf
> > > Technical Lead
> > >
> > >
> > > GeoSolutions S.A.S.
> > > Via Poggio alle Viti 1187
> > > 55054  Massarosa (LU)
> > > Italy
> > > phone: +39 0584 962313
> > > fax: +39 0584 1660272
> > > mob: +39 339 8844549
> > >
> > >
> > > http://www.geo-solutions.it
> > > http://twitter.com/geosolutions_it
> > >
> > >
> > > AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
> > >
> > > Le informazioni contenute in questo messaggio di posta elettronica e/o
> > > nel/i file/s allegato/i sono da considerarsi strettamente riservate.
> > > Il loro utilizzo è consentito esclusivamente al destinatario del
> > > messaggio, per le finalità indicate nel messaggio stesso. Qualora
> > > riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
> > > cortesemente di darcene notizia via e-mail e di procedere alla
> > > distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
> > > Conservare il messaggio stesso, divulgarlo anche in parte,
> > > distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
> > > diverse, costituisce comportamento contrario ai principi dettati dal
> > > D.Lgs. 196/2003.
> > >
> > >
> > >
> > > The information in this message and/or attachments, is intended solely
> > > for the attention and use of the named addressee(s) and may be
> > > confidential or proprietary in nature or covered by the provisions of
> > > privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
> > > Data Protection Code).Any use not in accord with its purpose, any
> > > disclosure, reproduction, copying, distribution, or either
> > > dissemination, either whole or partial, is strictly forbidden except
> > > previous formal approval of the named addressee(s). If you are not the
> > > intended recipient, please contact immediately the sender by
> > > telephone, fax or e-mail and delete the information in this message
> > > that has been received in error. The sender does not give any warranty
> > > or accept liability as the content, accuracy or completeness of sent
> > > messages and accepts no responsibility  for changes made after they
> > > were sent or for other risks which arise as a result of e-mail
> > > transmission, viruses, etc.
> > >
> > >
> > >
> > >
> > > -------------------------------------------------------
> >
>
> --
>
> Best regards,
> Stefano Costa
>
> ==
> GeoServer Professional Services from the experts! Visit
> http://goo.gl/it488V for more information.
> ==
> Dott. Stefano Costa
> 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
>
> -------------------------------------------------------
> AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
> Le informazioni contenute in questo messaggio di posta elettronica e/o
> nel/i file/s allegato/i sono da considerarsi strettamente riservate.
> Il loro utilizzo è consentito esclusivamente al destinatario del
> messaggio, per le finalità indicate nel messaggio stesso. Qualora
> riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
> cortesemente di darcene notizia via e-mail e di procedere alla
> distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
> Conservare il messaggio stesso, divulgarlo anche in parte,
> distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
> diverse, costituisce comportamento contrario ai principi dettati dal
> D.Lgs. 196/2003.
>
> The information in this message and/or attachments, is intended solely
> for the attention and use of the named addressee(s) and may be
> confidential or proprietary in nature or covered by the provisions of
> privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
> Data Protection Code).Any use not in accord with its purpose, any
> disclosure, reproduction, copying, distribution, or either
> dissemination, either whole or partial, is strictly forbidden except
> previous formal approval of the named addressee(s). If you are not the
> intended recipient, please contact immediately the sender by
> telephone, fax or e-mail and delete the information in this message
> that has been received in error. The sender does not give any warranty
> or accept liability as the content, accuracy or completeness of sent
> messages and accepts no responsibility  for changes made after they
> were sent or for other risks which arise as a result of e-mail
> transmission, viruses, etc.
>
>
>
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> GeoTools-Devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
------------------------------------------------------------------------------
_______________________________________________
GeoTools-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to