Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Fixed: MergeDelta special case transformation throws when second oper…
Browse files Browse the repository at this point in the history
…ation is NoOperation.
  • Loading branch information
scofalik committed Aug 24, 2017
1 parent b7cc243 commit 0e99fc5
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/model/delta/basic-transformations.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ addTransformationCase( AttributeDelta, SplitDelta, ( a, b, context ) => {

// Add special case for InsertDelta x MergeDelta transformation.
addTransformationCase( InsertDelta, MergeDelta, ( a, b, context ) => {
// Do not apply special transformation case if `MergeDelta` has `NoOperation` as the second operation.
if ( !b.position ) {
return defaultTransform( a, b, context );
}

const undoMode = context.aWasUndone || context.bWasUndone;

// If insert is applied at the same position where merge happened, we reverse the merge (we treat it like it
Expand Down Expand Up @@ -136,10 +141,14 @@ addTransformationCase( MarkerDelta, RenameDelta, transformMarkerDelta );

// Add special case for MoveDelta x MergeDelta transformation.
addTransformationCase( MoveDelta, MergeDelta, ( a, b, context ) => {
// Do not apply special transformation case if `MergeDelta` has `NoOperation` as the second operation.
if ( !b.position ) {
return defaultTransform( a, b, context );
}

// If move delta is supposed to move a node that has been merged, we reverse the merge (we treat it like it
// didn't happen) and then apply the original move operation. This is "mirrored" in MergeDelta x MoveDelta
// transformation below, where we simply do not apply MergeDelta.

const operateInSameParent =
a.sourcePosition.root == b.position.root &&
compareArrays( a.sourcePosition.getParentPath(), b.position.getParentPath() ) === 'same';
Expand All @@ -158,6 +167,11 @@ addTransformationCase( MoveDelta, MergeDelta, ( a, b, context ) => {

// Add special case for MergeDelta x InsertDelta transformation.
addTransformationCase( MergeDelta, InsertDelta, ( a, b, context ) => {
// Do not apply special transformation case if `MergeDelta` has `NoOperation` as the second operation.
if ( !a.position ) {
return defaultTransform( a, b, context );
}

const undoMode = context.aWasUndone || context.bWasUndone;

// If merge is applied at the same position where we inserted a range of nodes we cancel the merge as it's results
Expand All @@ -171,9 +185,13 @@ addTransformationCase( MergeDelta, InsertDelta, ( a, b, context ) => {

// Add special case for MergeDelta x MoveDelta transformation.
addTransformationCase( MergeDelta, MoveDelta, ( a, b, context ) => {
// Do not apply special transformation case if `MergeDelta` has `NoOperation` as the second operation.
if ( !a.position ) {
return defaultTransform( a, b, context );
}

// If merge is applied at the position between moved nodes we cancel the merge as it's results may be unexpected and
// very weird. Even if we do some "magic" we don't know what really are users' expectations.

const operateInSameParent =
a.position.root == b.sourcePosition.root &&
compareArrays( a.position.getParentPath(), b.sourcePosition.getParentPath() ) === 'same';
Expand Down
25 changes: 25 additions & 0 deletions tests/model/delta/transform/insertdelta.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import SplitDelta from '../../../../src/model/delta/splitdelta';
import InsertOperation from '../../../../src/model/operation/insertoperation';
import MoveOperation from '../../../../src/model/operation/moveoperation';
import ReinsertOperation from '../../../../src/model/operation/reinsertoperation';
import NoOperation from '../../../../src/model/operation/nooperation';

import { getNodesAndText } from '../../../../tests/model/_utils/utils';

Expand Down Expand Up @@ -156,6 +157,30 @@ describe( 'transform', () => {
} );
} );

it( 'merge in same position as insert - undo mode', () => {
// If MergeDelta second operation is NoOperation, default transformation algorithm should be used.
const mergeDelta = getMergeDelta( insertPosition, 4, 12, baseVersion );
mergeDelta.operations[ 1 ] = new NoOperation( 1 );

const transformed = transform( insertDelta, mergeDelta, context );

baseVersion = mergeDelta.operations.length;

expect( transformed.length ).to.equal( 1 );

expectDelta( transformed[ 0 ], {
type: InsertDelta,
operations: [
{
type: InsertOperation,
position: Position.createFromPosition( insertPosition ),
nodes: [ nodeA, nodeB ],
baseVersion
}
]
} );
} );

it( 'merge the node that is parent of insert position (sticky move test)', () => {
const mergeDelta = getMergeDelta( new Position( root, [ 3, 3 ] ), 1, 4, baseVersion );
const transformed = transform( insertDelta, mergeDelta, context );
Expand Down
57 changes: 57 additions & 0 deletions tests/model/delta/transform/mergedelta.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,34 @@ describe( 'transform', () => {
} );
} );

it( 'insert at same position as merge - merge second operation is NoOperation', () => {
// If MergeDelta second operation is NoOperation, default transformation algorithm should be used.
mergeDelta.operations[ 1 ] = new NoOperation( 1 );
const insertDelta = getInsertDelta( mergePosition, [ nodeA, nodeB ], baseVersion );
const transformed = transform( mergeDelta, insertDelta, context );

baseVersion = insertDelta.operations.length;

expect( transformed.length ).to.equal( 1 );

expectDelta( transformed[ 0 ], {
type: MergeDelta,
operations: [
{
type: MoveOperation,
sourcePosition: new Position( root, [ 3, 3, 5, 0 ] ),
howMany: mergeDelta.operations[ 0 ].howMany,
targetPosition: mergeDelta.operations[ 0 ].targetPosition,
baseVersion
},
{
type: NoOperation,
baseVersion: baseVersion + 1
}
]
} );
} );

it( 'insert inside merged node (sticky move test)', () => {
const insertDelta = getInsertDelta( new Position( root, [ 3, 3, 3, 12 ] ), [ nodeA, nodeB ], baseVersion );
const transformed = transform( mergeDelta, insertDelta, context );
Expand Down Expand Up @@ -227,6 +255,35 @@ describe( 'transform', () => {
// MoveDelta is applied. MergeDelta is discarded.
expect( nodesAndText ).to.equal( 'DIVXabcdabcfoobarxyzXXXXXDIV' );
} );

it( 'node on the left side of merge was moved - second merge operation is NoOperation', () => {
// If second MergeDelta operation is NoOperation default algorithm should be used.
mergeDelta.operations[ 1 ] = new NoOperation( 1 );

const moveDelta = getMoveDelta( new Position( root, [ 3, 3, 2 ] ), 1, new Position( root, [ 3, 3, 0 ] ), baseVersion );
const transformed = transform( mergeDelta, moveDelta, context );

expect( transformed.length ).to.equal( 1 );

baseVersion = moveDelta.operations.length;

expectDelta( transformed[ 0 ], {
type: MergeDelta,
operations: [
{
type: MoveOperation,
sourcePosition: new Position( root, [ 3, 3, 3, 0 ] ),
howMany: 12,
targetPosition: new Position( root, [ 3, 3, 0, 4 ] ),
baseVersion
},
{
type: NoOperation,
baseVersion: baseVersion + 1
}
]
} );
} );
} );

describe( 'MergeDelta', () => {
Expand Down
27 changes: 27 additions & 0 deletions tests/model/delta/transform/movedelta.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import MoveDelta from '../../../../src/model/delta/movedelta';
import SplitDelta from '../../../../src/model/delta/splitdelta';

import MoveOperation from '../../../../src/model/operation/moveoperation';
import NoOperation from '../../../../src/model/operation/nooperation';

import { getNodesAndText } from '../../../../tests/model/_utils/utils';

Expand Down Expand Up @@ -159,6 +160,32 @@ describe( 'transform', () => {
]
} );
} );

it( 'node on the right side of merge was moved - second merge operation is NoOperation', () => {
// If second MergeDelta operation is NoOperation default algorithm should be used.
const mergePosition = new Position( root, [ 3, 3, 3 ] );
const mergeDelta = getMergeDelta( mergePosition, 4, 12, baseVersion );
mergeDelta.operations[ 1 ] = new NoOperation( 1 );

const transformed = transform( moveDelta, mergeDelta, context );

expect( transformed.length ).to.equal( 1 );

baseVersion = mergeDelta.operations.length;

expectDelta( transformed[ 0 ], {
type: MoveDelta,
operations: [
{
type: MoveOperation,
sourcePosition: moveDelta._moveOperation.sourcePosition,
howMany: moveDelta._moveOperation.howMany,
targetPosition: moveDelta._moveOperation.targetPosition,
baseVersion
}
]
} );
} );
} );
} );
} );

0 comments on commit 0e99fc5

Please sign in to comment.