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.garnett@anonymised.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.giannecchini@anonymised.com> 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.
