Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong type in relationships section when using lookup by id and resource is a subtype #862

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import io.crnk.core.engine.registry.ResourceRegistry;
import io.crnk.core.engine.result.Result;
import io.crnk.core.engine.result.ResultFactory;
import io.crnk.core.exception.InvalidResourceException;
import io.crnk.core.exception.RepositoryNotFoundException;
import io.crnk.core.exception.ResourceNotFoundException;
import io.crnk.core.repository.response.JsonApiResponse;
Expand All @@ -26,6 +27,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -127,6 +129,7 @@ public Result<Set<Resource>> lookupRelatedResourcesWithId(IncludeRequest request
Set<Resource> related = new HashSet<>();

Set<Object> relatedIdsToLoad = new HashSet<>();
Map<Object, List<ResourceIdentifier>> mapRelatedIdsToLoadToResourceIdentifier = new HashMap<>();
for (Resource sourceResource : sourceResources) {
Relationship relationship = sourceResource.getRelationships().get(relationshipField.getJsonName());
PreconditionUtil.verify(relationship.getData().isPresent(), "expected relationship data to be loaded for @JsonApiResourceId annotated field, sourceType=%d sourceId=%d, relationshipName=%s", sourceResource.getType(), sourceResource.getId(), relationshipField.getJsonName());
Expand All @@ -138,6 +141,9 @@ public Result<Set<Resource>> lookupRelatedResourcesWithId(IncludeRequest request
related.add(request.getResource(id));
} else {
relatedIdsToLoad.add(oppositeResourceInformation.parseIdString(id.getId()));
// ResourceIdentifier may have the wrong type, e.g. when resource is a subtype of declared type in source resource,
// so we store the resource identifier to be able to set the correct type later
mapRelatedIdsToLoadToResourceIdentifier.computeIfAbsent(id.getId(), k -> new ArrayList<>()).add(id);
}
}
}
Expand All @@ -153,6 +159,17 @@ public Result<Set<Resource>> lookupRelatedResourcesWithId(IncludeRequest request
Collection responseList = (Collection) response.getEntity();
for (Object responseEntity : responseList) {
Resource relatedResource = request.merge(responseEntity);
// ResourceIdentifier may have the wrong type, e.g. when resource is a subtype of declared type in source resource,
// so we set the correct type here
List<ResourceIdentifier> resourceIdentifiers = mapRelatedIdsToLoadToResourceIdentifier.get(relatedResource.getId());
if (resourceIdentifiers != null) {
for(ResourceIdentifier resourceIdentifier : resourceIdentifiers) {
resourceIdentifier.setType(relatedResource.getType());
}
} else {
throw new InvalidResourceException("type=" + relationshipField.getOppositeResourceType() + ", "
+ "id=" + relatedResource.getId() + " : There must be an issue with serializing this id.");
}
related.add(relatedResource);
Object responseEntityId = oppositeResourceInformation.getId(responseEntity);
relatedIdsToLoad.remove(responseEntityId);
Expand Down
4 changes: 4 additions & 0 deletions crnk-core/src/test/java/io/crnk/core/CoreTestModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
import io.crnk.core.mock.repository.RelationIdTestRepository;
import io.crnk.core.mock.repository.RelationshipBehaviorTestRepository;
import io.crnk.core.mock.repository.ScheduleRepositoryImpl;
import io.crnk.core.mock.repository.TopTaskRepository;
import io.crnk.core.mock.repository.TaskRepository;
import io.crnk.core.mock.repository.TaskToProjectRepository;
import io.crnk.core.mock.repository.TaskWithLookupRepository;
import io.crnk.core.mock.repository.TaskWithLookupToProjectRepository;
import io.crnk.core.mock.repository.ThingRepository;
import io.crnk.core.mock.repository.TopTaskWrapperRepository;
import io.crnk.core.mock.repository.UserRepository;
import io.crnk.core.mock.repository.UserToProjectRepository;
import io.crnk.core.mock.repository.UserToTaskRepository;
Expand All @@ -43,6 +45,8 @@ public void setupModule(ModuleContext context) {
context.addRepository(new TaskToProjectRepository());
context.addRepository(new TaskWithLookupRepository());
context.addRepository(new TaskWithLookupToProjectRepository());
context.addRepository(new TopTaskRepository());
context.addRepository(new TopTaskWrapperRepository());
context.addRepository(new UserRepository());
context.addRepository(new UserToProjectRepository());
context.addRepository(new UserToTaskRepository());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package io.crnk.core.engine.internal.document.mapper.lookup.relationid;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import io.crnk.core.engine.document.Document;
import io.crnk.core.engine.document.Resource;
import io.crnk.core.engine.document.ResourceIdentifier;
import io.crnk.core.engine.internal.document.mapper.AbstractDocumentMapperTest;
import io.crnk.core.mock.models.BottomTask;
import io.crnk.core.mock.models.RelationIdTestResource;
import io.crnk.core.mock.models.TopTask;
import io.crnk.core.mock.models.TopTaskWrapper;
import io.crnk.core.mock.repository.TopTaskRepository;
import io.crnk.core.queryspec.QuerySpec;
import io.crnk.core.repository.ResourceRepository;
import io.crnk.core.utils.Nullable;

public class SubTypedRelationIdLookupTest extends AbstractDocumentMapperTest {

public static final Long TEST_RESOURCE_ID = 1L;
public static final Long TASK_1_ID = 2L;
public static final Long TASK_2_ID = 3L;
public static final Long TASK_WRAPPER_1_ID = 4L;
public static final Long TASK_WRAPPER_2_ID = 5L;

private TopTaskRepository topTaskRepository;

private BottomTask bottomTask1;

private BottomTask bottomTask2;

private TopTaskWrapper topTaskWrapper1;

private TopTaskWrapper topTaskWrapper2;

@SuppressWarnings({"rawtypes", "unchecked"})
@Before
public void setup() {
super.setup();

topTaskRepository = (TopTaskRepository) (ResourceRepository) container.getRepository(TopTask.class);

bottomTask1 = createTask(TASK_1_ID);
topTaskRepository.save(bottomTask1);

bottomTask2 = createTask(TASK_2_ID);
topTaskRepository.save(bottomTask2);

topTaskWrapper1 = createTopTaskWrapper(TASK_WRAPPER_1_ID);
topTaskWrapper2 = createTopTaskWrapper(TASK_WRAPPER_2_ID);
}

private BottomTask createTask(final Long id) {
BottomTask task = new BottomTask();
task.setId(id);
task.setName("test" + id);
return task;
}

@Test
public void checkOnlyIdSet() {
check(false, true);
}

@Test
public void checkNull() {
check(false, false);
}

@Test
public void checkEntitySet() {
check(true, true);
}

private void check(boolean setRelatedEntity, boolean setRelatedId) {
RelationIdTestResource entity = new RelationIdTestResource();
entity.setId(TEST_RESOURCE_ID);
entity.setName("test");
entity.setTopTaskWrappers(Arrays.asList(topTaskWrapper1, topTaskWrapper2));
if (setRelatedId) {
entity.setTestSubTypedResourceId(TASK_1_ID);
topTaskWrapper1.setTaskId(bottomTask2.getId());
topTaskWrapper2.setTaskId(bottomTask2.getId());
}
if (setRelatedEntity) {
entity.setTestSubTypedResource(bottomTask1);
topTaskWrapper1.setTask(bottomTask2);
topTaskWrapper2.setTask(bottomTask2);
}

QuerySpec querySpec = new QuerySpec(RelationIdTestResource.class);
querySpec.includeRelation(Collections.singletonList("testSubTypedResource"));
querySpec.includeRelation(Arrays.asList("topTaskWrappers", "task"));

Document document = mapper.toDocument(toResponse(entity), toAdapter(querySpec), mappingConfig).get();
Resource resource = document.getSingleData().get();
Assert.assertEquals(TEST_RESOURCE_ID.toString(), resource.getId());
Assert.assertEquals("relationIdTest", resource.getType());
Assert.assertEquals("test", resource.getAttributes().get("name").asText());

Nullable<ResourceIdentifier> testSubTypedResourceId = resource.getRelationships().get("testSubTypedResource").getSingleData();
Assert.assertTrue(testSubTypedResourceId.isPresent());
Nullable<List<ResourceIdentifier>> topTaskWrappersIds = resource.getRelationships().get("topTaskWrappers").getCollectionData();
Assert.assertTrue(topTaskWrappersIds.isPresent());
Assert.assertNotNull(topTaskWrappersIds.get());
List<Resource> topTaskWrappers = document.getIncluded().stream().filter(it -> it.getType().equals("topTaskWrapper")).collect(Collectors.toList());
Assert.assertEquals(2, topTaskWrappers.size());

if (setRelatedId) {
Assert.assertNotNull(testSubTypedResourceId.get());
Assert.assertEquals("bottomTask", testSubTypedResourceId.get().getType());
Assert.assertEquals(TASK_1_ID.toString(), testSubTypedResourceId.get().getId());

List<ResourceIdentifier> taskIds = topTaskWrappers.stream().map(it -> it.getRelationships().get("task").getSingleData().get()).collect(Collectors.toList());
Assert.assertEquals(2, findResourceIdentifierByTypeAndId(taskIds, "bottomTask", TASK_2_ID).size());
Assert.assertEquals(0, findResourceIdentifierByTypeAndId(taskIds, "topTask", TASK_2_ID).size());

Assert.assertEquals(4, document.getIncluded().size());
Assert.assertEquals(1, findIncludedByTypeAndId(document.getIncluded(), "bottomTask", TASK_1_ID).size());
Assert.assertEquals(0, findIncludedByTypeAndId(document.getIncluded(), "topTask", TASK_1_ID).size());
Assert.assertEquals(1, findIncludedByTypeAndId(document.getIncluded(), "bottomTask", TASK_2_ID).size());
Assert.assertEquals(0, findIncludedByTypeAndId(document.getIncluded(), "topTask", TASK_2_ID).size());
} else {
Assert.assertNull(testSubTypedResourceId.get());
}
}

private Collection<Object> findIncludedByTypeAndId(final List<Resource> included, final String type, final Long id) {
return included.stream().filter(i -> i.getId().equals(id.toString()) && i.getType().equals(type)).collect(Collectors.toList());
}

private Collection<Object> findResourceIdentifierByTypeAndId(final List<ResourceIdentifier> ids, final String type, final Long id) {
return ids.stream().filter(i -> i.getId().equals(id.toString()) && i.getType().equals(type)).collect(Collectors.toList());
}

private TopTaskWrapper createTopTaskWrapper(final long id) {
final TopTaskWrapper topTaskWrapper = new TopTaskWrapper();
topTaskWrapper.setId(id);
return topTaskWrapper;
}
}
27 changes: 27 additions & 0 deletions crnk-core/src/test/java/io/crnk/core/mock/models/BottomTask.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.crnk.core.mock.models;

import io.crnk.core.resource.annotations.JsonApiResource;

@JsonApiResource(type = "bottomTask", resourcePath = "treeTasks")
public class BottomTask extends MiddleTask {

private boolean recurring;

private String end;

public boolean isRecurring() {
return recurring;
}

public void setRecurring(final boolean recurring) {
this.recurring = recurring;
}

public String getEnd() {
return end;
}

public void setEnd(final String end) {
this.end = end;
}
}
27 changes: 27 additions & 0 deletions crnk-core/src/test/java/io/crnk/core/mock/models/MiddleTask.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.crnk.core.mock.models;

import io.crnk.core.resource.annotations.JsonApiResource;

@JsonApiResource(type = "middleTask", subTypes = BottomTask.class, resourcePath = "treeTasks")
public abstract class MiddleTask extends TopTask {

private String publicComment;

private String privateComment;

public String getPublicComment() {
return publicComment;
}

public void setPublicComment(final String publicComment) {
this.publicComment = publicComment;
}

public String getPrivateComment() {
return privateComment;
}

public void setPrivateComment(final String privateComment) {
this.privateComment = privateComment;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public class RelationIdTestResource {
@JsonApiRelation(lookUp = LookupIncludeBehavior.AUTOMATICALLY_WHEN_NULL)
private Schedule testLookupWhenNull;


@JsonApiRelationId
private List<Long> testMultipleValueIds = new ArrayList<>();

Expand Down Expand Up @@ -81,6 +80,15 @@ public class RelationIdTestResource {
@JsonApiRelation(lookUp = LookupIncludeBehavior.AUTOMATICALLY_WHEN_NULL)
private Schedule testResourceIdRef;

@JsonApiRelationId
private Long testSubTypedResourceId;

@JsonApiRelation(lookUp = LookupIncludeBehavior.AUTOMATICALLY_WHEN_NULL)
private TopTask testSubTypedResource;

@JsonApiRelation(lookUp = LookupIncludeBehavior.NONE)
private List<TopTaskWrapper> topTaskWrappers = new ArrayList<>();

public Long getId() {
return id;
}
Expand Down Expand Up @@ -273,4 +281,30 @@ public void setTestResourceIdRef(Schedule testResourceIdRef) {
this.testResourceIdRefId = testResourceIdRef != null ?
new ResourceIdentifier(testResourceIdRef.getId().toString(), "schedules") : null;
}

public Long getTestSubTypedResourceId() {
return testSubTypedResourceId;
}

public void setTestSubTypedResourceId(final Long testSubTypedResourceId) {
this.testSubTypedResourceId = testSubTypedResourceId;
this.testSubTypedResource = null;
}

public TopTask getTestSubTypedResource() {
return testSubTypedResource;
}

public void setTestSubTypedResource(final TopTask testSubTypedResource) {
this.testSubTypedResource = testSubTypedResource;
this.testSubTypedResourceId = testSubTypedResource != null ? testSubTypedResource.getId() : null;
}

public List<TopTaskWrapper> getTopTaskWrappers() {
return topTaskWrappers;
}

public void setTopTaskWrappers(final List<TopTaskWrapper> topTaskWrappers) {
this.topTaskWrappers = topTaskWrappers;
}
}
40 changes: 40 additions & 0 deletions crnk-core/src/test/java/io/crnk/core/mock/models/TopTask.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package io.crnk.core.mock.models;

import io.crnk.core.resource.annotations.JsonApiId;
import io.crnk.core.resource.annotations.JsonApiResource;

@JsonApiResource(type = "topTask", subTypes = MiddleTask.class, resourcePath = "treeTasks")
public abstract class TopTask {

@JsonApiId
private Long id;

private String name;

private String category;

public Long getId() {
return id;
}

public TopTask setId(Long id) {
this.id = id;
return this;
}

public String getName() {
return name;
}

public void setName(@SuppressWarnings("SameParameterValue") String name) {
this.name = name;
}

public String getCategory() {
return category;
}

public void setCategory(final String category) {
this.category = category;
}
}
Loading