ContentFeatureStore fixes

Description

These three fixes need to be made to ContentFeatureStore to support CSV datastore:

1. removeFeatures() has this loop:

//remove everything
while( writer.hasNext() ) {
writer.next();
writer.remove();
writer.write();
}

but it really should look like this:

//remove everything
while( writer.hasNext() ) {
writer.next();
writer.remove();
}

The writer.write() should not be there.

2. ContentFeatureStore.getWriter(Query, int) needs to have (at least) a FilteringWriter. See attached patch.

3. There's a minor tweak in ContentFeatureStore.getWriter(Filter, int). Instead of "new DefaultQuery(..." it should be "new Query(.." – DefaultQuery is deprecated.

Environment

None

Activity

Show:
codehaus
April 10, 2015, 3:05 PM

CodeHaus Comment From: jdeolive - Time: Tue, 24 May 2011 19:59:00 -0500
---------------------
Sounds good. Sorry I came off as harsh... just started to sound like you were going to run wild in that module and i got thrown off. The work is appreciated and I look forward to reviewing future patches.

codehaus
April 10, 2015, 3:05 PM

CodeHaus Comment From: jgarnett - Time: Tue, 24 May 2011 19:51:59 -0500
---------------------
To be honest I don't know what is coming; this is the first time we are writing a non JDBC FeatureStore; I imagine a few more controlled wrapper such as provided in this patch. At the very least for TransactionStateDiff (as that is not something JDBC uses). I am looking for collaboration here Justin; Lee and I are here as volunteers, as are you, this is a background task welcoming Lee to the code base as it were

So for any changes to ContentDataStore we will be submitting Jiras and asking you to check.

codehaus
April 10, 2015, 3:05 PM

CodeHaus Comment From: jdeolive - Time: Tue, 24 May 2011 18:17:29 -0500
---------------------
Instability? This looks like a simple patch to me. What else is coming? To be honest you send so many emails that I simply don't have enough time to read over all of them. All I can do is watch jira and review issues that people assign to me or add me as a watcher. If you plan on adding "instability" we'll have to fork the current ContentDataStore.

codehaus
April 10, 2015, 3:05 PM

CodeHaus Comment From: jgarnett - Time: Tue, 24 May 2011 17:54:53 -0500
---------------------
Yep (and I did send a warning to the email list that you should expect some instability, and we created this issue to give you a chance to review ).

codehaus
April 10, 2015, 3:05 PM

CodeHaus Comment From: jdeolive - Time: Tue, 24 May 2011 09:54:20 -0500
---------------------
Well when we found those classes they really were just a proof of concept and the jdbc datastores took them to something that actually worked. They now form the base for our most robust set of datastores, and are also the new official starting point for new datastores. So I dont; expect major changes to be made to them without giving myself or andrea a chance to review. So let's say we are co-maintaining them.

Fixed

Assignee

Unassigned

Reporter

codehaus

Triage

None

Components

Fix versions

Affects versions

Priority

Medium