Wow that is a lot of work. So you are needing to set the hint on a feature by feature basis - suspect we have completely given up on Transaction for hint purposes? You have not made an api change so committing to 2.6.x seems reasonable. Proposals work best when you don't have an implementation ready and would like to discuss alternatives; I think you did that already in your email discussion.
I do ask that we document this somewhere; let me take your existing code example and work with it: - http://docs.codehaus.org/display/GEOTDOC/07+FeatureStore Let me know what you think. Jody On 23/05/2010, at 5:51 AM, Andrea Aime wrote: > Hi, > some time ago we started a discussion on how to support > the ability to insert a feature with FeatureStore.addFeatures > so that the feature id is used as is in the underlying > storage instead of being generated. > > Today I cooked a little patch that enables the above > in all JDBC data stores and in the property data store > (attached, use patch -p1 to apply or tell whatever you > use to skip the first item in the path). > > How does it work? Well, if the feature store QueryCapabilities > adverties the support of "useExisting" you basically put a hint in > the feature user data stating that you want to > preserve the feature id as is (if the feature id cannot > be parsed an exception will the thrown). > > Here is an example from the JDBC tests: > > -------------------------------------------------------------- > > public void testAddFeaturesUseExisting() throws IOException { > // check we advertise the ability to reuse feature ids > assertTrue(featureStore.getQueryCapabilities().isUseExisingFIDSupported()); > > SimpleFeatureBuilder b = new > SimpleFeatureBuilder(featureStore.getSchema()); > DefaultFeatureCollection collection = new DefaultFeatureCollection(null, > featureStore.getSchema()); > > String typeName = b.getFeatureType().getTypeName(); > for (int i = 3; i < 6; i++) { > b.set(aname("intProperty"), new Integer(i)); > b.set(aname("geometry"), new GeometryFactory().createPoint(new > Coordinate(i, i))); > b.featureUserData(Hints.USE_EXISTING_FID, Boolean.TRUE); > collection.add(b.buildFeature(typeName + "." + (i * 10))); > } > List<FeatureId> fids = featureStore.addFeatures(collection); > > assertEquals(3, fids.size()); > > // check the created ids are the ones expected > assertTrue(fids.contains(SimpleFeatureBuilder.createDefaultFeatureIdentifier(typeName > + ".30"))); > assertTrue(fids.contains(SimpleFeatureBuilder.createDefaultFeatureIdentifier(typeName > + ".40"))); > assertTrue(fids.contains(SimpleFeatureBuilder.createDefaultFeatureIdentifier(typeName > + ".50"))); > > > SimpleFeatureCollection features = featureStore.getFeatures(); > assertEquals(6, features.size()); > > FilterFactory ff = dataStore.getFilterFactory(); > > // manually check features are actually there > for (Iterator f = fids.iterator(); f.hasNext();) { > FeatureId identifier = (FeatureId) f.next(); > String fid = identifier.getID(); > Id filter = ff.id(Collections.singleton(identifier)); > > features = featureStore.getFeatures(filter); > assertEquals(1, features.size()); > } > } > > -------------------------------------------------------------- > > Code wise, I made the following changes: > - added a isUseExistingFIDSupported() method to the QueryCapabilities > class (which makes it not so "query" only anymore...) > - modified a bit the FeatureIdImpl to allow the feature id > to be modified by the internal API (users should not know/use > that class anyways) > - modified ContentFeatureStore and AbstractFeatureStore to > pass down the external FID when writing new features into > the underlying storage > - modified the JDBCDataStore to use the hint and parse the FID > instead of trying to generate the code > > As a result of this modification all JDBC data stores plus > PropertyDataStore support the Hints.USE_EXISTING_FID hint. > > Not so bad for a 31k patch. > > So, how does it look? Shall I: > - modify it in any way > - make a proposal > > Given the patch is small I would like to commit it to 2.6.x > as well. > It would also make one of the new WFS 1.1 CITE tests > happy, now they actually test a WFS-T server supports > useExisting and for the moment we just skip that test in > the CITE nightly builds (boy I'm so happy they are there). > > Well, let me know. > > Cheers > Andrea > > -- > Andrea Aime > OpenGeo - http://opengeo.org > Expert service straight from the developers. > diff --git > a/modules/library/api/src/main/java/org/geotools/data/QueryCapabilities.java > b/modules/library/api/src/main/java/org/geotools/data/QueryCapabilities.java > index 9e18937..733336c 100644 > --- > a/modules/library/api/src/main/java/org/geotools/data/QueryCapabilities.java > +++ > b/modules/library/api/src/main/java/org/geotools/data/QueryCapabilities.java > @@ -16,11 +16,13 @@ > */ > package org.geotools.data; > > +import org.geotools.factory.Hints; > +import org.opengis.feature.Feature; > import org.opengis.filter.sort.SortBy; > > /** > * Describes the query capabilities for a specific FeatureType, so client > code can request which > - * features are nativelly supported by a FeatureSource. > + * features are natively supported by a FeatureSource. > * <p> > * This is the minimal Query capabilities we could come up in order to > reliably support paging. Yet, > * the need for a more complete set of capabilities is well known and a new > proposal should be done > @@ -92,4 +94,17 @@ public class QueryCapabilities { > public boolean isReliableFIDSupported() { > return true; > } > + > + /** > + * If true the datastore supports using the provided feature id in the > data insertion > + * workflow as opposed to generating a new id. In that case it will look > into the user data > + * map ({...@link Feature#getUserData()}) for a {...@link > Hints#USE_EXISTING_FID} key associated to a > + * {...@link Boolean#TRUE} value, if the key/value pair is there an > attempt to use the provided > + * id will be made, and the operation will fail of the key cannot be > parsed into a valid > + * storage identifier. > + * @return > + */ > + public boolean isUseExisingFIDSupported() { > + return false; > + } > } > diff --git > a/modules/library/data/src/main/java/org/geotools/data/store/ContentFeatureStore.java > > b/modules/library/data/src/main/java/org/geotools/data/store/ContentFeatureStore.java > index c584a03..e686841 100644 > --- > a/modules/library/data/src/main/java/org/geotools/data/store/ContentFeatureStore.java > +++ > b/modules/library/data/src/main/java/org/geotools/data/store/ContentFeatureStore.java > @@ -35,8 +35,11 @@ import org.geotools.data.LockingManager; > import org.geotools.data.Query; > import org.geotools.data.simple.SimpleFeatureCollection; > import org.geotools.data.simple.SimpleFeatureStore; > +import org.geotools.factory.Hints; > import org.geotools.feature.FeatureCollection; > import org.geotools.feature.NameImpl; > +import org.geotools.feature.simple.SimpleFeatureImpl; > +import org.geotools.filter.identity.FeatureIdImpl; > import org.opengis.feature.simple.SimpleFeature; > import org.opengis.feature.simple.SimpleFeatureType; > import org.opengis.feature.type.AttributeDescriptor; > @@ -190,35 +193,16 @@ public abstract class ContentFeatureStore extends > ContentFeatureSource implement > public final List<FeatureId> addFeatures(Collection collection) > throws IOException { > > - //gather up id's > + // gather up id's > List<FeatureId> ids = new LinkedList<FeatureId>(); > > FeatureWriter<SimpleFeatureType, SimpleFeature> writer = getWriter( > Filter.INCLUDE, WRITER_ADD ); > try { > for ( Iterator f = collection.iterator(); f.hasNext(); ) { > - SimpleFeature feature = (SimpleFeature) f.next(); > - > - // grab next feature and populate it > - // JD: worth a note on how we do this... we take a "pull" > approach > - // because the raw schema we are inserting into may not > match the > - // schema of the features we are inserting > - SimpleFeature toWrite = writer.next(); > - for ( int i = 0; i < toWrite.getAttributeCount(); i++ ) { > - String name = > toWrite.getType().getDescriptor(i).getLocalName(); > - toWrite.setAttribute( name, feature.getAttribute(name)); > - } > - > - //perform the write > - writer.write(); > - > - //add the id to the set of inserted > - ids.add( toWrite.getIdentifier() ); > - > - //copy any metadata from teh feature that was actually > written > - feature.getUserData().putAll( toWrite.getUserData() ); > + FeatureId id = addFeature((SimpleFeature) f.next(), writer); > + ids.add( id ); > } > - } > - finally { > + } finally { > writer.close(); > } > > @@ -233,39 +217,56 @@ public abstract class ContentFeatureStore extends > ContentFeatureSource implement > */ > public final List<FeatureId> > addFeatures(FeatureCollection<SimpleFeatureType,SimpleFeature> collection) > throws IOException { > - //gather up id's > + // gather up id's > List<FeatureId> ids = new LinkedList<FeatureId>(); > > FeatureWriter<SimpleFeatureType, SimpleFeature> writer = getWriter( > Filter.INCLUDE, WRITER_ADD ); > Iterator f = collection.iterator(); > try { > while (f.hasNext()) { > - SimpleFeature feature = (SimpleFeature) f.next(); > - > - // grab next feature and populate it > - // JD: worth a note on how we do this... we take a "pull" > approach > - // because the raw schema we are inserting into may not > match the > - // schema of the features we are inserting > - SimpleFeature toWrite = writer.next(); > - for ( int i = 0; i < toWrite.getAttributeCount(); i++ ) { > - String name = > toWrite.getType().getDescriptor(i).getLocalName(); > - toWrite.setAttribute( name, feature.getAttribute(name)); > - } > - > - //perform the write > - writer.write(); > - > - //add the id to the set of inserted > - ids.add( toWrite.getIdentifier() ); > + FeatureId id = addFeature((SimpleFeature) f.next(), writer); > + ids.add( id ); > } > - } > - finally { > + } finally { > writer.close(); > collection.close( f ); > } > return ids; > } > > + FeatureId addFeature(SimpleFeature feature, > FeatureWriter<SimpleFeatureType, SimpleFeature> writer) throws IOException { > + // grab next feature and populate it > + // JD: worth a note on how we do this... we take a "pull" approach > + // because the raw schema we are inserting into may not match the > + // schema of the features we are inserting > + SimpleFeature toWrite = writer.next(); > + for ( int i = 0; i < toWrite.getAttributeCount(); i++ ) { > + String name = toWrite.getType().getDescriptor(i).getLocalName(); > + toWrite.setAttribute( name, feature.getAttribute(name)); > + } > + > + // copy over the user data > + if(feature.getUserData().size() > 0) { > + toWrite.getUserData().putAll(feature.getUserData()); > + } > + > + // pass through the fid if the user asked so > + boolean useExisting = > Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_EXISTING_FID)); > + if(getQueryCapabilities().isUseExisingFIDSupported() && useExisting) > { > + ((FeatureIdImpl) toWrite.getIdentifier()).setID(feature.getID()); > + } > + > + //perform the write > + writer.write(); > + > + // copy any metadata from the feature that was actually written > + feature.getUserData().putAll( toWrite.getUserData() ); > + > + // add the id to the set of inserted > + FeatureId id = toWrite.getIdentifier(); > + return id; > + } > + > /** > * Sets the feature of the source. > * <p> > @@ -519,4 +520,5 @@ public abstract class ContentFeatureStore extends > ContentFeatureSource implement > reader.close(); > } > } > + > } > diff --git > a/modules/library/jdbc/src/main/java/org/geotools/data/jdbc/JDBCFeatureSource.java > > b/modules/library/jdbc/src/main/java/org/geotools/data/jdbc/JDBCFeatureSource.java > index ed1b0c6..1dddfee 100644 > --- > a/modules/library/jdbc/src/main/java/org/geotools/data/jdbc/JDBCFeatureSource.java > +++ > b/modules/library/jdbc/src/main/java/org/geotools/data/jdbc/JDBCFeatureSource.java > @@ -33,6 +33,7 @@ import org.geotools.data.DataStore; > import org.geotools.data.DefaultQuery; > import org.geotools.data.FeatureListener; > import org.geotools.data.FeatureSource; > +import org.geotools.data.FeatureStore; > import org.geotools.data.Query; > import org.geotools.data.QueryCapabilities; > import org.geotools.data.ResourceInfo; > @@ -226,6 +227,11 @@ public class JDBCFeatureSource implements > SimpleFeatureSource { > > return mapper instanceof NullFIDMapper; > } > + > + @Override > + public boolean isUseExisingFIDSupported() { > + return this instanceof FeatureStore; > + } > } > > /** FeatureType being provided */ > diff --git > a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java > b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java > index d514e51..a008f42 100644 > --- a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java > +++ b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCDataStore.java > @@ -1269,27 +1269,29 @@ public final class JDBCDataStore extends > ContentDataStore > st = cx.createStatement(); > } > > - //figure out if we should determine what the fid is pre or > post insert > + // figure out if we should determine what the fid is pre or > post insert > boolean postInsert = > dialect.lookupGeneratedValuesPostInsert() && isGenerated(key); > > - List<Object> keyValues = null; > - if ( !postInsert ) { > - keyValues = getNextValues( key, cx ); > - } > - > for (Iterator f = features.iterator(); f.hasNext();) { > SimpleFeature feature = (SimpleFeature) f.next(); > + > + List<Object> keyValues = null; > + boolean useExisting = > Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_EXISTING_FID)); > + if(useExisting) { > + keyValues = decodeFID(key, feature.getID(), true); > + } else if (!postInsert) { > + keyValues = getNextValues( key, cx ); > + } > + > > if ( dialect instanceof PreparedStatementSQLDialect ) { > PreparedStatement ps = insertSQLPS( featureType, > feature, keyValues, cx ); > try { > ps.execute(); > - } > - finally { > + } finally { > closeSafe( ps ); > } > - } > - else { > + } else { > String sql = insertSQL(featureType, feature, > keyValues, cx); > LOGGER.log(Level.FINE, "Inserting new feature: {0}", > sql); > > @@ -1530,8 +1532,7 @@ public final class JDBCDataStore extends > ContentDataStore > > try { > FID = URLDecoder.decode(FID,"UTF-8"); > - } > - catch (UnsupportedEncodingException e) { > + } catch (UnsupportedEncodingException e) { > throw new RuntimeException( e ); > } > > @@ -1546,8 +1547,7 @@ public final class JDBCDataStore extends > ContentDataStore > for ( int i = 0; i < split.length; i++ ) { > values.add(split[i]); > } > - } > - else { > + } else { > //single value case > values = new ArrayList(); > values.add( FID ); > @@ -3252,9 +3252,10 @@ public final class JDBCDataStore extends > ContentDataStore > } > > //primary key values > + boolean useExisting = > Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_EXISTING_FID)); > for (PrimaryKeyColumn col : key.getColumns() ) { > //only include if its non auto generating > - if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) ) { > + if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) || > useExisting) { > dialect.encodeColumnName(col.getName(), sql); > sql.append( ","); > } > @@ -3299,11 +3300,12 @@ public final class JDBCDataStore extends > ContentDataStore > > sql.append(","); > } > + // handle the primary key > for ( int i = 0; i < key.getColumns().size(); i++ ) { > PrimaryKeyColumn col = key.getColumns().get( i ); > > //only include if its non auto generating > - if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) ) { > + if (!(col instanceof AutoGeneratedPrimaryKeyColumn ) || > useExisting) { > try { > //Object value = getNextValue(col, key, cx); > Object value = keyValues.get( i ); > @@ -3370,9 +3372,10 @@ public final class JDBCDataStore extends > ContentDataStore > } > > // primary key values > + boolean useExisting = > Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_EXISTING_FID)); > for (PrimaryKeyColumn col : key.getColumns() ) { > //only include if its non auto generating > - if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) ) { > + if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) || > useExisting ) { > dialect.encodeColumnName(col.getName(), sql); > sql.append( ","); > } > @@ -3400,7 +3403,7 @@ public final class JDBCDataStore extends > ContentDataStore > } > for (PrimaryKeyColumn col : key.getColumns() ) { > //only include if its non auto generating > - if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) ) { > + if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) || > useExisting) { > sql.append("?").append( ","); > } > } > @@ -3449,7 +3452,7 @@ public final class JDBCDataStore extends > ContentDataStore > for ( int j = 0; j < key.getColumns().size(); j++ ) { > PrimaryKeyColumn col = key.getColumns().get( j ); > //only include if its non auto generating > - if ( !(col instanceof AutoGeneratedPrimaryKeyColumn ) ) { > + if (!(col instanceof AutoGeneratedPrimaryKeyColumn ) || > useExisting) { > //get the next value for the column > //Object value = getNextValue( col, key, cx ); > Object value = keyValues.get( j ); > diff --git > a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureReader.java > b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureReader.java > index 8867f5a..fea9d85 100644 > --- > a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureReader.java > +++ > b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureReader.java > @@ -37,6 +37,7 @@ import java.util.logging.Logger; > import org.geotools.data.DefaultQuery; > import org.geotools.data.FeatureReader; > import org.geotools.data.Transaction; > +import org.geotools.data.store.ContentFeatureStore; > import org.geotools.factory.Hints; > import org.geotools.feature.IllegalAttributeException; > import org.geotools.feature.simple.SimpleFeatureBuilder; > diff --git > a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCQueryCapabilities.java > > b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCQueryCapabilities.java > index 840ae19..ebf362e 100644 > --- > a/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCQueryCapabilities.java > +++ > b/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCQueryCapabilities.java > @@ -16,6 +16,7 @@ > */ > package org.geotools.jdbc; > > +import org.geotools.data.FeatureStore; > import org.geotools.data.QueryCapabilities; > import org.opengis.feature.type.AttributeDescriptor; > import org.opengis.filter.expression.PropertyName; > @@ -98,4 +99,9 @@ class JDBCQueryCapabilities extends QueryCapabilities { > public boolean isOffsetSupported() { > return source.getDataStore().getSQLDialect().isLimitOffsetSupported(); > } > + > + @Override > + public boolean isUseExisingFIDSupported() { > + return true; > + } > } > diff --git > a/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCFeatureStoreTest.java > > b/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCFeatureStoreTest.java > index efedb75..a365b07 100644 > --- > a/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCFeatureStoreTest.java > +++ > b/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCFeatureStoreTest.java > @@ -30,6 +30,7 @@ import org.geotools.data.Transaction; > import org.geotools.data.FeatureEvent.Type; > import org.geotools.data.simple.SimpleFeatureCollection; > import org.geotools.factory.CommonFactoryFinder; > +import org.geotools.factory.Hints; > import org.geotools.feature.AttributeTypeBuilder; > import org.geotools.feature.DefaultFeatureCollection; > import org.geotools.feature.simple.SimpleFeatureBuilder; > @@ -99,6 +100,43 @@ public abstract class JDBCFeatureStoreTest extends > JDBCTestSupport { > } > } > > + public void testAddFeaturesUseExisting() throws IOException { > + // check we advertise the ability to reuse feature ids > + > assertTrue(featureStore.getQueryCapabilities().isUseExisingFIDSupported()); > + > + SimpleFeatureBuilder b = new > SimpleFeatureBuilder(featureStore.getSchema()); > + DefaultFeatureCollection collection = new > DefaultFeatureCollection(null, > + featureStore.getSchema()); > + > + String typeName = b.getFeatureType().getTypeName(); > + for (int i = 3; i < 6; i++) { > + b.set(aname("intProperty"), new Integer(i)); > + b.set(aname("geometry"), new GeometryFactory().createPoint(new > Coordinate(i, i))); > + b.featureUserData(Hints.USE_EXISTING_FID, Boolean.TRUE); > + collection.add(b.buildFeature(typeName + "." + (i * 10))); > + } > + List<FeatureId> fids = featureStore.addFeatures(collection); > + > + assertEquals(3, fids.size()); > + > assertTrue(fids.contains(SimpleFeatureBuilder.createDefaultFeatureIdentifier(typeName > + ".30"))); > + > assertTrue(fids.contains(SimpleFeatureBuilder.createDefaultFeatureIdentifier(typeName > + ".40"))); > + > assertTrue(fids.contains(SimpleFeatureBuilder.createDefaultFeatureIdentifier(typeName > + ".50"))); > + > + SimpleFeatureCollection features = featureStore.getFeatures(); > + assertEquals(6, features.size()); > + > + FilterFactory ff = dataStore.getFilterFactory(); > + > + for (Iterator f = fids.iterator(); f.hasNext();) { > + FeatureId identifier = (FeatureId) f.next(); > + String fid = identifier.getID(); > + Id filter = ff.id(Collections.singleton(identifier)); > + > + features = featureStore.getFeatures(filter); > + assertEquals(1, features.size()); > + } > + } > + > public void testAddInTransaction() throws IOException { > SimpleFeatureBuilder b = new > SimpleFeatureBuilder(featureStore.getSchema()); > DefaultFeatureCollection collection = new > DefaultFeatureCollection(null, > diff --git > a/modules/library/main/src/main/java/org/geotools/data/AbstractFeatureStore.java > > b/modules/library/main/src/main/java/org/geotools/data/AbstractFeatureStore.java > index 7a9399e..87d0787 100644 > --- > a/modules/library/main/src/main/java/org/geotools/data/AbstractFeatureStore.java > +++ > b/modules/library/main/src/main/java/org/geotools/data/AbstractFeatureStore.java > @@ -24,9 +24,11 @@ import java.util.List; > import java.util.Set; > > import org.geotools.data.simple.SimpleFeatureStore; > +import org.geotools.factory.Hints; > import org.geotools.feature.FeatureCollection; > import org.geotools.feature.IllegalAttributeException; > import org.geotools.feature.NameImpl; > +import org.geotools.filter.identity.FeatureIdImpl; > import org.opengis.feature.simple.SimpleFeature; > import org.opengis.feature.simple.SimpleFeatureType; > import org.opengis.feature.type.AttributeDescriptor; > @@ -271,6 +273,11 @@ public abstract class AbstractFeatureStore extends > AbstractFeatureSource > + typeName + " out of provided feature: " > + feature.getID(), writeProblem); > } > + > + boolean useExisting = > Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_EXISTING_FID)); > + if(getQueryCapabilities().isUseExisingFIDSupported() && > useExisting) { > + ((FeatureIdImpl) > newFeature.getIdentifier()).setID(feature.getID()); > + } > > writer.write(); > addedFids.add(newFeature.getID()); > @@ -305,6 +312,11 @@ public abstract class AbstractFeatureStore extends > AbstractFeatureSource > + typeName + " out of provided feature: " > + feature.getID(), writeProblem); > } > + > + boolean useExisting = > Boolean.TRUE.equals(feature.getUserData().get(Hints.USE_EXISTING_FID)); > + if(getQueryCapabilities().isUseExisingFIDSupported() && > useExisting) { > + ((FeatureIdImpl) > newFeature.getIdentifier()).setID(feature.getID()); > + } > > writer.write(); > addedFids.add(newFeature.getIdentifier()); > diff --git > a/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureBuilder.java > > b/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureBuilder.java > index b5cd631..8865848 100644 > --- > a/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureBuilder.java > +++ > b/modules/library/main/src/main/java/org/geotools/feature/simple/SimpleFeatureBuilder.java > @@ -152,6 +152,8 @@ public class SimpleFeatureBuilder { > > Map<Object, Object>[] userData; > > + Map<Object, Object> featureUserData; > + > boolean validating; > > public SimpleFeatureBuilder(SimpleFeatureType featureType) { > @@ -174,6 +176,7 @@ public class SimpleFeatureBuilder { > values = new Object[featureType.getAttributeCount()]; > next = 0; > userData = null; > + featureUserData = null; > } > > /** > @@ -341,10 +344,11 @@ public class SimpleFeatureBuilder { > > Object[] values = this.values; > Map<Object,Object>[] userData = this.userData; > + Map<Object,Object> featureUserData = this.featureUserData; > reset(); > SimpleFeature sf = factory.createSimpleFeature(values, featureType, > id); > > - // handle the user data > + // handle the per attribute user data > if(userData != null) { > for (int i = 0; i < userData.length; i++) { > if(userData[i] != null) { > @@ -353,6 +357,11 @@ public class SimpleFeatureBuilder { > } > } > > + // handle the feature wide user data > + if(featureUserData != null) { > + sf.getUserData().putAll(featureUserData); > + } > + > return sf; > } > > @@ -557,6 +566,21 @@ public class SimpleFeatureBuilder { > return this; > } > > + /** > + * Sets a feature wide use data key/value pair. The user data map is > reset > + * when the feature is built > + * @param key > + * @param value > + * @return > + */ > + public SimpleFeatureBuilder featureUserData(Object key, Object value) { > + if(featureUserData == null) { > + featureUserData = new HashMap<Object, Object>(); > + } > + featureUserData.put(key, value); > + return this; > + } > + > public boolean isValidating() { > return validating; > } > diff --git > a/modules/library/main/src/main/java/org/geotools/filter/identity/FeatureIdImpl.java > > b/modules/library/main/src/main/java/org/geotools/filter/identity/FeatureIdImpl.java > index aff1cee..0531d08 100644 > --- > a/modules/library/main/src/main/java/org/geotools/filter/identity/FeatureIdImpl.java > +++ > b/modules/library/main/src/main/java/org/geotools/filter/identity/FeatureIdImpl.java > @@ -48,17 +48,15 @@ public class FeatureIdImpl implements FeatureId { > } > > public void setID( String id ){ > - if ( fid == null ) { > + if ( id == null ) { > throw new NullPointerException( "fid must not be null" > ); > - } > + } > if( origionalFid == null ){ > - origionalFid = fid; > - fid = id; > - } > - else { > - throw new IllegalStateException("You can only assign a > real id once during a commit"); > - } > + origionalFid = fid; > + } > + fid = id; > } > + > public boolean matches(Feature feature) { > return feature != null && fid.equals( > feature.getIdentifier().getID() ); > } > diff --git > a/modules/library/metadata/src/main/java/org/geotools/factory/Hints.java > b/modules/library/metadata/src/main/java/org/geotools/factory/Hints.java > index cb69275..34f6226 100644 > --- a/modules/library/metadata/src/main/java/org/geotools/factory/Hints.java > +++ b/modules/library/metadata/src/main/java/org/geotools/factory/Hints.java > @@ -367,7 +367,18 @@ public class Hints extends RenderingHints { > * @since 2.4 > */ > public static final Key VERSION = new Key("org.geotools.util.Version"); > + > + > > + /** > + * When this key is used in the user data section of a feature and the > feature store > + * query capabilities reports being able to use provided feature ids the > store will > + * try to use the provided feature id during insertion, and will fail if > the FID > + * cannot be parsed into a valid storage identifier > + * > + * @since 2.7 > + */ > + public static final Key USE_EXISTING_FID = new > Key("org.geotools.fidPolicy.UseExisting"); > > > //////////////////////////////////////////////////////////////////////// > diff --git > a/modules/plugin/jdbc/jdbc-postgis/src/test/java/org/geotools/data/postgis/PostgisFeatureStoreTest.java > > b/modules/plugin/jdbc/jdbc-postgis/src/test/java/org/geotools/data/postgis/PostgisFeatureStoreTest.java > index 8585045..6941634 100644 > --- > a/modules/plugin/jdbc/jdbc-postgis/src/test/java/org/geotools/data/postgis/PostgisFeatureStoreTest.java > +++ > b/modules/plugin/jdbc/jdbc-postgis/src/test/java/org/geotools/data/postgis/PostgisFeatureStoreTest.java > @@ -16,8 +16,6 @@ > */ > package org.geotools.data.postgis; > > -import java.io.IOException; > - > import org.geotools.jdbc.JDBCFeatureStoreTest; > import org.geotools.jdbc.JDBCTestSetup; > > diff --git > a/modules/plugin/property/src/main/java/org/geotools/data/property/PropertyFeatureSource.java > > b/modules/plugin/property/src/main/java/org/geotools/data/property/PropertyFeatureSource.java > index 635698b..350d39d 100644 > --- > a/modules/plugin/property/src/main/java/org/geotools/data/property/PropertyFeatureSource.java > +++ > b/modules/plugin/property/src/main/java/org/geotools/data/property/PropertyFeatureSource.java > @@ -26,6 +26,7 @@ import org.geotools.data.DataStore; > import org.geotools.data.FeatureEvent; > import org.geotools.data.FeatureListener; > import org.geotools.data.Query; > +import org.geotools.data.QueryCapabilities; > import org.geotools.data.Transaction; > import org.geotools.geometry.jts.ReferencedEnvelope; > import org.opengis.feature.simple.SimpleFeatureType; > @@ -58,8 +59,15 @@ public class PropertyFeatureSource extends > AbstractFeatureLocking { > } > cacheCount = -1; > } > - }); > + }); > + this.queryCapabilities = new QueryCapabilities() { > + @Override > + public boolean isUseExisingFIDSupported() { > + return true; > + } > + }; > } > + > public DataStore getDataStore() { > return store; > } > diff --git > a/modules/plugin/property/src/test/java/org/geotools/data/property/PropertyDataStoreTest.java > > b/modules/plugin/property/src/test/java/org/geotools/data/property/PropertyDataStoreTest.java > index 9f8dff5..c3752ec 100644 > --- > a/modules/plugin/property/src/test/java/org/geotools/data/property/PropertyDataStoreTest.java > +++ > b/modules/plugin/property/src/test/java/org/geotools/data/property/PropertyDataStoreTest.java > @@ -25,6 +25,7 @@ import java.util.Arrays; > import java.util.Collections; > import java.util.List; > import java.util.NoSuchElementException; > +import java.util.Set; > > import junit.framework.TestCase; > > @@ -40,13 +41,16 @@ import org.geotools.data.simple.SimpleFeatureIterator; > import org.geotools.data.simple.SimpleFeatureSource; > import org.geotools.data.simple.SimpleFeatureStore; > import org.geotools.factory.CommonFactoryFinder; > +import org.geotools.factory.Hints; > import org.geotools.feature.IllegalAttributeException; > import org.geotools.feature.simple.SimpleFeatureBuilder; > +import org.geotools.filter.identity.FeatureIdImpl; > import org.opengis.feature.simple.SimpleFeature; > import org.opengis.feature.simple.SimpleFeatureType; > import org.opengis.feature.type.AttributeDescriptor; > import org.opengis.filter.Filter; > import org.opengis.filter.FilterFactory2; > +import org.opengis.filter.identity.FeatureId; > > /** > * Test functioning of PropertyDataStore. > @@ -431,4 +435,25 @@ public class PropertyDataStoreTest extends TestCase { > assertEquals( "client 1 after client 2 commits addition of fid5 (fid1 > previously removed)", 4, roadFromClient1.getFeatures().size() ); > assertEquals( "client 2 after commiting addition of fid5 (fid1 > previously removed)", 4, roadFromClient2.getFeatures().size() ); > } > + > + public void testUseExistingFid() throws Exception { > + SimpleFeatureType ROAD = store.getSchema( "road" ); > + SimpleFeature chrisFeature = SimpleFeatureBuilder.build(ROAD, new > Object[]{ new Integer(5), "chris"}, "fid5" ); > + chrisFeature.getUserData().put(Hints.USE_EXISTING_FID, Boolean.TRUE); > + > + SimpleFeatureStore roadAuto = (SimpleFeatureStore) > store.getFeatureSource("road"); > + List<FeatureId> fids = > roadAuto.addFeatures(DataUtilities.collection(chrisFeature)); > + > + // checke the id was preserved > + assertEquals(1, fids.size()); > + FeatureId fid = > SimpleFeatureBuilder.createDefaultFeatureIdentifier("fid5"); > + assertTrue(fids.contains(fid)); > + > + // manually check the feature with the proper id is actually there > + SimpleFeatureIterator it = > roadAuto.getFeatures(ff.id(Collections.singleton(fid))).features(); > + assertTrue(it.hasNext()); > + SimpleFeature sf = it.next(); > + it.close(); > + assertEquals(fid, sf.getIdentifier()); > + } > } > ------------------------------------------------------------------------------ > > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel ------------------------------------------------------------------------------ _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
