Add more test cases, Fix Util#generatePositionBetween (includes debug comments)

src/main/java/me/mrletsplay/shareclientcore/document/SharedDocument.java

@@ -53,6 +53,8 @@ public class SharedDocument implements MessageListener {
 
 		Change[] changes = new Change[bytes.length];
 		for(int i = 0; i < bytes.length; i++) {
+//			System.out.println(charBefore);
+//			System.out.println(charAfter);
 			Identifier[] newPos = Util.generatePositionBetween(charBefore.position(), charAfter.position(), site);
 			lamport++;
 			Char ch = new Char(newPos, lamport, bytes[i]);
@@ -61,6 +63,11 @@ public class SharedDocument implements MessageListener {
 			charBefore = ch;
 		}
 
+//		System.out.println("!! New changes:");
+//		for(Change c : changes) {
+//			System.out.println(c);
+//		}
+
 		return changes;
 	}
 

src/main/java/me/mrletsplay/shareclientcore/document/Util.java

@@ -26,13 +26,19 @@ public class Util {
 	}
 
 	public static Identifier[] generatePositionBetween(Identifier[] before, Identifier[] after, int site) {
-		if(comparePositions(before, after) != -1) throw new IllegalArgumentException("before must be strictly less than after");
+		if(comparePositions(before, after) != -1) {
+			System.err.println("Got invalid positions");
+			System.err.println(Arrays.toString(before));
+			System.err.println(Arrays.toString(after));
+			Thread.dumpStack();
+			throw new IllegalArgumentException("before must be strictly less than after");
+		}
 
 		List<Identifier> newPosition = new ArrayList<>();
 
 		for(int i = 0; i < Math.max(before.length, after.length) + 1; i++) {
 			Identifier c1 = i >= before.length ? new Identifier(0, site) : before[i];
-			Identifier c2 = i >= after.length ? new Identifier(BASE - 1, site) : after[i];
+			Identifier c2 = i >= after.length ? new Identifier(BASE, site) : after[i];
 
 			// Modification to the original source because otherwise it can lead to cases where positions can't be generated and this seems to work
 			if(i >= before.length || c1.digit() != c2.digit()) {
@@ -48,8 +54,12 @@ public class Util {
 			if(c1.site() < c2.site()) {
 				// Anything starting with before will be sorted before after
 				newPosition.add(c1);
-				newPosition.add(new Identifier(1, site));
+
-				return newPosition.toArray(Identifier[]::new);
+				// Should be equivalent to recursively calling generatePosition with a truncated after (as in the reference implementation in the blog post)
+				Identifier[] newAfter = Arrays.copyOf(after, i + 2);
+				newAfter[i + 1] = new Identifier(BASE, site);
+				int[] incremented = getIncremented(before, newAfter, i + 1);
+				return constructPosition(incremented, before, newAfter, site);
 			}
 
 			System.err.println("Got invalid state");

src/test/java/me/mrletsplay/shareclientcore/DecimalTest.java

@@ -3,6 +3,8 @@ package me.mrletsplay.shareclientcore;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import java.util.Arrays;
+
 import org.junit.jupiter.api.Test;
 
 import me.mrletsplay.shareclientcore.document.Identifier;
@@ -139,4 +141,12 @@ public class DecimalTest {
 		assertThrows(IllegalArgumentException.class, () -> Util.generatePositionBetween(a, b, 3));
 	}
 
+	@Test
+	public void testGeneratePositionEdgeCase1() {
+		Identifier[] a = { new Identifier(3, 0), new Identifier(1, 1) };
+		Identifier[] b = { new Identifier(3, 1) };
+		Identifier[] sus = Util.generatePositionBetween(a, b, 1);
+		System.out.println(Arrays.toString(sus));
+	}
+
 }

src/test/java/me/mrletsplay/shareclientcore/SharingTest.java

@@ -2,17 +2,22 @@ package me.mrletsplay.shareclientcore;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Random;
 
 import org.junit.jupiter.api.Test;
 
 import me.mrletsplay.shareclientcore.connection.DummyConnection;
+import me.mrletsplay.shareclientcore.connection.message.Message;
 import me.mrletsplay.shareclientcore.document.SharedDocument;
 
 public class SharingTest {
 
+	private static final String CHARS = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890";
+
 	@Test
-	public void testSharedDocument() {
+	public void testSharedDocumentSimple() {
 		DummyConnection a = new DummyConnection(0);
 		DummyConnection b = new DummyConnection(1);
 
@@ -46,8 +51,23 @@ public class SharingTest {
 		assertEquals("This is!", sharedB.getContentsAsString());
 	}
 
+	private void performRandomEdit(Random r, SharedDocument document) {
+		if(document.getContents().length == 0 || r.nextBoolean()) {
+//			System.out.println("INSERT");
+			char[] insert = new char[r.nextInt(16)];
+			for(int j = 0; j < insert.length; j++) insert[j] = CHARS.charAt(r.nextInt(CHARS.length()));
+			document.localInsert(r.nextInt(document.getContents().length + 1), String.valueOf(insert));
+		}else {
+//			System.out.println("DELETE");
+			int len = document.getContents().length;
+			int idx = r.nextInt(len);
+			int n = r.nextInt(Math.min(16, len - idx)) + 1;
+			document.localDelete(idx, n);
+		}
+	}
+
 	@Test
-	public void testSharedDocument2() {
+	public void testSharedDocumentRandom() {
 		DummyConnection a = new DummyConnection(0);
 		DummyConnection b = new DummyConnection(1);
 
@@ -60,42 +80,9 @@ public class SharingTest {
 		a.addListener(sharedA);
 		b.addListener(sharedB);
 
-		String chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890";
-
 		Random r = new Random(0);
 		for(int i = 0; i < 10_000; i++) {
-			switch(r.nextInt(sharedA.getContents().length == 0 ? 2 : 4)) {
+			performRandomEdit(r, r.nextBoolean() ? sharedA : sharedB);
-				case 0: {
-					// Insert A
-					char[] insert = new char[r.nextInt(16)];
-					for(int j = 0; j < insert.length; j++) insert[j] = chars.charAt(r.nextInt(chars.length()));
-					sharedA.localInsert(r.nextInt(sharedA.getContents().length + 1), String.valueOf(insert));
-					break;
-				}
-				case 1: {
-					// Insert B
-					char[] insert = new char[r.nextInt(16)];
-					for(int j = 0; j < insert.length; j++) insert[j] = chars.charAt(r.nextInt(chars.length()));
-					sharedB.localInsert(r.nextInt(sharedB.getContents().length + 1), String.valueOf(insert));
-					break;
-				}
-				case 2: {
-					// Delete A
-					int len = sharedA.getContents().length;
-					int idx = r.nextInt(len);
-					int n = r.nextInt(Math.min(16, len - idx)) + 1;
-					sharedA.localDelete(idx, n);
-					break;
-				}
-				case 3: {
-					// Delete B
-					int len = sharedB.getContents().length;
-					int idx = r.nextInt(len);
-					int n = r.nextInt(Math.min(16, len - idx)) + 1;
-					sharedB.localDelete(idx, n);
-					break;
-				}
-			}
 
 //			System.out.println("A: " + sharedA.getContentsAsString());
 //			System.out.println("B: " + sharedB.getContentsAsString());
@@ -105,7 +92,80 @@ public class SharingTest {
 
 	@Test
 	public void testSharedDocumentInterleaving() {
-		// TODO: test random interleaving of messages
+		List<Message> messagesA = new ArrayList<>();
+		List<Message> messagesB = new ArrayList<>();
+
+		DummyConnection a = new DummyConnection(0);
+		DummyConnection b = new DummyConnection(1);
+
+		a.setSendMessageHandler(messagesA::add);
+		b.setSendMessageHandler(messagesB::add);
+
+		SharedDocument sharedA = new SharedDocument(a, "doc");
+		SharedDocument sharedB = new SharedDocument(b, "doc");
+
+		a.addListener(sharedA);
+		b.addListener(sharedB);
+
+		Random r = new Random(2);
+		for(int i = 0; i < 1000; i++) {
+//			System.out.println(i);
+
+			// Perform some random edits
+			for(int j = 0; j < 100; j++) {
+				boolean bEdit = r.nextBoolean();
+//				System.out.println(bEdit ? "B EDIT" : "A EDIT");
+				performRandomEdit(r, bEdit ? sharedA : sharedB);
+			}
+
+			// Randomly interleave messages
+			while(!messagesA.isEmpty() || !messagesB.isEmpty()) {
+				if(messagesB.isEmpty() || (!messagesA.isEmpty() && r.nextBoolean())) {
+//					System.out.println("A -> B: " + messagesA.get(0));
+					b.receive(messagesA.remove(0));
+				}else {
+//					System.out.println("B -> A: " + messagesB.get(0));
+					a.receive(messagesB.remove(0));
+				}
+			}
+
+			// At this point, both documents should be in sync again
+			assertEquals(sharedA.getContentsAsString(), sharedB.getContentsAsString());
+			assertEquals(sharedA.getCharBag().toList(), sharedB.getCharBag().toList());
+		}
+	}
+
+	@Test
+	public void testShareDocumentDeleteAndReinsert() {
+		List<Message> messagesA = new ArrayList<>();
+		List<Message> messagesB = new ArrayList<>();
+
+		DummyConnection a = new DummyConnection(0);
+		DummyConnection b = new DummyConnection(1);
+
+		a.setSendMessageHandler(messagesA::add);
+		b.setSendMessageHandler(messagesB::add);
+
+		SharedDocument sharedA = new SharedDocument(a, "doc");
+		SharedDocument sharedB = new SharedDocument(b, "doc");
+
+		a.addListener(sharedA);
+		b.addListener(sharedB);
+
+		sharedA.localInsert(0, "Test");
+		messagesA.forEach(b::receive);
+		messagesA.clear();
+		assertEquals("Test", sharedA.getContentsAsString());
+		assertEquals("Test", sharedB.getContentsAsString());
+
+		sharedA.localDelete(3, 1);
+		sharedA.localInsert(3, "t");
+		assertEquals("Test", sharedA.getContentsAsString());
+
+		messagesA.forEach(b::receive);
+		messagesA.clear();
+
+		assertEquals("Test", sharedA.getContentsAsString());
 	}
 
 }