diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/logging/Messages.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/logging/Messages.java index 405552e1b0f..ea48cec7db9 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/logging/Messages.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/logging/Messages.java @@ -54,4 +54,6 @@ public enum Messages { VALUE_OUT_OF_RANGE_DETAIL_TEMPLATE, NOT_SUPPORTED_REASON, + + INCORRECT_FOLLOW_LINK } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java index cd6489a9007..4a551ef77f4 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java @@ -24,6 +24,7 @@ import org.ovirt.engine.api.restapi.invocation.CurrentManager; import org.ovirt.engine.api.restapi.logging.MessageBundle; import org.ovirt.engine.api.restapi.logging.Messages; +import org.ovirt.engine.api.restapi.resource.exception.IncorrectFollowLinkException; import org.ovirt.engine.api.restapi.resource.utils.LinkFollower; import org.ovirt.engine.api.restapi.resource.utils.LinksTreeNode; import org.ovirt.engine.api.restapi.types.MappingLocator; @@ -426,7 +427,13 @@ public final void follow (ActionableResource entity) { ParametersHelper.removeParameter(MAX); LinksTreeNode linksTree = linkFollower.createLinksTree(entity.getClass(), followValue); follow(entity, linksTree); - linkFollower.followLinks(entity, linksTree); + try { + linkFollower.followLinks(entity, linksTree); + } catch (IncorrectFollowLinkException e) { + throw new WebFaultException(e, + localize(Messages.INCORRECT_FOLLOW_LINK, e.getLink(), e.getEntityName()), + Response.Status.BAD_REQUEST); + } } } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/exception/IncorrectFollowLinkException.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/exception/IncorrectFollowLinkException.java new file mode 100644 index 00000000000..37dd6e57320 --- /dev/null +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/exception/IncorrectFollowLinkException.java @@ -0,0 +1,21 @@ +package org.ovirt.engine.api.restapi.resource.exception; + +public class IncorrectFollowLinkException extends RuntimeException { + + private final String link; + private final String entityName; + + public IncorrectFollowLinkException(String link, String entityName, Throwable cause) { + super(cause); + this.entityName = entityName; + this.link = link; + } + + public String getLink() { + return link; + } + + public String getEntityName() { + return entityName; + } +} diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/LinkFollower.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/LinkFollower.java index 5726033e31c..6503fe61e43 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/LinkFollower.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/utils/LinkFollower.java @@ -15,6 +15,7 @@ import org.ovirt.engine.api.model.Link; import org.ovirt.engine.api.restapi.resource.BaseBackendResource; import org.ovirt.engine.api.restapi.resource.ResourceLocator; +import org.ovirt.engine.api.restapi.resource.exception.IncorrectFollowLinkException; import org.ovirt.engine.api.utils.EntityHelper; import org.ovirt.engine.api.utils.ReflectionHelper; @@ -139,6 +140,10 @@ private void followLinks(List entities, LinksTreeNode node) * follow all links in the provided links-tree, recursively. */ private void followLink(ActionableResource entity, LinksTreeNode node) { + if (entity == null || node == null) { + return; + } + List nextStepEntities = new LinkedList<>(); if (EntityHelper.isCollection(entity)) { nextStepEntities.addAll(fetchData((BaseResources) entity, node)); @@ -181,7 +186,7 @@ private List fetchData(BaseResources collectionEntity, Links results.add(fetchData(entity, node)); } } catch (Exception e) { - throw new IllegalStateException("Problem following '" + node.getElement() + "' link in " + collectionEntity.getClass().getSimpleName() + " entity.", e); + throw new IncorrectFollowLinkException(node.getElement(), collectionEntity.getClass().getSimpleName(), e); } return results; } @@ -204,17 +209,19 @@ private ActionableResource fetchData(BaseResource entity, LinksTreeNode link) { String element = underscoreToCamelCase(link.getElement()); if (link.isFollowed()) { Method getter = ReflectionHelper.getGetter(entity, element); - return (ActionableResource) getter.invoke(entity); + return getter != null ? (ActionableResource) getter.invoke(entity) : null; } else { String href = getHref((BaseResource) entity, link.getElement()); ActionableResource result = fetch(href); - Method setter = ReflectionHelper.getSetter(entity, element); - setter.invoke(entity, result); + + if (result != null) { + Method setter = ReflectionHelper.getSetter(entity, element); + setter.invoke(entity, result); + } return result; } } catch (Exception e) { - throw new IllegalStateException("Problem fetching '" + link.getElement() + - "' from " + entity.getClass().getSimpleName(), e); + throw new IncorrectFollowLinkException(link.getElement(), entity.getClass().getSimpleName(), e); } } @@ -243,8 +250,11 @@ private String getHref(BaseResource entity, String link) throws IllegalAccessExc return optional.get().getHref(); } else { //assume this is not a sub-collection, since it wasn't found among links. Method getter = ReflectionHelper.getGetter(entity, underscoreToCamelCase(link)); + if (getter == null) { + throw new IllegalStateException("Follow link '" + link + "' is incorrect."); + } BaseResource member = (BaseResource) getter.invoke(entity); - return member.getHref(); + return member != null ? member.getHref() : null; } } @@ -252,6 +262,10 @@ private String getHref(BaseResource entity, String link) throws IllegalAccessExc * This scope of this method is 'protected' for testing purposes. */ protected ActionableResource fetch(String href) { + if (href == null) { + return null; + } + try { BaseBackendResource resource = resourceLocator.locateResource(href); //need to invoke the method in the resource annotated with @GET diff --git a/backend/manager/modules/restapi/jaxrs/src/main/resources/org/ovirt/engine/api/restapi/logging/Messages.properties b/backend/manager/modules/restapi/jaxrs/src/main/resources/org/ovirt/engine/api/restapi/logging/Messages.properties index 7b64aed6a5f..bc13f0d0b7e 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/resources/org/ovirt/engine/api/restapi/logging/Messages.properties +++ b/backend/manager/modules/restapi/jaxrs/src/main/resources/org/ovirt/engine/api/restapi/logging/Messages.properties @@ -25,3 +25,4 @@ VALUE_OUT_OF_RANGE_DETAIL_TEMPLATE=The value {0} of attribute ''{1}'' is outside DISK_UPDATE_NOT_PERMITTED=Updating disk attributes other than QCOW version is permitted only for disk-attachments, which reside under VMs. CPU_UPDATE_NOT_PERMITTED=Attempt to automatically configure CPU topology or pinning while CPU topology or pinning is also specified. NOT_SUPPORTED_REASON=The input is unsupported. {0} is not supported. +INCORRECT_FOLLOW_LINK=Problem following {0} link in {1} entity. \ No newline at end of file diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/LinkFollowerTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/LinkFollowerTest.java index 72609ba96c7..cd74e8512d6 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/LinkFollowerTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/util/LinkFollowerTest.java @@ -3,6 +3,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.BeforeEach; @@ -18,8 +20,10 @@ import org.ovirt.engine.api.model.Nic; import org.ovirt.engine.api.model.Nics; import org.ovirt.engine.api.model.Vm; +import org.ovirt.engine.api.model.Vms; import org.ovirt.engine.api.restapi.resource.BackendVmNicsResource; import org.ovirt.engine.api.restapi.resource.ResourceLocator; +import org.ovirt.engine.api.restapi.resource.exception.IncorrectFollowLinkException; import org.ovirt.engine.api.restapi.resource.utils.LinkFollower; import org.ovirt.engine.api.restapi.resource.utils.LinksTreeNode; @@ -38,7 +42,9 @@ public void setUp() { linkFollower = new LinkFollower(resourceLocator) { //override fetch() since it requires a real environment and would crash tests. protected ActionableResource fetch(String href) { - if (href.equals("/ovirt-engine/api/vms/63978315-2d17-4e67-b393-2ea60a8aeacb/nics")) { + if (href == null) { + return null; + } else if (href.equals("/ovirt-engine/api/vms/63978315-2d17-4e67-b393-2ea60a8aeacb/nics")) { return createNics(); } else if (href.equals("/ovirt-engine/api/vms/63978315-2d17-4e67-b393-2ea60a8aeacb/diskattachments")) { return createDiskAttachments(); @@ -75,6 +81,47 @@ public void testFollowLinks() throws SecurityException, IllegalArgumentException assertNotNull(vm.getDiskAttachments().getDiskAttachments().get(2).getDisk()); } + @Test + public void testFollowLinksIfFollowIsIncorrect() { + LinksTreeNode linksTree = linkFollower.createLinksTree(Vm.class, "incorrect_nics"); + Vm vm = createVm(); + IncorrectFollowLinkException actualException = assertThrows( + IncorrectFollowLinkException.class, + () -> linkFollower.followLinks(vm, linksTree) + ); + assertEquals("incorrect_nics", actualException.getLink()); + assertEquals("Vm", actualException.getEntityName()); + } + + @Test + public void testFollowLinksForCollectionEntityIfFollowIsIncorrect() { + LinksTreeNode linksTree = linkFollower.createLinksTree(Vm.class, "incorrect_nics"); + Vms vms = createVms(); + IncorrectFollowLinkException actualException = assertThrows( + IncorrectFollowLinkException.class, + () -> linkFollower.followLinks(vms, linksTree) + ); + assertEquals("incorrect_nics", actualException.getLink()); + assertEquals("Vms", actualException.getEntityName()); + } + + @Test + public void testFollowLinksIfFollowedEntityIsNull() { + LinksTreeNode linksTree = linkFollower.createLinksTree(Vm.class, "template"); + Vm vm = createVm(); + linkFollower.followLinks(vm, linksTree); + assertNull(vm.getTemplate()); + assertNull(vm.getNics()); + assertNull(vm.getDiskAttachments()); + } + + private Vms createVms() { + Vms vms = new Vms(); + vms.getVms().add(createVm()); + vms.getVms().add(createVm()); + return vms; + } + private Vm createVm() { Vm vm = new Vm(); //add an irrelevant link