Skip to content

[682] Correct the fix for 670 and return the type if the type has alr… #683

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.

Sign up for GitHub

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

Open
jamezp wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[682] Correct the fix for 670 and return the type if the type has alr… #683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion src/main/java/org/eclipse/yasson/internal/ReflectionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public static Type resolveTypeArguments(ParameterizedType typeToResolve, Type ty
}
}

if (resolvedArgs[i] == null || resolvedArgs[i].equals(typeToResolve)) {
if (resolvedArgs[i] == null) {
if (typeToSearch instanceof Class) {
return Object.class;
}
Expand All @@ -262,6 +262,10 @@ public static Type resolveTypeArguments(ParameterizedType typeToResolve, Type ty
typeToSearch));
}
}
// The expected type and the resolved type are the same, simply return the type
if (resolvedArgs[i].equals(typeToResolve)) {
return typeToResolve;
}
if (resolvedArgs[i] instanceof ParameterizedType) {
resolvedArgs[i] = resolveTypeArguments((ParameterizedType) resolvedArgs[i], typeToSearch);
} else if (unresolvedArg instanceof GenericArrayType) {
Expand Down
25 changes: 22 additions & 3 deletions src/test/java/org/eclipse/yasson/defaultmapping/generics/GenericsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.eclipse.yasson.defaultmapping.generics.model.GenericArrayClass;
import org.eclipse.yasson.defaultmapping.generics.model.GenericTestClass;
import org.eclipse.yasson.defaultmapping.generics.model.GenericWithUnboundedWildcardClass;
import org.eclipse.yasson.defaultmapping.generics.model.ListContainer;
import org.eclipse.yasson.defaultmapping.generics.model.LowerBoundTypeVariableWithCollectionAttributeClass;
import org.eclipse.yasson.defaultmapping.generics.model.MultiLevelExtendedGenericTestClass;
import org.eclipse.yasson.defaultmapping.generics.model.MultipleBoundsContainer;
Expand Down Expand Up @@ -516,14 +517,32 @@ public void wildcardCollectionContainer() {
}

@Test
public void genericUpperBoundContainer() {
final String expectedJson = "{\"tree\":{\"children\":[{\"children\":[],\"name\":\"child\"}],\"name\":\"parent\"}}";
public void genericUpperBoundContainer() throws Exception {
final String expectedJson = "{\"tree\":{\"children\":[{\"name\":\"child\"}],\"name\":\"parent\"}}";
final TreeContainer<TreeElement> container = new TreeContainer<>();
final TreeElement parent = new TreeElement("parent");
parent.setChildren(List.of(new TreeElement("child")));
container.setTree(parent);

assertEquals(expectedJson, defaultJsonb.toJson(container));
// Use a new instance of Jsonb to avoid any caching
try (var jsonb = JsonbBuilder.create()) {
assertEquals(expectedJson, jsonb.toJson(container));
}
Comment on lines +527 to +530
Copy link
Member

@KyleAure KyleAure Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Use a new instance of Jsonb to avoid any caching
try (var jsonb = JsonbBuilder.create()) {
assertEquals(expectedJson, jsonb.toJson(container));
}
// Use a new instance of Jsonb to avoid any caching
try (var jsonb = JsonbBuilder.create()) {
assertEquals(expectedJson, jsonb.toJson(container));
TreeContainer<TreeElement> result = jsonb.fromJson(expectedJson, new TestTypeToken<TreeContainer<TreeElement>>() {}.getType());
assertIterableEquals(container.getTree().getChildren(), result.getTree().getChildren());
}

Requires addition of:

import static org.junit.jupiter.api.Assertions.assertIterableEquals;


}

@Test
public void genericUpperBoundContainerWithListContainer() throws Exception {
final String expectedJson = "{\"list\":[{\"children\":[{\"name\":\"child\"}],\"name\":\"parent\"}]}";
final ListContainer<TreeElement> container = new ListContainer<>();
final TreeElement parent = new TreeElement("parent");
parent.setChildren(List.of(new TreeElement("child")));
container.setList(List.of(parent));

// Use a new instance of Jsonb to avoid any caching
try (var jsonb = JsonbBuilder.create()) {
assertEquals(expectedJson, jsonb.toJson(container));
}
Comment on lines +542 to +545
Copy link
Member

@KyleAure KyleAure Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Use a new instance of Jsonb to avoid any caching
try (var jsonb = JsonbBuilder.create()) {
assertEquals(expectedJson, jsonb.toJson(container));
}
// Use a new instance of Jsonb to avoid any caching
try (var jsonb = JsonbBuilder.create()) {
assertEquals(expectedJson, jsonb.toJson(container));
ListContainer<TreeElement> result = jsonb.fromJson(expectedJson, new TestTypeToken<ListContainer<TreeElement>>() {}.getType());
assertIterableEquals(container.getList(), result.getList());
}

Requires addition of:

import static org.junit.jupiter.api.Assertions.assertIterableEquals;


}

Expand Down
33 changes: 33 additions & 0 deletions src/test/java/org/eclipse/yasson/defaultmapping/generics/model/ListContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright (c) 2025 IBM, Inc. and/or its affiliates.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0,
* or the Eclipse Distribution License v. 1.0 which is available at
* http://www.eclipse.org/org/documents/edl-v10.php.
*
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
*/

package org.eclipse.yasson.defaultmapping.generics.model;

import java.util.List;

/**
*
* @author <a href="mailto:jperkins@ibm.com">James R. Perkins</a>
*/
public class ListContainer<T> {

private List<T> list;

public List<T> getList() {
return list;
}

public void setList(List<T> list) {
this.list = list;
}

}
14 changes: 1 addition & 13 deletions src/test/java/org/eclipse/yasson/defaultmapping/generics/model/TreeElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,12 @@

package org.eclipse.yasson.defaultmapping.generics.model;

import java.util.ArrayList;
import java.util.List;

/**
* @author <a href="mailto:jperkins@redhat.com">James R. Perkins</a>
*/
public class TreeElement implements TreeTypeContainer<TreeElement> {
public class TreeElement extends TreeTypeContainer<TreeElement> {

private final String name;
Copy link
Member

@KyleAure KyleAure Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private final String name;
private String name;
public TreeElement() {
}

suggestion: allow for deserialization

private List<TreeElement> children = new ArrayList<>();

public TreeElement(final String name) {
this.name = name;
Expand All @@ -30,12 +26,4 @@ public TreeElement(final String name) {
public String getName() {
return name;
}

public List<TreeElement> getChildren() {
return children;
}

public void setChildren(final List<TreeElement> children) {
this.children = children;
}
}
Copy link
Member

@KyleAure KyleAure Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
public void setName(final String name) {
this.name = name;
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
if (!super.equals(o)) {
return false;
}
TreeElement that = (TreeElement) o;
return name != null ? name.equals(that.name) : that.name == null;
}
}

suggestion: allow for deserialization and assertions.

11 changes: 8 additions & 3 deletions src/test/java/org/eclipse/yasson/defaultmapping/generics/model/TreeTypeContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@
/**
* @author <a href="mailto:jperkins@redhat.com">James R. Perkins</a>
*/
public interface TreeTypeContainer<T extends TreeTypeContainer<T>> {
public class TreeTypeContainer<T extends TreeTypeContainer<T>> {
private List<T> children;

List<T> getChildren();
public List<T> getChildren() {
return children;
}

void setChildren(List<T> children);
public void setChildren(final List<T> children) {
this.children = children;
}
}
Copy link
Member

@KyleAure KyleAure Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
TreeTypeContainer<?> that = (TreeTypeContainer<?>) o;
if (children == null) {
return that.children == null;
}
return children.containsAll(that.children) && that.children.containsAll(children);
}
}

Suggestion: allow for assertions.