Skip to content

Commit

Permalink
restapi: incorrect href for parent href
Browse files Browse the repository at this point in the history
If User send REST API request  for ovirt-engine/api/disks/{diskid}/disksnapshots?include_active&include_template he will get this  incorrect href back <parent href="/ovirt-engine/api/disks/{diskid}" id="{parentid}"/>.
Now it will get correct output that will look like this <parent href="/ovirt-engine/api/disks/{diskid}/disksnapshots/{parentid}" id="{parentid}"/>.

Bug-Url: https://bugzilla.redhat.com/2013697
Signed-off-by: Artiom Divak <adivak@redhat.com>
  • Loading branch information
ArtiomDivak committed Sep 18, 2022
1 parent 27dc8ee commit 80af2c8
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,31 @@

import org.ovirt.engine.core.bll.QueriesCommandBase;
import org.ovirt.engine.core.bll.context.EngineContext;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
import org.ovirt.engine.core.dao.DiskImageDao;
import org.ovirt.engine.core.dao.ImageDao;

public class GetDiskSnapshotByImageIdQuery<P extends IdQueryParameters> extends QueriesCommandBase<P> {
@Inject
private DiskImageDao diskImageDao;

@Inject
private ImageDao imageDao;

public GetDiskSnapshotByImageIdQuery(P parameters, EngineContext engineContext) {
super(parameters, engineContext);
}

@Override
protected void executeQueryCommand() {
getQueryReturnValue().setReturnValue(diskImageDao.getSnapshotById(getParameters().getId()));
DiskImage diskImage = diskImageDao.getSnapshotById(getParameters().getId());
if (diskImage == null) {
getQueryReturnValue().setReturnValue(null);
return;
}
diskImage.setParentDiskId(imageDao.get(diskImage.getParentId()).getDiskId());

getQueryReturnValue().setReturnValue(diskImage);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,23 @@ protected void executeQueryCommand() {

// If the 'include_template' flag requested - check if one of the disk images' parents is a template disk,
// fetch it and add to the result
if (getParameters().getIncludeTemplate()) {
Set<Guid> imageIds = imagesToReturn.stream().map(DiskImage::getImageId).collect(Collectors.toSet());
DiskImage imageWithMissingParent = imagesToReturn.stream().
filter(image -> image.hasParent() && !imageIds.contains(image.getParentId()))
.findFirst().orElse(null);
if (imageWithMissingParent != null) {
// Found image with parent guid that does not belong to the requested disk
DiskImage parentImage = diskImageDao.getAncestor(imageWithMissingParent.getImageId());
if (parentImage != null && parentImage.isTemplate()) {
Set<Guid> imageIds = imagesToReturn.stream().map(DiskImage::getImageId).collect(Collectors.toSet());
DiskImage imageWithMissingParent = imagesToReturn.stream()
.filter(image -> image.hasParent() && !imageIds.contains(image.getParentId()))
.findFirst().orElse(null);
if (imageWithMissingParent != null) {
// Found image with parent guid that does not belong to the requested disk
DiskImage parentImage = diskImageDao.getAncestor(imageWithMissingParent.getImageId());
if (parentImage != null && parentImage.isTemplate()) {
imageWithMissingParent.setParentDiskId(parentImage.getId());
if (getParameters().getIncludeTemplate()) {
imagesToReturn.add(parentImage);
} else {
log.error("Image '{}' which is a parent of image '{}', was not found",
imageWithMissingParent.getParentId(), imageWithMissingParent.getId());
}
} else {
log.error("Image '{}' which is a parent of image '{}', was not found",
imageWithMissingParent.getParentId(), imageWithMissingParent.getId());
}
}

getQueryReturnValue().setReturnValue(imagesToReturn);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import java.util.List;
import java.util.Objects;

import org.ovirt.engine.core.common.businessentities.TransientField;
import org.ovirt.engine.core.common.businessentities.VmEntityType;
import org.ovirt.engine.core.compat.Guid;

import com.fasterxml.jackson.annotation.JsonIgnore;

Expand All @@ -31,6 +33,9 @@ public abstract class Disk extends BaseDisk {
private boolean plugged;
private String logicalName;

@TransientField
private Guid parentDiskId;

/**
* Image Transfer information is only for display purposes
*/
Expand Down Expand Up @@ -174,4 +179,14 @@ public boolean isDiskSnapshot() {
public boolean isOvfStore() {
return getContentType() == DiskContentType.OVF_STORE;
}

@JsonIgnore
public Guid getParentDiskId() {
return parentDiskId;
}

@JsonIgnore
public void setParentDiskId(Guid parentDiskId) {
this.parentDiskId = parentDiskId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.ovirt.engine.api.resource.DiskSnapshotResource;
import org.ovirt.engine.core.common.action.ActionType;
import org.ovirt.engine.core.common.action.RemoveDiskSnapshotsParameters;
import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
import org.ovirt.engine.core.common.queries.QueryType;
import org.ovirt.engine.core.compat.Guid;
Expand All @@ -26,14 +27,16 @@ protected BackendDiskSnapshotResource(String id, BackendDiskSnapshotsResource pa

@Override
public DiskSnapshot get() {
DiskSnapshot diskSnapshot = performGet(QueryType.GetDiskSnapshotByImageId,
new IdQueryParameters(guid), Disk.class);
DiskImage diskImage = runQuery(QueryType.GetDiskSnapshotByImageId,
new IdQueryParameters(guid)).getReturnValue();
DiskSnapshot diskSnapshot = map(diskImage, null);
diskSnapshot.setDisk(new Disk());
diskSnapshot.getDisk().setId(diskId.toString());
diskSnapshot.getDisk().setHref(backendDiskSnapshotsResource.buildParentHref(diskId.toString(), false));
diskSnapshot.setHref(backendDiskSnapshotsResource.buildHref(diskId.toString(), diskSnapshot.getId().toString()));
if (diskSnapshot.getParent() != null) {
diskSnapshot.getParent().setHref(backendDiskSnapshotsResource.buildHref(diskId.toString(),
diskSnapshot.setHref(backendDiskSnapshotsResource.buildHref(diskId.toString(), diskSnapshot.getId()));
Guid parentDiskId = diskImage.getParentDiskId();
if (parentDiskId != null && diskSnapshot.getParent() != null) {
diskSnapshot.getParent().setHref(backendDiskSnapshotsResource.buildHref(parentDiskId.toString(),
diskSnapshot.getParent().getId()));
}
return diskSnapshot;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,18 @@ public DiskSnapshots list() {
protected DiskSnapshots mapCollection(List<org.ovirt.engine.core.common.businessentities.storage.Disk> entities) {
DiskSnapshots collection = new DiskSnapshots();
for (org.ovirt.engine.core.common.businessentities.storage.Disk disk : entities) {
DiskSnapshot diskSnapshot = getMapper(org.ovirt.engine.core.common.businessentities.storage.Disk.class, DiskSnapshot.class).map(disk, null);
DiskSnapshot diskSnapshot = getMapper(org.ovirt.engine.core.common.businessentities.storage.Disk.class, DiskSnapshot.class)
.map(disk, null);
diskSnapshot.setDisk(new Disk());
diskSnapshot.getDisk().setId(this.diskId.toString());
diskSnapshot.getDisk().setId(disk.getId().toString());
collection.getDiskSnapshots().add(addLinks(populate(diskSnapshot, disk), Disk.class));
diskSnapshot.setHref(buildHref(diskId.toString(), diskSnapshot.getId().toString()));
if (diskSnapshot.getParent() != null) {
diskSnapshot.getParent().setHref(buildParentHref(diskId.toString(), false));
diskSnapshot.setHref(buildHref(disk.getId().toString(), diskSnapshot.getId()));
Guid parentDiskId = disk.getParentDiskId();
if (parentDiskId != null) {
diskSnapshot.getParent().setHref(buildHref(parentDiskId.toString(),
diskSnapshot.getParent().getId()));
} else if (diskSnapshot.getParent() != null) {
diskSnapshot.getParent().setHref(buildHref(disk.getId().toString(), diskSnapshot.getParent().getId()));
}
}
return collection;
Expand Down

0 comments on commit 80af2c8

Please sign in to comment.