[REQ][Java][Jersey2] Generate java method for each oneOf/anyOf

[sebastien-rosset]

Is your feature request related to a problem? Please describe.

Currently, in the generated Java jersey2 library, a class is generated for every oneOf, anyOf schema in the OpenAPI document. Every oneOf/anyOf class inherits from AbstractOpenApiSchema. The AbstractOpenApiSchema.java class has the public Object getActualInstance() and Object getActualInstanceRecursively() methods which both return an Object. There is no type information in the return value besides being an Object.

IMO, there are two problems that could be improved:

1. It's hard to tell what are the possible oneOf/anyOf classes returned by getActualInstance() (documentation issue)
2. The developer must cast the return value of getActualInstance() and getActualInstanceRecursively()

Describe the solution you'd like

Suppose the OpenAPI document has oneOf: [X, Y, Z]. It would be helpful to:

1. Generate documentation in the derived class indicating that getActualInstance can be a X, Y or Z. This could be done by overriding getActualInstance() which would just invoke super.getActualInstance(), and there would be generated method comments listing the possible type values.
2. Add getter methods for each oneOf/anyOf (e.g. X, Y, Z). If we generate getX(), getY(), getZ(), the developer can invoke these generated methods instead of casting the return value. That way, the developers can no longer cast the return value to the wrong class. They can still invoke the wrong method, which presumably would cause a run time exception such as ClassCastException.

Describe alternatives you've considered

Currently, the developer must read the oneOf or anyOf sections of the OpenAPI document, find out what are the possible types. Then when calling getActualInstance(), the developer must cast the return value to the expected class.

Additional context

This would work well if the oneOf/anyOf schemas don't have recursion and if the oneOf/anyOf children are concrete. If there is recursion or if there are abstract classes involved, the developer may still have to cast the return value, but it would still be an improvement over what we have now.

[wing328]

@sebastien-rosset thanks for the feedback. Agreed with these improvements. I'll see what I can do to make it better.

[bkabrda]

@sebastien-rosset I would also love the oneOf implementation to get better. Could you please provide a specific example of code where having getX(), getY() and getZ() brings benefit? I'm trying to figure out how I'd use it and I can't see what the benefit would be - currently I use:

Object o = foo.getActualInstance();
if (o instanceof X) {
  X x = (X) o;
  // do something with x
}

and with what you propose I'll have do

try {
  X x = foo.getX();
  // do something with x
} catch (ClassCastException e) {
  // do nothing
}

What I'm trying to say is that I don't see a benefit in LoC/how the code reads/how fast you write the code/how the code is performant. Could you explain the use case where this helps?

[sebastien-rosset]

In practice I see the problems below:

// How do you know getActualInstance can return an instance of X, Y and Z?
// How do you know getActualInstance cannot return an instance of W?
// How do you know you are handling all possible classes?
Object o = foo.getActualInstance();
if (o instanceof X) {  
  X x = (X) o;
  // do something with x
}

1. Documentation issue: how do developers even know what are all the possible returned classes in getActualInstance? In practice this depends on the level of familiarity with the associated OpenAPI doc. If you are both the author and Java SDK user, then presumably you would know. But there are also casual users who don't know much about the OpenAPI doc structure. You can infer by reading the generated code but not in the javadoc. For example, you can read the setActualInstance code or the static block of code or the generated constructors. You can also infer by reading the generated deserialize method, but it's very verbose. This could be easily improved by generating javadoc comments. Users should be able to read the javadoc and understand the code flow.
2. Sometimes at build time developers know the exact type that will be returned even though it's a oneOf. For example, potentially the oneOf class could depend on the request URL query parameters, so if the developer sets a query parameter, she knows the response is going to be of a specific type. In that case, the code may simply be just one line as shown below, and maybe the developer has a catch all at a higher level to catch logic bugs.

X x = foo.getX();

I'm not advocating that we eliminate the getActualInstance method. If we also generate these getters, developers will know the exact set of classes.

[bkabrda]

Ok, that makes sense, thanks for the explanation. That said, we have to ensure that we have protection set to ensure there are no method name clashes (e.g. if someone had a oneOf which had actualInstance as a member, the class would suddenly have two getActualInstance methods and wouldn't compile).

[wing328]

@bkabrda good catch about the edge case. We'll handle it when there's a report on the issue.