Fernando: Rather than watching communication go by: - Moved your proposal to the description section (you had the proposal outlined as tasks) and wrote in my assumption for MarkFactoryProcessor interface. - Wrote up alternate proposal more formally
-- Jody Garnett On Thu, 12 Aug 2021 at 14:34, Fernando Mino < fernando.m...@geosolutionsgroup.com> wrote: > Hi Jody, > > Thanks for the insight on Factory registry as a better alternative. > Indeed the DynamicSymbolFactoryFinder.getMarkFactories() -> > FactoryRegister.setOrdering(..) approach will do the trick for one of the > scenarios we want to support (I will amend the proposal to add this to > support the global configuration use case) but we also have the > per-workspace use case that allows: > - To have several different MarkFactory ordering and filtering on several > workspaces. > - To have several WMS GetMap requests (and rendering threads) in parallel, > rendering Mark shapes with different MarkFactory implementations order and > availability profiles. > > Probably I'm wrong (and please let me know if indeed there is a way to > achieve this with DynamicSymbolFactoryFinder.getMarkFactories() -> > FactoryRegister.setOrdering), but looks like the per-workspace > configuration + multithreading support is not possible with FactoryRegister > due to its not-thread-safe, globally cached and synchronized-static-methods > caller(DynamicSymbolFactoryFinder). > > To use FactoryRegistry we would need to change DynamicSymbolFactoryFinder > to allow us to store and index different FactoryRegistry instances per > workspace, but the problem here is GeoTools Rendering module is not aware > of GeoServer workspaces, and that is why I proposed the on-the-fly > filtering and ordering interceptor extension point, to allow us to control > this filtering from GeoServer code during request runtime. > > Naturally any better idea to solve this second use case is welcome. > > Regards, > > Fernando Mino > > == > > GeoServer Professional Services from the experts! > > Visit http://bit.ly/gs-services-us for more information. > > == > > Fernando Mino > > Software Engineer > > @fmy2kec > > GeoSolutions Group > phone: +39 0584 962313 > > fax: +39 0584 1660272 > > https://www.geosolutionsgroup.com/ > > http://twitter.com/geosolutions_it > > ------------------------------------------------------- > > Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE > 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si > precisa che ogni circostanza inerente alla presente email (il suo > contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è > riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il > messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra > operazione è illecita. Le sarei comunque grato se potesse darmene notizia. > > This email is intended only for the person or entity to which it is > addressed and may contain information that is privileged, confidential or > otherwise protected from disclosure. We remind that - as provided by > European Regulation 2016/679 “GDPR” - copying, dissemination or use of this > e-mail or the information herein by anyone other than the intended > recipient is prohibited. If you have received this email by mistake, please > notify us immediately by telephone or e-mail. > > > On Wed, Aug 11, 2021 at 8:11 PM Jody Garnett <jody.garn...@gmail.com> > wrote: > >> Thanks for doing this up as a proposal, it really helps with discussion. >> >> I did not think that MarkFactory or ExternalGraphicFactory needed to be >> aware of priority as this is handled by FactoryRegistry; but you are >> correct none of the existing implementations extend AbstractFactory. >> >> Your proposal is on MarkFactory; do you need a similar change for >> ExternalGraphicFactory? >> >> I am not quite sure how MarkFactoriesProcessor is intended to work: >> >> - processFactories( Iterator<MarkFactory> factories ): >> Iterator<MarkFactory> >> Processing an iterator on the fly is good, it amounts to doing pair >> wise ordering, or a comparator, which is already supported by >> FactoryRegistry. >> - priority(): int >> What is the int for? Is it to order between competing instances >> of MarkFactoriesProcessor ? >> >> >> My hesitation is seeing more than one way to do things, especially if it >> is a one-off for a specific factory. So I would like to see what the >> limitation you are running into with the existing setup, and if your >> proposal is an improvement we could do it for all FactoryFinders. We can >> look at other factory finders for examples of helper methods used to >> address common challenges (factory iterator providers for injecting >> instances, establishing pairwise ordering >> <https://github.com/geotools/geotools/blob/06d379a2fdbfdbf94641c16813c37ef02aa7f63c/modules/library/referencing/src/main/java/org/geotools/referencing/ReferencingFactoryFinder.java#L546>, >> ...). I know that establishing pairwise ordering is a pain, and there is >> already a helper method that takes a comparator >> <https://github.com/geotools/geotools/blob/main/modules/library/metadata/src/main/java/org/geotools/util/factory/FactoryRegistry.java#L1265> >> to make this easier. >> >> Checking your proposal you reference propose changing the code here >> <https://github.com/geotools/geotools/blob/2c0f22f0a5a13885aa1d85faa6f715ea87f3c90e/modules/library/render/src/main/java/org/geotools/renderer/style/SLDStyleFactory.java#L1362> >> : >> >> *Iterator<MarkFactory> it = >> DynamicSymbolFactoryFinder.getMarkFactories();* >> >> >> I would ask instead that you change DynamicSymbolFactoryFinder directly; >> that interface is already responsible for SPI discovery, >> >> Here is a proposed alternative approach: >> >> 1. Leave StyleFactory alone (it already has too much logic) and >> trust DynamicSymbolFactoryFinder.getMarkFactories() to respect your >> ordering, especially since you are trying to work around how factory >> discovery is based on classpath order. >> >> 2. Create a method DynamicSymbolFactoryFinder setMarkFactoryOrder( >> Comparator<MarkFactory> comparator ): >> >> public static synchronized Iterator<MarkFactory> setMarkFactoryOrder( >> Comparator<MarkFactory> comparator ) { >> getServiceRegistry().setOrdering( MarkFactory.class, comparator ); >> } >> >> 4. This calls FactoryRegister.setOrdering( category, comparator ) >> <https://github.com/geotools/geotools/blob/main/modules/library/metadata/src/main/java/org/geotools/util/factory/FactoryRegistry.java#L1265> >> which >> should do the trick. >> >> >> -- >> Jody Garnett >> >
_______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
