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

A rest method with an accessor method name #40

Open
gabor-farkas opened this issue May 27, 2016 · 3 comments
Open

A rest method with an accessor method name #40

gabor-farkas opened this issue May 27, 2016 · 3 comments

Comments

@gabor-farkas
Copy link
Contributor

Let's suppose I have a rest endpoint class like:

public class MyService {
  public enum SomeStatus {
    ACTIVE, INACTIVE;
  }
  @GET
  @Path("/status")
  public getSomeStatus() {
    return SomeStatus.ACTIVE;
  }
}

This currently causes:

Caused by: java.lang.NullPointerException
        at java2typescript.jaxrs.ServiceDescriptorGenerator.decorateParamNames(ServiceDescriptorGenerator.java:262)
        at java2typescript.jaxrs.ServiceDescriptorGenerator.generateTypeScript(ServiceDescriptorGenerator.java:161)
        at org.java2typescript.maven.plugin.MainMojo.execute(MainMojo.java:95)

This is because java2typescript.jackson.module.visitors.TSJsonObjectFormatVisitor#isAccessorMethod identifies this as an accessor method.
What would be the best way to solve this? Maybe the isAccessorMethod could look for jax-rs annotations?

@atsu85
Copy link
Collaborator

atsu85 commented May 27, 2016

Hi @gabor-farkas

What would be the best way to solve this? Maybe the isAccessorMethod could look for jax-rs annotations?

It could be made configurable through Configuration. Current logic from isAccessorMethod could be moved into Configuration and if You want to override that behavior, You could extend Configuration to override the default logic. See the test for custom ignored methods that are added to the configuration - You could pass a subclass of Configuration the same way as the configuration is passed from the test.

@atsu85
Copy link
Collaborator

atsu85 commented May 27, 2016

What do You think about my suggestion?

To be honest, probably the decision of including/excluding the method (isPublic, isAccessor, isIgnored) probably should have been delegated entirely to the Configuration, but as I'm not using JAX-RS myself, i didn't think about this use-case at the time when i implemented the isIgnoredMethod. The Configuration.isIgnoredMethod would be now a good candidate for also containing logic from isPublic and isAccessor methods, but If it would be done now, it would break backwards compatibility (not sure if that argument is strong enough).
Also to avoid growing the class too much, the default implementation should not be directly included in the Configuration class itself, but included as a strategy, just like NamingStrategy.

@gabor-farkas
Copy link
Contributor Author

gabor-farkas commented May 30, 2016

Hmm, shouldn't the jax-rs behaviour be followed?
I'm new to this framework, but if I understand things right, the 'problem' is that the same visitor is used to map data transfer classes to typescript interfaces that is used to read metadata about the rest service class itself. And actually, the methods from the data transfer classes shouldn't be written to the typescript declaration (data transfer classes might have helper methods on them, but they are only needed in Java), so we only need public methods from the rest service class itself.

For example, let's say I have an entity named Customer, and then I have an ApiCustomerVO. The API maps some data, resolves some dictionary items and also adds some additional fields fetched from other entities (list of contact phone numbers maybe). Now if the ApiCustomerVO has two methods:

  • public static ApiCustomerVO fromCustomer(Customer customer);
  • public Customer toCustomer();

The both these two methods will be output to the ApiCustomerVO in the d.ts file, and also the Customer class will be processed, though it's clearly not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants