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

Provide codelenses for endpoints of the running app #140

Merged
merged 1 commit into from
Nov 29, 2019

Conversation

angelozerr
Copy link
Contributor

Provide codelenses for endpoints of the running app

Fixes #114

Signed-off-by: azerr azerr@redhat.com

@angelozerr
Copy link
Contributor Author

angelozerr commented Nov 12, 2019

This draft PR requires the draft PR redhat-developer/vscode-quarkus#146 (from vscode-quarkus):

image

First please don't review the code, I must clean it, but you can start play with the codelens but there are several limitation:

  • codelens should be displayed according the settings quarkus.tools.java.codelens.urlCodeLensEnabled.
  • click on URL codelens open a web browser with a Command, but we should check that client capability can manage this open URL
  • when Java text changed, the ICompilationUnit get from JDTUtils is not synchronized with the text editor. How to fix that? Not sure with that but Spring LS seems creating their own ICompilationUnit, I would like to avoid doing that to avoird parsing twice the Java content.
  • Codelens URL is displayed even if Quarkus application is not started, How to do that?
  • the base URL is hard coded, we should compute it by using port of application.properties and gives the capability to customize this base URL (in an array string)) when base URL cannot be computed (ex : base URL coming from a pom.xml)

@fbricon
Copy link
Collaborator

fbricon commented Nov 12, 2019

Before displaying the code lens, you might want to see if the url is accessible (check for connection refused)

@angelozerr
Copy link
Contributor Author

angelozerr commented Nov 12, 2019

Before displaying the code lens, you might want to see if the url is accessible (check for connection refused)

Indeed it's a limitation that I wrote: "Codelens URL is displayed even if Quarkus application is not started, How to do that?"

check for connection refused could work but when you start the Quarkus application, it should be nice to refresh Codelens and show the URL. That's why how to do that?

For local URL, an idea is to track the start / end of DevModeMain application:
image

@fbricon
Copy link
Collaborator

fbricon commented Nov 12, 2019

try https://stackoverflow.com/a/3584332/753170 before sending codelenses

@fbricon
Copy link
Collaborator

fbricon commented Nov 12, 2019

Slashes should be de-duplicated.
Screen Shot 2019-11-12 at 11 23 12 AM

Using

package foo.bar;

import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;

@Path("/salut")
public class HelloResource {
    
    @GET
    @Produces(MediaType.TEXT_PLAIN)
    @Path("/tadaa")
    //should generate http://localhost:8080/salut/tadaa instead of http://localhost:8080/salut//tadaa
    public String hello() {
        return "Salut";
    }
}

@angelozerr angelozerr force-pushed the codelens branch 4 times, most recently from 1999f83 to f673a3a Compare November 13, 2019 15:12
@angelozerr angelozerr force-pushed the codelens branch 14 times, most recently from d4ff83c to 4240573 Compare November 25, 2019 09:06
@angelozerr angelozerr marked this pull request as ready for review November 25, 2019 10:10
@angelozerr
Copy link
Contributor Author

I must write tests but you can play with PR. Please note this PR requires redhat-developer/vscode-quarkus#146

@xorye
Copy link

xorye commented Nov 25, 2019

works quite well on my end

@angelozerr angelozerr force-pushed the codelens branch 3 times, most recently from 56737fa to 3693ab6 Compare November 25, 2019 17:42
@angelozerr angelozerr force-pushed the codelens branch 4 times, most recently from b7c7869 to 983c939 Compare November 26, 2019 10:04
@fbricon
Copy link
Collaborator

fbricon commented Nov 26, 2019

adding static methods, they get codelenses. I don't think this is allowed, certainly not for private methods:

Screen Shot 2019-11-26 at 7 40 30 PM

@fbricon
Copy link
Collaborator

fbricon commented Nov 26, 2019

After adding quarkus.http.port=9090 to application.properties and reopening a JAX-RS resource, the codelenses still point toward 8080

}
IJavaElement[] elements = typeRoot.getChildren();
Collection<CodeLens> lenses = new LinkedHashSet<>(elements.length);
if (params.isUrlCodeLensEnabled()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

check for server available should happen here, only once

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I do that I check the server for any Java file. In my case I check the server only once for Java class bound with JAX-RS Path (just for the class not for the method), that's to say, the server check is done once.

Copy link
Collaborator

Choose a reason for hiding this comment

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

there's no need to iterate on all members if the server is not even started

Copy link
Contributor Author

@angelozerr angelozerr Nov 26, 2019

Choose a reason for hiding this comment

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

there's no need to iterate on all members if the server is not even started

I would like to avoid checking the server state of Java class which is not a JAX-RS resource (class which is not annotated with @path).

Given this java file:

@Path
public class A {

  public void f()

  public void g()
}

One check is done (only on A java element IType not for the methods)

Given this java file

class B {

}

None check on server state is done since B is no bound to @path.

@angelozerr angelozerr force-pushed the codelens branch 2 times, most recently from dc65b92 to 28d2f6e Compare November 26, 2019 23:03
@angelozerr
Copy link
Contributor Author

After adding quarkus.http.port=9090 to application.properties and reopening a JAX-RS resource, the codelenses still point toward 8080

It should work now, but please check that URL is available because sometimes when I change the value of port, the server is not restarted and keep the old port?

@angelozerr
Copy link
Contributor Author

angelozerr commented Nov 28, 2019

adding static methods, they get codelenses. I don't think this is allowed, certainly not for private methods:

You are right, after playing with JAX-RS, the rule is:

  • method must be public (static seems working)
  • and method must be annotated with @GET

@angelozerr angelozerr force-pushed the codelens branch 3 times, most recently from c0229c2 to c64c0d1 Compare November 29, 2019 09:41
@angelozerr
Copy link
Contributor Author

After adding quarkus.http.port=9090 to application.properties and reopening a JAX-RS resource, the codelenses still point toward 8080

It's an issue for Quarkus I think quarkusio/quarkus#5864

Fixes redhat-developer#114

Signed-off-by: azerr <azerr@redhat.com>
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

Successfully merging this pull request may close these issues.

Provide codelenses for endpoints of the running app
3 participants