FhirPatch diff treats a simultaneous add/delete as an update

[TahaAttari]

hapi-fhir/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java

Lines 508 to 548 in e15d043

	int sourceIndex = 0;
	int targetIndex = 0;
	while (sourceIndex < sourceValues.size() && targetIndex < targetValues.size()) {
	IBase sourceChildField = sourceValues.get(sourceIndex);
	Validate.notNull(sourceChildField); // not expected to happen, but just in case
	BaseRuntimeElementDefinition<?> def = myContext.getElementDefinition(sourceChildField.getClass());
	IBase targetChildField = targetValues.get(targetIndex);
	Validate.notNull(targetChildField); // not expected to happen, but just in case
	String sourcePath = theSourcePath + "." + elementName + (repeatable ? "[" + sourceIndex + "]" : "");
	String targetPath = theSourcePath + "." + elementName + (repeatable ? "[" + targetIndex + "]" : "");
	compare(theDiff, theSourceEncodePath, def, sourcePath, targetPath, sourceChildField, targetChildField);
	sourceIndex++;
	targetIndex++;
	}
	// Find newly inserted items
	while (targetIndex < targetValues.size()) {
	String path = theTargetPath + "." + elementName;
	addInsertItems(theDiff, targetValues, targetIndex, path, theChildDef);
	targetIndex++;
	}
	// Find deleted items
	while (sourceIndex < sourceValues.size()) {
	IBase operation = ParametersUtil.addParameterToParameters(myContext, theDiff, PARAMETER_OPERATION);
	ParametersUtil.addPartCode(myContext, operation, PARAMETER_TYPE, OPERATION_DELETE);
	ParametersUtil.addPartString(
	myContext,
	operation,
	PARAMETER_PATH,
	theTargetPath + "." + elementName + (repeatable ? "[" + targetIndex + "]" : ""));
	sourceIndex++;
	targetIndex++;
	}
	theSourceEncodePath.popPath();
	}

Array element comparison has the following assumptions:

1. Order of elements never changes
2. All insertions are appended to the end
3. All deletions happen from the end
4. There is no simultaneous insert + delete operation

Because of this a simultaneous insert + delete is treated as an update, and the same with reordering array elements

[marcindz88]

The issue described is hard to solve i think, however we have a similar situation with a different case where not last item in an array was deleted (e.g. we have 3 CarePlan Activities and remove second) and hence it shows us 1 deletion (of last item) and 2 modifications of second to last item (as code and status differs).

I think that the diff operation should identify which element of an array has been deleted not by simply checking which list is larger, but instead compare array elements by equalsDeep, to identify, which one was deleted, exclude them from further checks and only then check modifications.