Skip to content

#1482 - Delete function for AlignedString does not work properly #1645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
reckart merged 1 commit into from
Aug 9, 2026
+189 −11
Merged

#1482 - Delete function for AlignedString does not work properly #1645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 87 additions & 11 deletions ...api-transform-asl/src/main/java/org/dkpro/core/api/transform/alignment/AlignedString.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;

Expand Down Expand Up @@ -87,15 +88,25 @@ private void addChangeListener(final AlignedString l)

public void fireChange()
{
// if (!_startDirty) {
// System.out.println("startDirty true");
// }
fireChange(new HashSet<AlignedString>());
}

/**
* Mark this level and all levels stacked on top of it as dirty. Propagation is recursive so
* that a change also reaches levels which do not directly wrap the changed level. The set of
* already notified levels guards against cycles.
*/
private void fireChange(final Set<AlignedString> notified)
{
if (!notified.add(this)) {
return;
}

_stringDirty = true;
_startDirty = true;

for (final AlignedString a : _changeListeners) {
a._stringDirty = true;
a._startDirty = true;
a.fireChange(notified);
}
}

Expand Down Expand Up @@ -474,7 +485,6 @@ public void updateCaches()
seg = seg._next;
}
_startDirty = false;
System.out.println("startDirty false");
}
}

Expand Down Expand Up @@ -553,9 +563,33 @@ private void dropSuperflourous(final AbstractDataSegment seg)
}
}

/**
* Drop zero-length oblique segments that mark the same position in the underlying data as their
* immediate zero-length neighbour. Such duplicates carry no additional information.
*/
private void dropDuplicateMarkers(final AbstractDataSegment from)
{
if (from == null) {
return;
}

AbstractDataSegment s = from;
while (s != null && s._next != null) {
final AbstractDataSegment n = s._next;
if ((s instanceof ObliqueSegment) && (n instanceof ObliqueSegment) && (s.length() == 0)
&& (n.length() == 0) && (((ObliqueSegment) s)._end
.getPosition() == ((ObliqueSegment) n)._end.getPosition())) {
n._prev._next = n._next;
n._next._prev = n._prev;
continue;
}
s = s._next;
}
}

/**
* Deletes data.
*
*
* @param start
* the start offset.
* @param end
Expand All @@ -579,7 +613,10 @@ public void delete(final int start, final int end)
public void replace(final int start, final int end, final String d)
{
if (start == end) {
insert(start, d);
// Replacing an empty range is an insert - and inserting nothing is a no-op.
if (d != null) {
insert(start, d);
}
return;
}

Expand All @@ -602,8 +639,21 @@ public void replace(final int start, final int end, final String d)
suffix = suffix.split(end);

if (d == null || d.length() == 0) {
prefix._next = suffix;
suffix._prev = prefix;
// The segment between prefix and suffix covers the deleted data. Instead of
// unlinking it, collapse it to zero length so that it survives as a marker for
// the position at which data was deleted. Inverse-resolving an interval that
// was fully deleted relies on such a marker being present.
final AbstractDataSegment deleted = prefix._next;
if (deleted != suffix && deleted.collapse()) {
deleted._prev = prefix;
deleted._next = suffix;
prefix._next = deleted;
suffix._prev = deleted;
}
else {
prefix._next = suffix;
suffix._prev = prefix;
}
}
else {
final BaseSegment s = new BaseSegment(prefix, suffix, d);
Expand Down Expand Up @@ -631,10 +681,16 @@ public void replace(final int start, final int end, final String d)

if (d == null || d.length() == 0) {
AbstractDataSegment s = prefix._next;
AbstractDataSegment marker = null;
while (s != suffix) {
if (s.isAnchor()) {
// anchors need to be preserved
}
else if (marker == null && s.collapse()) {
// Keep the first collapsible segment as a zero-length marker for the
// position at which the data was deleted.
marker = s;
}
else {
// non-anchors need to be removed
s._prev._next = s._next;
Expand All @@ -653,6 +709,7 @@ public void replace(final int start, final int end, final String d)
// Drop useless segments
dropSuperflourous(prefix);
dropSuperflourous(suffix);
dropDuplicateMarkers(prefix);

// if (_log.isDebugEnabled()) {
// _log.debug("post delete("+start+","+end+") - "+dataSegmentsToString());
Expand Down Expand Up @@ -844,6 +901,16 @@ public AbstractDataSegment(final AbstractDataSegment prev, final AbstractDataSeg

public abstract AbstractDataSegment split(int position);

/**
* Shrink the segment to zero length, retaining its position information. Returns
* {@code false} if the segment cannot serve as a zero-length marker and should rather be
* unlinked.
*/
public boolean collapse()
{
return false;
}

