eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[editors] File Save conflict can lose content #2179

Open eclipse-ocl-bot opened 3 days ago

eclipse-ocl-bot commented 3 days ago

| --- | --- | | Bugzilla Link | 573916 | | Status | NEW | | Importance | P3 major | | Reported | Jun 02, 2021 04:24 EDT | | Modified | Jun 05, 2021 15:18 EDT | | Depends on | 573923 | | See also | 573982 | | Reporter | Ed Willink |

Description

While pursuing the OCLinEcoreTutorial Tutorial.ecore was open in the OCLinEcore editor.

I experimented with some Sample Ecore Edits on Tutorial.ecore. The edits were probably aborted, but ...

After adding the Book invariant, the OclInEcore editpr file save failed - popup dialog 'says no'. Popup should offer overwrite.

Atgtemping to save as Tutorial2.ecore instead resulted in an editor on the empty Tutorial2.ecore with nothing saved to disk.

MAJOR since a user edit was lost.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 02, 2021 06:00

The start of the problem would appear to be that an edit and Save of Tutorial.xmi marks Tutorial.ecore dirty wrt the OclInEcore editor.

Console log shows

java.lang.AssertionError\ at org.eclipse.ocl.xtext.base.ui.model.BaseDocument.setInput(BaseDocument.java:295)\ at org.eclipse.xtext.ui.editor.model.XtextDocumentProvider.setDocumentResource(XtextDocumentProvider.java:213)\ at org.eclipse.ocl.xtext.base.ui.model.BaseCSorASDocumentProvider.setDocumentResource(BaseCSorASDocumentProvider.java:553)\ at org.eclipse.xtext.ui.editor.model.XtextDocumentProvider.setDocumentContent(XtextDocumentProvider.java:195)\ at org.eclipse.ocl.xtext.base.ui.model.BaseDocumentProvider.setDocumentContent(BaseDocumentProvider.java:101)\ at org.eclipse.ocl.xtext.base.ui.model.BaseCSorASDocumentProvider.setDocumentContent(BaseCSorASDocumentProvider.java:333)\ at org.eclipse.ocl.xtext.base.ui.model.BaseCSorASDocumentProvider.handleElementContentChanged(BaseCSorASDocumentProvider.java:236)\ at org.eclipse.ui.editors.text.FileDocumentProvider$FileSynchronizer$2.execute(FileDocumentProvider.java:253)\ at org.eclipse.ui.editors.text.FileDocumentProvider$SafeChange.run(FileDocumentProvider.java:143)\ at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)\ at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185)\ at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4001) ...\ at org.eclipse.ui.internal.SaveableHelper.savePart(SaveableHelper.java:160)\ at org.eclipse.ui.internal.WorkbenchPage.saveSaveable(WorkbenchPage.java:3820)\ at org.eclipse.ui.internal.WorkbenchPage.saveEditor(WorkbenchPage.java:3833)

where line 295 is the last line of

