Insert transaction doesn't return id of features in the response

[nsavard]

An Insert transaction could include in the response the Insert Result? section with the fid of the new features (I pasted the relevant XSD section below) (could because: minOccurs="0" maxOccurs="unbounded"). The actual version of Tinyows returns an fid named "id" if the "id" element is specified in the request. For example <feature:id>t0008</feature:id> in the request will be output as <wfs:InsertResults><wfs:Feature><ogc:FeatureId fid="layername.t0008"/></wfs:Feature></wfs:InsertResults>. The Insert Results? section is empty otherwise.

Open Layer? among others needs the fid of the newly inserted feature eventhough no "id" element is included in the request. We have to automate that in some sort.

<xsd:element name="Insert Result?"

type="wfs:InsertResultType" minOccurs="0" maxOccurs="unbounded">

<xsd:annotation>

<xsd:documentation>

The Insert Result? element contains a list of ogc:FeatureId elements that identify any newly created feature instances.

</xsd:documentation>

</xsd:annotation>

</xsd:element>

[nsavard]

I propose the following solution. Check if an id is provided in the request. If yes, use this id in the response. Otherwise retrieve the primary key of the newly inserted element in this layer. I'll add a patch for this.

[nsavard]

patch to fix insert transaction response

[ol]

I've a look at the WFS 1.1.0 spec (p66-68)

Indeed the default behaviour (from the spec) should be the "idgen=Generate New?" and this one seems not implemented in current TinyOWS. Only the 'Use Existing?' one is there.

I think we should implement the whole idgen behaviour, according to WFS spec: - Generate New? - Use Existing? - Replace Duplicate?

[nsavard]

Olivier: I'check that.

[nsavard]

Finally I came with a solution. Comments are welcome.

Modify wfs_insert_xml() 1) Define fid as an array with the layer name as the key

2) Loop through all insert properties to retrieve the handle and the idgen values (if it exists)

3) Set fid[layername] to -1 in the "insert elements layer by layer" for loop (existing loop)

4) Loop (existing for loop) through all fields and get values, if this is an "id" field:

if id use != 0:

if field = id:

retrieve id field value

if idgen exists and idgen != Generate New?:

check if id exists in database

if id exists:

if idgen = Use Existing?

throw exception return

else

if fid[layername] = -1:

get last id in the database for this specific layer and put it in "fid[layername]" variable

id = fid[layername] + 1 fid[layername] = id //keep track of fid

put "id" value in the "sql" statement

else:

retrieve field value and put it in the "sql" statement

Modify wfs_execute_transaction()

if command = insert:

Loop through all results

add id to insert_results // wfs_request structure

execute the transaction

[nsavard]

This comment is exactly the same as comment:4 except for the formating.

Finally I came with a solution. Comments are welcome.

Modify wfs_insert_xml() 1) Define fid as an array with the layer name as the key

2) Loop through all insert properties to retrieve the handle and the idgen values (if it exists)

3) Set fid[layername] to -1 in the "insert elements layer by layer" for loop (existing loop)

4) Loop (existing for loop) through all fields and get values, if this is an "id" field:

   if id use != 0:
     if field = id:
       retrieve id field value

       if idgen exists and idgen != GenerateNew:
         check if id exists in database

         if id exists:
           if idgen = UseExisting
             throw exception
             return
           else
             if fid[layername] = -1:
               get last id in the database for this specific layer and put it in "fid[layername]" variable                

             id = fid[layername] + 1
             fid[layername] = id //keep track of fid

           put "id" value in the "sql" statement

     else:
       retrieve field value and put it in the "sql" statement
   

Modify wfs_execute_transaction()

if command = insert:
  Loop through all results
    add id to insert_results // wfs_request structure


execute the transaction

[ol]

Improved with r127 r128 and r129. Still need to implement a decent random id generator to avoid collision. Id Gen? is now implemented

[nsavard]

Olivier: I don't think idgen = Replace Duplicate? means delete the existing feature in the database that has the same id as the one in the insert request. Of my understanding of the OGC 04-094 table 3 of the specification document, it means more to replace the id of the request with a new generated id.

Next, I don't think we need to generate a random id when idgen = Generatenew or when idgen = Replace Duplicate?. For my part I would take the id of the last feature in the layer and increment it by one. I think it will be simpler to manage.

[nsavard]

Existing code on how to fix this issue

[ol]

Quite a few things have been done:

• Use id generator based on PostgreSQL sequence (serial) or /dev/random (Unix system) or srand/rand (poor solution)
• Improve/fix ReplaceDuplicate (cf Normand input) behaviour
• Fix GetCapabilites and TransactionResponse stream

Still to do:

• With ReplaceDuplicate 2 uses cases are still KO
  • if we have 2 (or more) same id into the same handle on the same table
  • if we have 2 (or more) concurent acces on the same id (need to play with Lock i guess)
• Add specific units tests:
  • with PK serial
  • with PK varchar
  • without PK
  • with other PK type ?
  • several idgen scenarii
  • ...