In that case, I guess the current approach is fine.
Torben
On Tue, Jul 18, 2017 at 3:40 AM, Emanuele Tajariol <[email protected]>
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 <[email protected]>
> >
> > 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 <
> >
> > [email protected]> 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.
>
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
