ResourceDefinitionValidator does not check all dependency cycles

[dennisvang]

Background

The ResourceDefinitionValidator checks for dependency cycles:

FAIRDataPoint/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionValidator.java

Line 79 in e746e9a

validateDependencyCyclesChildDTO(uuid, reqDto.getChildren());

This is implemented as follows:

FAIRDataPoint/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionValidator.java

Lines 82 to 97 in e746e9a

	private void validateDependencyCyclesChildDTO(
	UUID uuid, List<ResourceDefinitionChildDTO> children
	) {
	for (ResourceDefinitionChildDTO child : children) {
	final UUID childUuid = child.getResourceDefinitionUuid();
	if (childUuid.equals(uuid)) {
	throw new ValidationException(ERR_DEP_CYCLE);
	}
	final ResourceDefinition rdChild = resourceDefinitionCache.getByUuid(childUuid);
	if (rdChild.getChildren().isEmpty()) {
	return;
	}
	validateDependencyCyclesChild(uuid, rdChild.getChildren());
	}
	}

Basically the method loops over all children of the reqDto, and, for each child, gets the corresponding ResourceDefinition (rdChild) from cache. If this rdChild itself has no children, the method returns directly in line 93, effectively short-circuiting.

Problem

Now suppose the reqDto has multiple children, but (one of) the first rdChild object(s) has no children of its own.
The method now returns without checking the remaining children.

Proposed solution

We could replace return by continue, but it looks like we could also simply remove the if-conditions altogether, because that will simply result in iteration over an empty list.

So we can probably remove the following lines:

FAIRDataPoint/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionValidator.java

Lines 92 to 94 in e746e9a

	if (rdChild.getChildren().isEmpty()) {
	return;
	}

and

FAIRDataPoint/src/main/java/org/fairdatapoint/service/resource/ResourceDefinitionValidator.java

Lines 109 to 111 in e746e9a

	if (rdChild.getChildren().isEmpty()) {
	return;
	}

Notes

• I wonder why there are two nearly identical methods, viz. validateDependencyCyclesChildDTO and validateDependencyCyclesChild.