[Qgis-developer] Vector buffer commitChanges (was Interaction between QGis and ArcGis)

[Radim Blazek]

On Sun, Apr 21, 2013 at 11:41 AM, Radim Blazek <radim.blazek at gmail.com> wrote:
> On Sun, Apr 21, 2013 at 9:34 AM, Martin Dobias <wonder.sk at gmail.com> wrote:
>> Hi Radim
>>
>> On Sat, Apr 20, 2013 at 8:39 AM, Radim Blazek <radim.blazek at gmail.com> wrote:
>>> On Fri, Apr 19, 2013 at 12:07 PM, JVerholle <julien.verholle at eaurmc.fr> wrote:
>>>> Hi all,
>>>>
>>>> If I want to open the same shapefile in both softwares at the same time, the
>>>> file is locked in ArcGis (even if the edition mode isn't used in QGis).
>>>
>>> QGIS opens layers (files) in update mode ("r+") if possible (file
>>> permission + OGR driver support). It may (but also may not) be a
>>> problem also for #6448 (slow shp over network) I am currently
>>> struggling with. In general, I think that it is bad to open files
>>> always in update mode even if in most cases they are not going to be
>>> edited.
>>
>> Agreed.
>>
>>> Currently there are no QgsVectorDataProvider::startEditing() and
>>> commitChanges() which may also be a problem for database providers
>>> because QgsVectorLayerEditBuffer::commitChanges() calls more provider
>>> methods changing
>>> data (deleteAttributes, addAttributes, deleteFeatures,
>>> addFeatures...).  The commitChanges() should do everything in one
>>> transaction IMO.
>>>
>>> My proposal is to:
>>> 1) Add QgsVectorDataProvider::startEditing() and
>>> QgsVectorDataProvider::commitChanges()
>>> 2) In OGR provider try to open layer in update mode only to get
>>> capabilities (get info if it can be modified when the provider is
>>> constructed) but then to reopen in read only mode.
>>> 3) Call QgsVectorDataProvider::startEditing() from
>>> QgsVectorLayer::startEditing() to be sure that the layer is still
>>> editable (permissions could change or it was opened by another
>>> application for editing since the layer was opened) and to reopen
>>> files in update mode (files based) or to start transaction (DB based).
>>> 4) Call QgsVectorDataProvider::commitChanges() from
>>> QgsVectorLayer::commitChanges()
>>> 4) In QgsOgrProvider::startEditing() reopen the layer in update mode
>>> 5) In QgsOgrProvider::commitChanges() reopen the layer in read only mode
>>
>> When would be startEditing() called - when user starts editing in GUI
>> or when user is going to commit the changes, just before issuing
>> provider editing functions?
>
> When user starts editing (Toggle Editing), that should ensure
> possibility of later commit (provider will keep files in update mode
> after startEditing()). startEditing() may also fail (if permissions
> changed), it would be bad to let user do editing which could not be
> commited. Opening a transaction in DB should ensure that commit will
> be done on the same data which were edited.

Well, I have reconsidered this and it may be better to call
QgsVectorDataProvider::startEditing() from
QgsVectorLayer::commitChanges(). It is not probable that permissions
changes when the layer is opened. It is more probable that user
decides to cancel changes, this way we avoid not necessary reopening
of files.

It should not be important for API and implementation in providers
where we call it from vector layer, we can move it around in the
vector layer later.

>> Maybe beginTransaction() and endTransaction() or commitTransaction()
>> would be better names?
>
> Maybe better, but we are already using startEditing() and
> commitChanges() in vector layer so I would prefer to use the same.

I added to QgsVectorDataProvider:

virtual bool startEditing();
virtual bool rollBack();
virtual bool commitChanges();
virtual bool isEdited();
signals:
  void editingStarted();
  void editingStopped();

As I said, to keep names consistent with vector layer. Currently not
implemented nor used. It would be useful to have it implemented for
2.0 at least in ogr provider to resolve #6448 at least for reading.

Currently all editing methods in postgres provider do changes in
transactions. If we implement QgsPostgresProvider::startEditing() it
should start new transaction and all changes should be done within
that transaction. Are there any potential problems?

Should the startEditing() / commitChanges() close all iterators?

What transaction should use QgsPostgresProvider::getFeatures()? That
opened for editing or another one?

Radim

> endTransaction() for rollback?
>
>> It would be practical to have also another
>> method to detect whether the provider is in editing mode.
>
> Yes and startEditing() should return false it a transaction is already opened.
>
>> What about legacy code that directly makes changes to
>> QgsVectorDataProvider? Will it need to wrap the editing functions with
>> these two additional calls? It would be good if that would not be
>> necessary and the provider would start and stop the transaction
>> automatically if not in editing mode.
>
> It can probably start automatically but when it should commit? It
> should not commit changes automatically in destructor, not commited
> changes should be lost if supported by provider (DB providers) . File
> based providers will do changes immediately, no rollback on provider
> level,  but still there will be rollback on layer level.
>
>> Regarding 2) is it necessary to open layer in update mode in order to
>> get editing capabilities?
>
> I think so, OGR_L_TestCapability works with opened layer and
> OGR_DS_TestCapability can only test ODsCCreateLayer and
> ODsCDeleteLayer.
>
> Radim
>
>>> We don't really have to implement startEditing() and commitChanges()
>>> in all providers for 2.0, but it should be in the vector API.
>>
>> Agreed.
>>
>> Regards
>> Martin