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.

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

Jump to bottom

incorrect generic typing in operations of EntityGraph #424

Closed
gavinking opened this issue Jul 3, 2023 · 4 comments · Fixed by #430
Closed

incorrect generic typing in operations of EntityGraph #424

gavinking opened this issue Jul 3, 2023 · 4 comments · Fixed by #430

Comments

@gavinking
Copy link
Contributor

gavinking commented Jul 3, 2023 •
edited
Loading

There are several problems in the EntityGraph interface:

  1. Calling addAttributeNodes(Attribute<T, ?>... attribute) produces the infamous "Unchecked generics array creation for varargs parameter" compiler warning.
  2. addAttributeNodes(Attribute<T, ?>... attribute) should be addAttributeNodes(Attribute<? super T, ?>... attribute)
  3. <X> Subgraph<X> addSubgraph(Attribute<T, X> attribute) should be<X> Subgraph<X> addSubgraph(Attribute<? super T, X> attribute)
  4. I'm not 100% sure, but I believe that the signature of <X> Subgraph<X> addKeySubgraph(Attribute<T, X> attribute) is simply wrong. I think it should be <X> Subgraph<X> addKeySubgraph(MapAttribute<T, X, ?> attribute).

We need to put some thought into these.
@gavinking
Copy link
Contributor Author

Additionally, I'm very confused by this method:

    /**
     * Add a node to the graph that corresponds to a managed
     * type with inheritance.  This allows for multiple subclass
     * subgraphs to be defined for this node of the entity
     * graph. Subclass subgraphs will automatically include the
     * specified attributes of superclass subgraphs. 
     *
     * @param attribute  attribute
     * @param type  entity subclass
     * @return subgraph for the attribute
     * @throws IllegalArgumentException if the attribute's target 
     *         type is not a managed type
     * @throws IllegalStateException if the EntityGraph has been 
     *         statically defined
     */
    public <X> Subgraph<? extends X> addSubgraph(Attribute<T, X> attribute, Class<? extends X> type);

The Javadoc is extremely cryptic, and the signature is weird. Is it supposed to be doing the equivalent of a treat()? If so, shouldn't the signature be:

public <X, Y extends X> Subgraph<Y> addSubgraph(Attribute<T, X> attribute, Class<?Y> type);

because with the present signature it doesn't seem like you could do anything useful with it, assuming my interpretation is correct.

@gavinking
Copy link
Contributor Author

I'm also confused by this one:

public <X> Subgraph<? extends X> addSubclassSubgraph(Class<? extends X> type);

which actually makes no sense even without knowing the semantics: Java has wildcard capture, so you don't need the wildcards there.

Perhaps it should be:

public <X extends T> Subgraph<X> addSubclassSubgraph(Class<X> type);

gavinking added a commit to gavinking/persistence that referenced this issue Jul 3, 2023
@gavinking
Proposal to fix signatures of operations of entity graphs
9c9dd2d 
Introduces AbstractGraph as a place to declare common operations of EntityGraph and SubGraph

See jakartaee#424
@gavinking gavinking mentioned this issue Jul 3, 2023
Closed
@gavinking
Copy link
Contributor Author

Please see #425 for one set of ideas for how to "fix" all this.

@lukasj lukasj linked a pull request Aug 4, 2023 that will close this issue
Proposal to fix signatures of operations of entity graphs #425
Closed
@lukasj
Copy link
Contributor

lukasj commented Aug 4, 2023

this supersedes #411

@lukasj lukasj mentioned this issue Aug 4, 2023
Closed
gavinking added a commit to gavinking/persistence that referenced this issue Aug 8, 2023
@gavinking
introduce Graph for common operations of EntityGraph and Subgraph
23a3e96 
in preparation for jakartaee#424
gavinking added a commit to gavinking/persistence that referenced this issue Aug 8, 2023
@gavinking
introduce Graph.addAttributeNode() and use correct wildcard type
cd521ef 
Needed because of "Unchecked generics array creation for varargs parameter"
compiler warning for the varargs version.