@Override
public DataSegment getPrevious()
{
Expand Down Expand Up @@ -968,7 +1035,7 @@ public String toString()
class ObliqueSegment
extends AbstractDataSegment
{
private final Anchor _start;
private Anchor _start;
private Anchor _end;

public ObliqueSegment(final AbstractDataSegment prev, final AbstractDataSegment next,
Expand Down Expand Up @@ -1019,6 +1086,15 @@ public AbstractDataSegment split(final int position)
return suffix;
}

@Override
public boolean collapse()
{
// Collapse onto the end anchor, consistent with the marker that a delete at the
// right-hand boundary of a segment leaves behind.
_start = _end;
return true;
}

@Override
public boolean isAnchor()
{
Expand Down
102 changes: 102 additions & 0 deletions ...transform-asl/src/test/java/org/dkpro/core/api/transform/alignment/AlignedStringTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,108 @@ public void testDirty()
assertEquals(topRef.toString(), top.get());
}

/**
* A change on a level must also become visible on levels that do not wrap the changed level
* directly but only transitively.
*/
@Test
public void testChangePropagatesTransitively()
{
final AlignedString l0 = new AlignedString("abcdef");
final AlignedString l1 = new AlignedString(l0);
final AlignedString l2 = new AlignedString(l1);

// Read once so that the caches are populated
assertEquals("abcdef", l2.get());

l0.insert(0, "ZZ");

assertEquals("ZZabcdef", l0.get());
assertEquals("ZZabcdef", l1.get());
assertEquals("ZZabcdef", l2.get());
}

/**
* Deleting an empty range must be a no-op rather than fail.
*/
@Test
public void testDeleteEmptyRange()
{
top.delete(2, 2);

assertEquals(baseString, top.get());
}

/**
* Deleting multiple regions must leave a zero-length marker behind for every deleted region so
* that inverse-resolving a fully deleted region yields the position at which it was removed.
*
* @see <a href="https://github.com/dkpro/dkpro-core/issues/1482">Issue 1482</a>
*/
@Test
public void testDeleteMultipleRegions()
{
// 11111111112222
// 012345678901234567890123
baseString = "<p>Hello<p>World</p></p>";
bottom = new AlignedString(baseString);
top = new AlignedString(bottom);

final ImmutableInterval[] tags = { new ImmutableInterval(0, 3),
new ImmutableInterval(8, 11), new ImmutableInterval(16, 20),
new ImmutableInterval(20, 24) };

// Delete back-to-front so that the offsets of the not-yet-deleted tags stay valid
for (int i = tags.length - 1; i >= 0; i--) {
top.delete(tags[i].getStart(), tags[i].getEnd());
}

assertEquals("HelloWorld", top.get());

// Every deleted tag must inverse-resolve to the empty interval at the position where it
// used to be - in particular the "<p>" between "Hello" and "World" must not collapse
// onto the end of the string.
assertEquals(new ImmutableInterval(0, 0), top.inverseResolve(tags[0]));
assertEquals(new ImmutableInterval(5, 5), top.inverseResolve(tags[1]));
assertEquals(new ImmutableInterval(10, 10), top.inverseResolve(tags[2]));
assertEquals(new ImmutableInterval(10, 10), top.inverseResolve(tags[3]));

// The text that survived must still resolve correctly in both directions
assertEquals(new ImmutableInterval(0, 5), top.inverseResolve(new ImmutableInterval(3, 8)));
assertEquals(new ImmutableInterval(5, 10),
top.inverseResolve(new ImmutableInterval(11, 16)));
assertEquals("Hello", bottom.get(top.resolve(new ImmutableInterval(0, 5)).getStart(),
top.resolve(new ImmutableInterval(0, 5)).getEnd()));
assertEquals("World", bottom.get(top.resolve(new ImmutableInterval(5, 10)).getStart(),
top.resolve(new ImmutableInterval(5, 10)).getEnd()));
}

/**
* Same as {@link #testDeleteMultipleRegions()} but deleting front-to-back, which exercises the
* other branch of {@code replace()}.
*/
@Test
public void testDeleteMultipleRegionsAscending()
{
baseString = "<p>Hello<p>World</p></p>";
bottom = new AlignedString(baseString);
top = new AlignedString(bottom);

top.delete(0, 3); // <p>
top.delete(5, 8); // <p>
top.delete(10, 14); // </p>
top.delete(10, 14); // </p>

assertEquals("HelloWorld", top.get());

assertEquals(new ImmutableInterval(0, 0), top.inverseResolve(new ImmutableInterval(0, 3)));
assertEquals(new ImmutableInterval(5, 5), top.inverseResolve(new ImmutableInterval(8, 11)));
assertEquals(new ImmutableInterval(10, 10),
top.inverseResolve(new ImmutableInterval(16, 20)));
assertEquals(new ImmutableInterval(10, 10),
top.inverseResolve(new ImmutableInterval(20, 24)));
}

/**
* For the given interval on the underlying data, get the corresponding interval on this level.
*
Expand Down
Loading