public void setInput(XtextResource resource) {\
    EnvironmentFactoryInternal environmentFactory = ThreadLocalExecutor.basicGetEnvironmentFactory();\
    assert environmentFactory != null;
eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 02, 2021 06:16

Minimum repro:

Open Tutorial.ecore with OCLinEcore editor\ Open Tutorial.xmi (with Sample Reflective Ecore Model Editor)\ For Tutorial.ecore, edit "date" to "dates" and Save\ For Tutorial.xmi, edit first loan from b1 to b2, and Save\ --- Tutorial.ecore is now dirty

The above digs the unpleasant deep hole, although it violates the no editing in-use metamodels warning in the tutorial.

Thereafter, after use of "Ignore file change" in the OCLinEcore editor, further edits are unsaveable. Then a Save AS fails with an OperationCanceledException popup.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 02, 2021 08:48

(In reply to Ed Willink from comment #2)

For Tutorial.xmi, edit first loan from b1 to b2, and Save --- Tutorial.ecore is now dirty

The redundant Tutorial.ecore save occurs because the save options from the Sample Ecore Model Editor and OCLinEcore editor differ folling the OPTION_SAVE_ONLY_IF_CHANGED_MEMORY_BUFFER to thing it changed.


java.lang.AssertionError at org.eclipse.ocl.xtext.base.ui.model.BaseDocument.setInput(BaseDocument.java:295)

Currently setInput handles the initial case, EnvironmentFactory primed prior to setInput so that the BaseDocumentLocker can be configured.

This problem occurs when a re-setInput occurs as a consequence of listeing to some other part, consequently this part has no EnvironmentFactory. If BaseDocumentLocker is already initialized we should stick with it.

(But beware Console's editor must chop and change.)

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 02, 2021 09:00

(In reply to Ed Willink from comment #3)

The redundant Tutorial.ecore save occurs because the save options from the Sample Ecore Model Editor and OCLinEcore editor differ folling the OPTION_SAVE_ONLY_IF_CHANGED_MEMORY_BUFFER to thing it changed.

Moved to Bug 573923.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 02, 2021 09:17

(In reply to Ed Willink from comment #3)

java.lang.AssertionError at org.eclipse.ocl.xtext.base.ui.model.BaseDocument.setInput(BaseDocument.java: 295)

Simpler repro

Open Tutorial.ecore with OCLinEcore editor.\ Open Tutorial.ecore with Text editor.\ edit and save using Text editor ... assertion failure occurs

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 03, 2021 08:35

(In reply to Ed Willink from comment #5)

Simpler repro

Open Tutorial.ecore with OCLinEcore editor. Open Tutorial.ecore with Text editor. edit and save using Text editor ... assertion failure occurs

This repro also triggers Bug 573982 when the new serialized ecore is shorter than th previous serialized ecore.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 05, 2021 04:18

The related fixes leave just the following reproducible problem

Open Tutorial.ecore with OCLinEcore editor\ Open Tutorial.ecore with text editor\ Edit in OCLinEcore editor\ Edit in text editor\ Save in text editor\ Activate OCLinEcore editor\ File Change pop up appears\ (- selecting Replace Edotror Content works fine)\ Select Ignore File Change - ok\ Save in OCLinEcore editor - fails\ Save OCLinEcore editor As Tutorial2.ecore

a) the save fail should have offered to overwrite\ b) the save as shoud have worled and not wiped the content

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 05, 2021 04:21

(In reply to Ed Willink from comment #7)

b) the save as should have worked and not wiped the content

java.lang.AssertionError\ at org.eclipse.ocl.xtext.base.ui.model.BaseDocument$BaseDocumentLocker.readOnly(BaseDocument.java:112)\ at org.eclipse.xtext.ui.editor.model.XtextDocument.readOnly(XtextDocument.java:136)\ at org.eclipse.xtext.util.concurrent.IReadAccess.tryReadOnly(IReadAccess.java:50)\ at org.eclipse.xtext.ui.editor.folding.DefaultFoldingRegionProvider.getFoldingRegions(DefaultFoldingRegionProvider.java:74)\ at org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.calculateProjectionAnnotationModel(DefaultFoldingStructureProvider.java:132)\ at org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.initialize(DefaultFoldingStructureProvider.java:67)\ at org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider.handleProjectionEnabled(DefaultFoldingStructureProvider.java:111)\ at org.eclipse.xtext.ui.editor.folding.DefaultFoldingStructureProvider$ProjectionChangeListener.projectionEnabled(DefaultFoldingStructureProvider.java:245)\ at org.eclipse.jface.text.source.projection.ProjectionViewer.fireProjectionEnabled(ProjectionViewer.java:475)\ at org.eclipse.jface.text.source.projection.ProjectionViewer.enableProjection(ProjectionViewer.java:523)\ at org.eclipse.jface.text.source.projection.ProjectionViewer.setDocument(ProjectionViewer.java:371)\ at org.eclipse.jface.text.source.SourceViewer.setDocument(SourceViewer.java:613)\ at org.eclipse.ui.texteditor.AbstractTextEditor.initializeSourceViewer(AbstractTextEditor.java:3985)\ at org.eclipse.ui.texteditor.AbstractTextEditor.doSetInput(AbstractTextEditor.java:4187)\ at org.eclipse.ui.texteditor.StatusTextEditor.doSetInput(StatusTextEditor.java:262)\ at org.eclipse.ui.texteditor.AbstractDecoratedTextEditor.doSetInput(AbstractDecoratedTextEditor.java:1479)\ at org.eclipse.ui.editors.text.TextEditor.doSetInput(TextEditor.java:153)\ at org.eclipse.xtext.ui.editor.XtextEditor.doSetInput(XtextEditor.java:261)\ at org.eclipse.ui.texteditor.AbstractTextEditor.setInputWithNotify(AbstractTextEditor.java:4243)\ at org.eclipse.ui.texteditor.AbstractTextEditor.setInput(AbstractTextEditor.java:4263)\ at org.eclipse.ui.texteditor.AbstractDecoratedTextEditor.performSaveAs(AbstractDecoratedTextEditor.java:1624)\ at org.eclipse.ui.texteditor.AbstractTextEditor.doSaveAs(AbstractTextEditor.java:4756)\ at org.eclipse.xtext.ui.editor.XtextEditor.doSaveAs(XtextEditor.java:320)

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 05, 2021 04:34

(In reply to Ed Willink from comment #8)

java.lang.AssertionError at org.eclipse.ocl.xtext.base.ui.model.BaseDocument$BaseDocumentLocker. readOnly(BaseDocument.java:112)

The save as failure and empty content is due to an unsound assert, so not a user problem. Easy fixed by a guard adjustment.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 05, 2021 04:53

(In reply to Ed Willink from comment #7)

a) the save fail should have offered to overwrite

The OCLinEcoreDocumentProvider.doSaveDocument has an overwrite option that seems to be largely ignored. Examine all super-implementations and all callers.

Perhaps AbstractDocumentProvider.saveDocument().SaveOperation holds the clues.

eclipse-ocl-bot commented 3 days ago

By Ed Willink on Jun 05, 2021 15:18

Bad asserts and corruptions fixed for 2021-06RC2

Overwrite fix still to do.