see jakartaee#424
gavinking added a commit to gavinking/persistence that referenced this issue Aug 8, 2023
@gavinking
use correct upper bounded wildcard type in Graph.addSubgraph()
9a8d1db 
see jakartaee#424
gavinking added a commit to gavinking/persistence that referenced this issue Aug 8, 2023
@gavinking
clean up addXxxxxSubgraph() operations
2287c06 
- fix incorrect generic signatures
- add way to navigate to collection elements

see jakartaee#424, jakartaee#411
@gavinking gavinking mentioned this issue Aug 8, 2023
Closed
gavinking added a commit to gavinking/persistence that referenced this issue Aug 9, 2023
@gavinking
clean up addXxxxxSubgraph() operations
546f511 
- fix incorrect generic signatures
- add way to navigate to collection elements

see jakartaee#424, jakartaee#411
@gavinking gavinking mentioned this issue Aug 9, 2023
Closed
gavinking added a commit to gavinking/persistence that referenced this issue Aug 10, 2023
@gavinking
introduce Graph for common operations of EntityGraph and Subgraph
1de18cb 
in preparation for jakartaee#424
gavinking added a commit to gavinking/persistence that referenced this issue Aug 10, 2023
@gavinking
introduce Graph.addAttributeNode() and use correct wildcard type
096f84a 
Needed because of "Unchecked generics array creation for varargs parameter"
compiler warning for the varargs version.

see jakartaee#424
gavinking added a commit to gavinking/persistence that referenced this issue Aug 10, 2023
@gavinking
use correct upper bounded wildcard type in Graph.addSubgraph()
2e726d3 
see jakartaee#424
gavinking added a commit to gavinking/persistence that referenced this issue Aug 10, 2023
@gavinking
clean up addXxxxxSubgraph() operations
1e6346c 
- fix incorrect generic signatures
- add way to navigate to collection elements

see jakartaee#424, jakartaee#411
gavinking added a commit to gavinking/persistence that referenced this issue Aug 10, 2023
@gavinking
mark deprecated methods on Graph/EntityGraph forRemoval=true
5f08951 
These operations have simply incorrect generic types, and we now have replacements.

jakartaee#424
@lukasj lukasj linked a pull request Aug 10, 2023 that will close this issue
fix signatures of operations of entity graphs #430
Merged
@lukasj lukasj mentioned this issue Aug 10, 2023
Merged
gavinking added a commit to gavinking/persistence that referenced this issue Aug 10, 2023
@gavinking
copy/paste new Graph APIs into the spec
5d43938 
jakartaee#424
lukasj pushed a commit that referenced this issue Aug 10, 2023
@gavinking @lukasj
introduce Graph for common operations of EntityGraph and Subgraph
f71a5c4 
in preparation for #424
lukasj pushed a commit that referenced this issue Aug 10, 2023
@gavinking @lukasj
introduce Graph.addAttributeNode() and use correct wildcard type
b41dd02 
Needed because of "Unchecked generics array creation for varargs parameter"
compiler warning for the varargs version.

see #424
lukasj pushed a commit that referenced this issue Aug 10, 2023
@gavinking @lukasj
use correct upper bounded wildcard type in Graph.addSubgraph()
0b91076 
see #424
lukasj pushed a commit that referenced this issue Aug 10, 2023
@gavinking @lukasj
clean up addXxxxxSubgraph() operations
b8adb1a 
- fix incorrect generic signatures
- add way to navigate to collection elements

see #424, #411
lukasj pushed a commit that referenced this issue Aug 10, 2023
@gavinking @lukasj
mark deprecated methods on Graph/EntityGraph forRemoval=true
f2a7f61 
These operations have simply incorrect generic types, and we now have replacements.

#424
lukasj pushed a commit that referenced this issue Aug 10, 2023
@gavinking @lukasj
copy/paste new Graph APIs into the spec
f2835cb 
#424
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix signatures of operations of entity graphs gavinking/persistence
Proposal to fix signatures of operations of entity graphs gavinking/persistence
2 participants
@gavinking @lukasj