Here is a pull request fixing the retyping problem:
https://github.com/geotools/geotools/pull/668
Because the issue only crops up sporadically, I am not sure I can create a
usefull test case...
Torben
On Wed, Jan 7, 2015 at 11:12 AM, Torben Barsballe <
[email protected]> wrote:
> I appear to have found a deeper root cause of this problem and it looks
> like it does not actually have much to do with locking.
> My origional thoughts about the cause of the error appear to be in error -
> this particular instance is correct in failing here as that is what the
> test in question is intended to test. My confusion comes from the fact that
> this test would run, through an (expected) exception, and then all
> subsequent test methods in the same test file would fail with a
> ServiceException, reminicient of a locking failure.
>
> Note that there are still some issues with locking in addition to this
> problem.
>
>
> It turns out that this failure was in part caused by an issue with
> retyping:
>
> The test file used for the locking tests contains two geometry attributes
>
> When ContentDataStore creates a retyping reader, it creates a target
> featureType using
> 605: SimpleFeatureTypeBuilder.retype(getSchema(),
> query.getPropertyNames());
>
> It then compares this target featureType to the featureType of the reader
> to test if a retyping reader is actually neccessary:
> 609 if ( !target.equals( reader.getFeatureType() ) ) {
> 610 reader = new ReTypeFeatureReader( reader, target, false );
> 611 }
>
> The error is introduced by our two geometries; ultimately getSchema()
> (sometimes) returns a featureType with a different defaultGeometry then
> that used by the reader (I suspect this is caused by arbitrary collection
> ordering of the attributes). This caused the .equals call to return false,
> despite the fact that both featureTypes have the same attributes. Then,
> when we go to build the ReTypeFeatureReader, we get a failure because both
> schemas have the same attribures.
>
> This failure is returned to WFS as a ServiceException.
>
> The problem can be fixed by changing line 605 of ContentDataStore to
> 605: SimpleFeatureTypeBuilder.retype(reader.getFeatureType(),
> query.getPropertyNames());
>
> I am currently testing this change, and should have a pull request shortly.
>
> One question I have is if there is any way to ensure that we always get
> the same defaultGeometry when we create a schema?
>
> Torben
>
>
> On Tue, Jan 6, 2015 at 10:44 AM, Torben Barsballe <
> [email protected]> wrote:
>
>> Oops, it looks like ContentState tracks DiffTransactionState, so the
>> DiffContentFeatureWriter can actually access the DiffTransactionState via
>> the ContentState. While this invalidates some of my previous points, it
>> still might be worthwile to consider storing the flag in ContentState,
>> since much of the work we will are disabling is done through ContentState.
>>
>> Torben
>>
>> On Tue, Jan 6, 2015 at 10:37 AM, Torben Barsballe <
>> [email protected]> wrote:
>>
>>> Following along here:
>>>
>>>> Adding an extra isClosing field to DiffTransactionState seems the
>>>> cleanest approach.
>>>>
>>>
>>> Using this method, we give the writer in DiffTransactionState the
>>> transaction.
>>> This transaction gives the lock writer the correct authorizations to
>>> pass the checks
>>> The isClosing field is then accessed by the diff writer. If set to true,
>>> the diffwriter doesn't write anything
>>>
>>> Based on the implementation, it looks like it might be better to put the
>>> isClosing field in ContentState:
>>> - The diff writer (DiffContentFeatureWriter) can access the Diff, the
>>> ContentState, and the DataStore, but not the DiffTransactionState
>>> - DiffTransactionState can access the Diff and the ContentState, but
>>> cannot directly access the diff writer.
>>> - The ContentState is unique to the transaction, and stores state info,
>>> so is at least somewhat appropriate for such a flag
>>>
>>> If this implementation is a bad idea for some reason, please tell me - I
>>> am still not entirely familiar with how Diffs, Transactions, and Locking
>>> behave and interact in geotools.
>>> If there is a better place to put this flag where it can be set by
>>> DiffTransactionState and still accessed by DiffContentFeatureWriter then I
>>> am open to suggestions.
>>>
>>> Torben
>>>
>>
>>
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net
_______________________________________________
GeoTools-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
