Hi all,

I added a commit in the PR in order to fix build breaks in community modules 
due to the change of the CatalogVisitor interface.

Again, please note that the travis check is failing because of the missing 
GeoTools modules still in PR.
Also note the the GeoTools PR has been updated as well: the modules required 
for the WMTS cascading feature in GeoServer have been moved in the extensions 
package, and some tests have been added in order to increase the coverage.

   Cheers,
   Emanuele




Alle 17:41:09 di Tuesday 25 July 2017, Torben Barsballe ha scritto:
> In that case, I guess the current approach is fine.
> 
> Torben
> 
> On Tue, Jul 18, 2017 at 3:40 AM, Emanuele Tajariol <e...@geo-solutions.it>
> 
> wrote:
> > Hi Torben! Hi Jody!
> > 
> > IMHO I don't see a big advantage in such a refactoring:
> > - Most of the if/elseif chains contain different logic in the WMS/WMTS
> > blocks,
> > so the refactoring in a single class (WebServiceInfo  + wstype) would
> > only add
> > nested IF blocks inside the existing chains. Ugly to see (but that's
> > personal
> > taste :)), and not helping a lot in "automatically" reducing issues when
> > dealing with cascaded services.
> > - The visitors should be changed in order to only handle the
> > WebServiceInfo case, and, once again, we'd need an IF on the wstype to
> > find out the type and
> > act accordingly. This would be quite against the visitor pattern itself.
> > - The configuration persisting logic should be changed a lot, and
> > probably a
> > migration of the configuration files would be needed, which adds more
> > complexity
> > to the change.
> > 
> > If we wish to group the WMSLayerInfo and the WMTSLayerInfo in some way,
> > we may
> > create an empty interface WebServiceInfo and make the WMSLayerInfo and
> > WMTSLayerInfo implement it. This would probably help in some OR'ed tests
> > in the form "...|| source instanceof WMSLayerInfo || ...", but I don't
> > see much
> > more use of this        .
> > 
> > As Simone said, we're using the current if/elseif and visitor
> > architecture; a
> > refactoring involves changing much more code than the current proposal,
> > and would probably introduce issues on its own.
> > 
> > > My main concern is that adding a new Resource interface has a greater
> > > potential to introduce instability then extending / refactoring an
> > 
> > existing
> > 
> > > one, and in ways that may be less immediately apparent.
> > 
> > Torben, I see your point, but I guess that the possible instabilities
> > would pop out only when using the new WMTS cascading feature.
> > Introducing a refactoring, we may make the existing WMS cascaded layers
> > unstable, and that's
> > a much worse scenario.
> > 
> > > Change it so that WebSeviceInfo contains has a "wstype" (similar to
> > > "dbtype") that is used to to configure WebServiceInfo with either
> > > WMSProtocolStratagy or WMTSProtcolStratagy responsible for speaking the
> > > correct protocol and producing a GridCoverage for the rendering engine.
> > 
> > Jody, I don't think that the underlying services can be as similar as
> > different
> > dbms types. Furthermore, this kind of refactoring would require to have
> > the same class representing both a WMTS and a WMS resource, which may
> > contain different info and may evolve independently.
> > 
> >    Cheers,
> >    Emanuele
> > 
> > Alle 00:31:36 di Saturday 15 July 2017, Torben Barsballe ha scritto:
> > > On Fri, Jul 14, 2017 at 10:20 AM, Jody Garnett <jody.garn...@gmail.com>
> > > 
> > >  wrote:
> > > > Torben could we do this without introducing additional "info"
> > > > objects?
> > > > 
> > > > 
> > > > Change it so that WebSeviceInfo contains has a "wstype" (similar to
> > > > "dbtype") that is used to to configure WebServiceInfo with either
> > > > WMSProtocolStratagy or WMTSProtcolStratagy responsible for speaking
> > > > the correct protocol and producing a GridCoverage for the rendering
> > > > engine.
> > > > 
> > > > I think so. Looking at the current WMSLayer / WMSStore interface, it
> > > > is
> > > 
> > > fairly general already. The only bit that actually depends on the
> > > service
> > > 
> > > type is this method in WMSStoreInfo:
> > >     /**
> > >     
> > >      * Returns the underlying {@link WebMapServer}
> > >      * <p>
> > >      * This method does I/O and is potentially blocking. The
> > > 
> > > <tt>listener</tt> may be used to report
> > > 
> > >      * the progress of loading the datastore and also to report any
> > 
> > errors
> > 
> > > or warnings that occur.
> > > 
> > >      * </p>
> > >      *
> > >      * @param listener
> > >      *            A progress listener, may be <code>null</code>.
> > >      *
> > >      * @return The datastore.
> > >      *
> > >      * @throws IOException
> > >      *             Any I/O problems.
> > >      */
> > >     
> > >     WebMapServer getWebMapServer(ProgressListener listener) throws
> > > 
> > > IOException;
> > > 
> > > Where WebMapServer extends AbstractOpenWebService<WMSCapabilities,
> > 
> > Layer>.
> > 
> > > If one was to convert this to an abstract WebServiceInfo, then almost
> > > everything would stay the same as in the current WMS__Info, just
> > > slightly more general in the case of this one method.
> > > 
> > > On Fri, Jul 14, 2017 at 10:32 AM, Simone Giannecchini <
> > > 
> > > simone.giannecch...@geo-solutions.it> wrote:
> > > > Hi Torben,
> > > > while your suggestion makes sense and I have asked Emanuele to take
> > 
> > into
> > 
> > > > account I am also a bit reluctant to follow it.
> > > > 
> > > > We tried to stick to the current structure (not saying it is good or
> > 
> > bad,
> > 
> > > > it is just the way it is) and doing a general refactor now might give
> > 
> > us
> > 
> > > > more harm than anything
> > > > 
> > > >  as it would introduce instability and open a can of worms.
> > > > 
> > > > My main concern is that adding a new Resource interface has a greater
> > > 
> > > potential to introduce instability then extending / refactoring an
> > 
> > existing
> > 
> > > one, and in ways that may be less immediately apparent.
> > > 
> > > > So let's see what Emanuele says but I would more in favour of
> > 
> > refactoring
> > 
> > > > the whole thing separately, even because winter is coming (see other
> > > > email threads).
> > > > 
> > > > I would think that picking an approach and sticking with it is less
> > > 
> > > disruptive overall, as well as taking less effort, but I do understand
> > 
> > that
> > 
> > > things are busy for everyone with the upcoming code freeze and
> > > everything else.
> > > 
> > > I would certainly be interested in hearing your thoughts Emanuele.
> > > 
> > > Torben
> > 
> > --
> > Regards,
> > Emanuele Tajariol
> > ==
> > GeoServer Professional Services from the experts! Visit
> > http://goo.gl/it488V
> > for more information.
> > ==
> > 
> > Ing. Emanuele Tajariol
> > Technical Lead
> > 
> > GeoSolutions S.A.S.
> > Via di Montramito 3/A
> > 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.


-- 
Regards,
Emanuele Tajariol
==
GeoServer Professional Services from the experts! Visit http://goo.gl/it488V 
for more information.
==

Ing. Emanuele Tajariol
Technical Lead

GeoSolutions S.A.S.
Via di Montramito 3/A
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.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Reply via email to