I like your extension and a half logic :)
Notes for discussion:
- I am not used to seeing "plugin" in a classname, would expect
TransactionListener2
(since it extends TransactionListener). The style of methods looks more
like a callback though ...
- Like the use of defaults for ExtensionPriority
- Consider splitting afterTransaction into afterCommitt and afterRollback
to avoid the "committed" boolean flag
- Proposal includes beforeCommit and afterTransaction methods ... this is a
slightly different callback style, where as TransactionListener uses an
"event" style
For comparison an event style to allow the same functionality would be:
1) Adding additional events covering beforeCommit and afterTransaction
TransactionEventType.PRE_COMMIT
TransactionEventType.POST_COMMIT
TransactionEventType.PRE_ROLLBACK
TransactionEventType.POST_ROLLBACK
2) Providing TransactionEvent with the TransactionResponse object
3) Creating TransactionListener2 with method to listen to transaction
(rather than daatstore) changes.
public interface TransactionListener2 extends TransactionListener {
/**
* Check/alter transaction before/after changes applied to datastores
allowing interaction with
* event.getTransactionResponse() and event.getTransactionResponse().
* <p>
* <b>Note</b> that caution should be exercised when relying on feature
identifiers from a
* {@link TransactionEventType#POST_INSERT} event. Depending on the
type of store those identifiers
* may be reliable. Essentially they can only be relied upon in the
case of a spatial dbms (such
* as PostGIS) is being used.
* </p>
*/
void transactionChange(TransactionEvent event) throws WFSException;
}
--
Jody Garnett
On 26 October 2017 at 10:49, Andrea Aime <andrea.a...@geo-solutions.it>
wrote:
> Hi folks,
> for some work we are doing on a new community module (the NSG profile for
> WFS, will talk about it in
> a separate mail) we need to have TransactionPlugin be able to alter
> transactions.
> Unfortunately when WFS 2.0 was introduced the refactoring broke it and it
> cannot do that any more,
> ticket here:
>
> https://osgeo-org.atlassian.net/browse/GEOS-7403
>
> What is more, the thing cannot just be fixed, it has to be replaced,
> because WFS 2.0 transactions
> cannot be translated down to WFS 1.1 ones and brought back without losing
> some information
> (in particular, the new Replace element).
> So, I'm proposing to create a new TransactionPlugin2 that takes the
> TransactionRequest object,
> and make the latter modifyable enough to allow transaction manipulation.
> The interface looks the same and it's easy to migrate from the old one
> (I've just migrated two
> implementations quickly):
>
> public interface TransactionPlugin2 extends TransactionListener,
> ExtensionPriority {
> /**
> * Check/alter the transaction request elements
> */
> TransactionRequest beforeTransaction(TransactionRequest request)
> throws WFSException;
>
> /**
> * Say the last word before we actually commit the transaction
> */
> void beforeCommit(TransactionRequest request) throws WFSException;
>
> /**
> * Notification the transaction ended
> *
> * @param request
> * the originating transaction request
> * @param result
> * {@code null} if {@code committed == false}, the transaction
> result object to be
> * sent back to the client otherwise.
> *
> * @param committed
> * true if the transaction was successful, false if the
> transaction was aborted for
> * any reason
> */
> void afterTransaction(TransactionRequest request, TransactionResponse
> result, boolean committed);
>
> @Override
> default int getPriority() {
> return ExtensionPriority.LOWEST;
> }
> }
>
>
> (given the similarty to the old one, and the fact it is there to fix a
> regression, I counted this as a half new extension point :-p)
>
> For the same NSG plugin we need to also be able to alter GetFeature
> request before they hit the database, for that we'd introduce this
> new interface:
>
> public interface GetFeatureCallback extends ExtensionPriority {
>
> void beforeQuerying(GetFeatureContext context);
>
> @Override
> default int getPriority() {
> return ExtensionPriority.LOWEST;
> }
> }
>
>
> public final class GetFeatureContext {
>
> private final GetFeatureRequest request;
> private FeatureSource<? extends FeatureType, ? extends Feature>
> featureSource;
> private Query query;
> private final FeatureTypeInfo featureTypeInfo;
>
> public GetFeatureContext(GetFeatureRequest request, FeatureSource<?
> extends FeatureType, ? extends Feature> featureSource,
> Query query, FeatureTypeInfo featureTypeInfo) {
> this.request = request;
> this.featureSource = featureSource;
> this.query = query;
> this.featureTypeInfo = featureTypeInfo;
> }
>
> public GetFeatureRequest getRequest() {
> return request;
> }
>
> public FeatureSource<? extends FeatureType, ? extends Feature>
> getFeatureSource() {
> return featureSource;
> }
>
> public void setFeatureSource(FeatureSource<? extends FeatureType, ?
> extends Feature> featureSource) {
> this.featureSource = featureSource;
> }
>
> public Query getQuery() {
> return query;
> }
>
> public void setQuery(Query query) {
> this.query = query;
> }
>
> public FeatureTypeInfo getFeatureTypeInfo() {
> return featureTypeInfo;
> }
>
> }
>
>
> The idea is that implementors are free to wrap/replace the feature source
> and the query. It's up to the implementor to
> make sure the changes are not going to break the WFS protocol (or even
> feel free to break it, if they are in complete
> control of the client, as sometimes happens).
> The feature type and request are viewable, but not changeable, because at
> the point where the callback is called,
> they have been both used already to do various things, so so allowing to
> change them might give the wrong
> idea to implementors (that they can get effects they really cannot
> achieve).
>
> So, two questions:
>
> - Any feedback?
> - Do you think the above is proposal worthy?
>
> If I don't hear anything I'll assume nobody is worried and no proposal is
> required. Ah, I have no intention of backporting
> for the time being.
>
> 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 di Montramito 3/A
> 55054 Massarosa (LU)
> phone: +39 0584 962313 <+39%200584%20962313>
> fax: +39 0584 1660272 <+39%200584%20166%200272>
> mob: +39 339 8844549 <+39%20339%20884%204549>
>
> 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
>
>
------------------------------------------------------------------------------
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
