Hi Jody,
my crusade is to leverage the ContendDataStore framwork as much as possible
and hence have less to re-write.
So this is the situation: DiffTransactionState is package private and so
are its members. If I wrote my classes on the same package then I wouldn't
need to make it public. But I don't want my subclass to be on the same
package.
As for its member fields, I can avoid making them protected and keep them
package private if we overload the constructor to receive the Diff to use
instead of it creating a new Diff.
Why? because a lot of stuff depends on a DiffTransactionState being there
for the transaction, like in ContentFeatureReader:
DiffTransactionState state = (DiffTransactionState)
getTransaction().getState(getEntry());
Now, In wfs-ng I'm keeping a DiffTransactionState per feature type, as the
framework "requires", which is a subclass, _and_ a per transaction and
datastore State, called WFSRemoteTransactionState. That is because
DiffTransactionState works on a per feature-type basis and that's ok to
hold on the uncommitted state for each type, but at commit I need to
perform a single commit for all and every type that has been modified for
the transaction, and also need to keep some state on which are "batch"
operations and which are not, as to make the WFS transaction simpler.
For the same reason I want to be able of
extending DiffContentFeatureWriter. So that my subclass can pass a WFSDiff
instead of a plain Diff to the superclass constructor, and to override the
commit behavior. Alternative is to copy and paste and be done. Which would
be not that bad either.
And some more of the same for DiffContentFeatureWriter. So yeah, as long as
there are protected accessors I'd be fine.
Let me know if the attached patch seems cleaner.
Cheers,
Gabriel
On Tue, Feb 7, 2012 at 10:40 AM, Jody Garnett <[email protected]>wrote:
> Do you have an example of what you want to accomplish?
>
> If possible I would prefer to keep those fields private (and advertise
> accessors for you) so we have the option of messing with them in the
> future. As an example we recently changed from a Set to a List in order to
> preserve the order in which features were added.
>
> Think about the above; but if you really need those fields to be protected
> then please go ahead and apply your patch.
> --
> Jody Garnett
>
> On Tuesday, 7 February 2012 at 11:26 PM, Gabriel Roldan wrote:
>
> Hi Jody,
>
> On Mon, Feb 6, 2012 at 10:32 PM, Jody Garnett <[email protected]>wrote:
>
> It seems fine to me Gabriel; but why do you need to make those fields
> protected? When an accessor method is available - it feels like you have a
> subclass around or something :-)
>
>
> Yes, need to subclass DiffTransactionState and DiffContentFeatureWriter
> for wfs-ng in order to leverage the most possible of the ContentDataStore
> and friends in wfs-ng.
>
>
>
> Note that I have taken these classes for a walk over to ContentDataStore
> land and cleaned them up a bit.
>
>
> Sounds good.
>
> Cheers,
> Gabriel
>
>
> --
> Jody Garnett
>
> On Monday, 6 February 2012 at 6:31 PM, Gabriel Roldan wrote:
>
> Hello,
>
> Working on wfs-ng datastore found myself in need for the following two
> patches. May as ask for a code review and a statement of whether it's
> ok to apply?
>
> <
> https://github.com/groldan/geotools/commit/35010c5023eba8a1b8de2d86da08384b0f035fc3
> >
> <
> https://github.com/groldan/geotools/commit/9a46af15dc63a94013a34cdc62a7043d67a699f7
> >
>
> TIA,
> Gabriel
>
> --
> Gabriel Roldan
> OpenGeo - http://opengeo.org
> Expert service straight from the developers.
>
>
> ------------------------------------------------------------------------------
> Try before you buy = See our experts in action!
> The most comprehensive online learning library for Microsoft developers
> is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
> Metro Style Apps, more. Free future releases when you subscribe now!
> http://p.sf.net/sfu/learndevnow-dev2
> _______________________________________________
> GeoTools-Devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
>
>
>
>
> --
> Gabriel Roldan
> OpenGeo - http://opengeo.org
> Expert service straight from the developers.
>
>
>
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.
diff --git a/modules/library/data/src/main/java/org/geotools/data/store/DiffContentFeatureWriter.java b/modules/library/data/src/main/java/org/geotools/data/store/DiffContentFeatureWriter.java
index 53352ad..4f4a248 100644
--- a/modules/library/data/src/main/java/org/geotools/data/store/DiffContentFeatureWriter.java
+++ b/modules/library/data/src/main/java/org/geotools/data/store/DiffContentFeatureWriter.java
@@ -55,11 +55,11 @@ public class DiffContentFeatureWriter implements FeatureWriter<SimpleFeatureType
protected Diff diff;
- SimpleFeature next; // next value aquired by hasNext()
+ protected SimpleFeature next; // next value aquired by hasNext()
- SimpleFeature live; // live value supplied by FeatureReader
+ protected SimpleFeature live; // live value supplied by FeatureReader
- SimpleFeature current; // duplicate provided to user
+ protected SimpleFeature current; // duplicate provided to user
ContentFeatureStore store;
diff --git a/modules/library/data/src/main/java/org/geotools/data/store/DiffTransactionState.java b/modules/library/data/src/main/java/org/geotools/data/store/DiffTransactionState.java
index 74ddf8a..92fe648 100644
--- a/modules/library/data/src/main/java/org/geotools/data/store/DiffTransactionState.java
+++ b/modules/library/data/src/main/java/org/geotools/data/store/DiffTransactionState.java
@@ -34,7 +34,7 @@ import org.opengis.filter.Filter;
/**
* Transaction state responsible for holding an in memory {@link Diff} of any modifications.
*/
-class DiffTransactionState implements Transaction.State {
+public class DiffTransactionState implements Transaction.State {
Diff diff;
/** The transaction (ie session) associated with this state */
@@ -52,9 +52,19 @@ class DiffTransactionState implements Transaction.State {
* @param state ContentState for the transaction
*/
public DiffTransactionState(ContentState state) {
+ this(state, new Diff());
+ }
+
+ /**
+ * Transaction state responsible for holding an in memory {@link Diff}.
+ *
+ * @param state ContentState for the transaction
+ */
+ public DiffTransactionState(ContentState state, Diff diff) {
this.state = state;
- this.diff = new Diff();
+ this.diff = diff;
}
+
/**
* Access the in memory Diff.
* @return in memory diff.
------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
GeoTools-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
