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

[BUG] Regression with location: ANNOTATION in java provider, not seeing rule match #620

Open
1 task done
jwmatthews opened this issue Jun 13, 2024 · 3 comments
Open
1 task done
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@jwmatthews
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Konveyor version

Latest as of June 13 2024

Priority

Undefined (Default)

Current Behavior

We noticed a regression where a custom rule that used to match no longer does.
Below is the information

Source file, using an '@Remote'
src/main/java/org/jboss/as/quickstarts/ejb/remote/stateful/CounterBean.java

package org.jboss.as.quickstarts.ejb.remote.stateful;

import javax.ejb.Remote;
import javax.ejb.Stateful;

/**
 * @author Jaikiran Pai
 */
@Stateful
@Remote(RemoteCounter.class)
public class CounterBean implements RemoteCounter {

    private int count = 0;

    @Override
    public void increment() {
        this.count++;
    }

    @Override
    public void decrement() {
        this.count--;
    }

    @Override
    public int getCount() {
        return this.count;
    }
}

Link to custom rule

  ruleID: remote-ejb-to-quarkus-00000
  when:
    or:
    - java.referenced:
        location: ANNOTATION
        pattern: javax.ejb.Remote
    - java.referenced:
        location: ANNOTATION
        pattern: jakarta.ejb.Remote

Link to full output.yaml

- name: kai/quarkus
  description: Quarkus focused rules to help migrate from Java EE
  unmatched:
  - remote-ejb-to-quarkus-00000

We originally suspected this may be related to the sample app having 2 different modules in it for client and server-side. I ran Kantra against those individual modules, i.e. ran against the 'server-side' module, and I still did not see the custom rule for the Remote annotation match.

At this point, I think the issue may be related to the ANNOTATION location.

Expected Behavior

Assume we'd have a match like this:

[](- name: kai/quarkus
description: Quarkus focused rules to help migrate from Java EE
violations:
remote-ejb-to-quarkus-00000:
description: Remote EJBs are not supported in Quarkus
category: mandatory
labels:
- konveyor.io/source=jakarta-ee
- konveyor.io/source=java-ee
- konveyor.io/target=quarkus
incidents:
- uri: file:///tmp/source-code/server-side/src/main/java/org/jboss/as/quickstarts/ejb/remote/stateful/CounterBean.java
message: "Remote EJBs are not supported in Quarkus, and therefore its use must be removed and replaced with REST functionality. In order to do this:\n 1. Replace the @Remote annotation on the class with a @jakarta.ws.rs.Path(\"<endpoint>\") annotation. An endpoint must be added to the annotation in place of <endpoint> to specify the actual path to the REST service.\n 2. Remove @Stateless annotations if present. Given that REST services are stateless by nature, it makes it unnecessary.\n 3. For every public method on the EJB being converted, do the following:\n - Annotate the method with @jakarta.ws.rs.GET\n - Annotate the method with @jakarta.ws.rs.Path(\"<endpoint>\") and give it a proper endpoint path. As a rule of thumb, the method name can be used as endpoint, for instance:\n \n @Path(\"/increment\")\n public void increment() \n \n - Add @jakarta.ws.rs.QueryParam(\"<param-name>\") to any method parameters if needed, where <param-name> is a name for the parameter."
codeSnip: " 1 /*\n 2 * JBoss, Home of Professional Open Source\n 3 * Copyright 2015, Red Hat, Inc. and/or its affiliates, and individual\n 4 * contributors by the @authors tag. See the copyright.txt in the\n 5 * distribution for a full listing of individual contributors.\n 6 *\n 7 * Licensed under the Apache License, Version 2.0 (the "License");\n 8 * you may not use this file except in compliance with the License.\n 9 * You may obtain a copy of the License at\n 10 * http://www.apache.org/licenses/LICENSE-2.0\n 11 * Unless required by applicable law or agreed to in writing, software\n 12 * distributed under the License is distributed on an "AS IS" BASIS,\n 13 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n 14 * See the License for the specific language governing permissions and\n 15 * limitations under the License.\n 16 */\n 17 package org.jboss.as.quickstarts.ejb.remote.stateful;\n 18 \n 19 import javax.ejb.Remote;\n 20 import javax.ejb.Stateful;\n 21 \n 22 /\n 23 * @author Jaikiran Pai\n 24 /\n 25 @stateful\n 26 @Remote(RemoteCounter.class)\n 27 public class CounterBean implements RemoteCounter {\n 28 \n 29 private int count = 0;\n 30 \n 31 @OverRide\n 32 public void increment() {\n 33 this.count++;\n 34 }\n 35 \n 36 @OverRide\n 37 public void decrement() {\n 38 this.count--;\n 39 }\n 40 \n 41 @OverRide\n 42 public int getCount() {\n 43 return this.count;\n 44 }\n 45 }\n"
lineNumber: 26
variables:
file: file:///tmp/source-code/server-side/src/main/java/org/jboss/as/quickstarts/ejb/remote/stateful/CounterBean.java
kind: Class
name: Stateful
package: org.jboss.as.quickstarts.ejb.remote.stateful
- uri: file:///tmp/source-code/server-side/src/main/java/org/jboss/as/quickstarts/ejb/remote/stateless/CalculatorBean.java
message: "Remote EJBs are not supported in Quarkus, and therefore its use must be removed and replaced with REST functionality. In order to do this:\n 1. Replace the @Remote annotation on the class with a @jakarta.ws.rs.Path(\"<endpoint>\") annotation. An endpoint must be added to the annotation in place of <endpoint> to specify the actual path to the REST service.\n 2. Remove @Stateless annotations if present. Given that REST services are stateless by nature, it makes it unnecessary.\n 3. For every public method on the EJB being converted, do the following:\n - Annotate the method with @jakarta.ws.rs.GET\n - Annotate the method with @jakarta.ws.rs.Path(\"<endpoint>\") and give it a proper endpoint path. As a rule of thumb, the method name can be used as endpoint, for instance:\n \n @Path(\"/increment\")\n public void increment() \n \n - Add @jakarta.ws.rs.QueryParam(\"<param-name>\") to any method parameters if needed, where <param-name> is a name for the parameter."
codeSnip: " 1 /
\n 2 * JBoss, Home of Professional Open Source\n 3 * Copyright 2015, Red Hat, Inc. and/or its affiliates, and individual\n 4 * contributors by the @authors tag. See the copyright.txt in the\n 5 * distribution for a full listing of individual contributors.\n 6 *\n 7 * Licensed under the Apache License, Version 2.0 (the "License");\n 8 * you may not use this file except in compliance with the License.\n 9 * You may obtain a copy of the License at\n 10 * http://www.apache.org/licenses/LICENSE-2.0\n 11 * Unless required by applicable law or agreed to in writing, software\n 12 * distributed under the License is distributed on an "AS IS" BASIS,\n 13 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n 14 * See the License for the specific language governing permissions and\n 15 * limitations under the License.\n 16 */\n 17 package org.jboss.as.quickstarts.ejb.remote.stateless;\n 18 \n 19 import javax.ejb.Remote;\n 20 import javax.ejb.Stateless;\n 21 \n 22 /
\n 23 * @author Jaikiran Pai\n 24 */\n 25 @stateless\n 26 @Remote(RemoteCalculator.class)\n 27 public class CalculatorBean implements RemoteCalculator {\n 28 \n 29 @OverRide\n 30 public int add(int a, int b) {\n 31 return a + b;\n 32 }\n 33 \n 34 @OverRide\n 35 public int subtract(int a, int b) {\n 36 return a - b;\n 37 }\n 38 }\n"
lineNumber: 26
variables:
file: file:///tmp/source-code/server-side/src/main/java/org/jboss/as/quickstarts/ejb/remote/stateless/CalculatorBean.java
kind: Class
name: Stateless
package: org.jboss.as.quickstarts.ejb.remote.stateless
links:
- url: https://jakarta.ee/specifications/restful-ws/
title: Jakarta RESTful Web Services
effort: 1)

How Reproducible

Always (Default)

Steps To Reproduce

I have a short reproducer here: https://github.com/jwmatthews/issue_reproducers/tree/main/kantra/ejb_remote

  1. git clone https://github.com/konveyor-ecosystem/ejb-remote
  2. Run: ./analyze.sh
    You can find the referenced custom rule at: https://github.com/jwmatthews/issue_reproducers/blob/main/kantra/ejb_remote/custom_rules/03-remote-ejb-to-quarkus.windup.yaml

Environment

Running on MacOS arm64 


./kantra version
version: latest
SHA: a89ffa3bce466bb46ecc658a2b07e7d2bec69af1
image: quay.io/konveyor/kantra

Anything else?

This appears to be a regression as we saw matches for this rule + sample application about 3 months ago, here is an example we checked in from an older notebook example.

https://github.com/konveyor-ecosystem/kai/blob/main/notebooks/ejb_remote/analysis_report/ejb-remote/output.yaml#L1278

This is related to an issue we are tracking in kai: konveyor/kai#204

There is another open issue on java.referenced location ANNOTATION, don't think it's related, but will link for awareness:
#508

@jwmatthews jwmatthews added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 13, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in Planning Jun 13, 2024
@konveyor-ci-bot
Copy link

This issue is currently awaiting triage.
If contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.
The triage/accepted label can be added by org members.

@konveyor-ci-bot konveyor-ci-bot bot added the needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. label Jun 13, 2024
@pranavgaikwad pranavgaikwad self-assigned this Jun 20, 2024
@pranavgaikwad pranavgaikwad added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates an issue or PR lacks a `priority/foo` label and requires one. labels Jun 20, 2024
@dymurray dymurray added this to the v0.5.0 milestone Jun 20, 2024
@dymurray dymurray moved this from 🆕 New to 🔖 Ready in Planning Jun 20, 2024
@dymurray dymurray modified the milestones: v0.5.0, v0.5.1 Jul 18, 2024
@shawn-hurley
Copy link
Contributor

When running this locally, I notice that the search scope for the project is empty, and that is why we are getting zero results.

At this time, I can only think of two reasons:

  1. I typo'ed the test environment
  2. That the unresolvable POM is causing jdt.LS to not be able to create any search scopes.

I will continue to work on this, as source only mode should be able to work even with this broken pom file.

@dymurray dymurray removed this from the v0.5.1 milestone Aug 1, 2024
@shawn-hurley
Copy link
Contributor

shawn-hurley commented Aug 1, 2024

I am postive that this is currently the more correct behavior, and the reasoning is this.

Currently this application, does not compile, and more importantly does not have any references to either Jakarta or Javax.

I think what is happening, is becuas the language server does not have these types, when we are asking for them, it is unable to find any reference for them. I tried to get this POM in a place that was usable, but was unable to.

My primary concern is two fold:

  1. If we are able to find these, it means that we are finding things that the language server is just guessing at, we did a lot of work to make this more stable in the 0.5 timeframe and I worry that going back will cause similar problems
  2. That folks have built up a system of rules based on the old behavior that now doesn't work, and we are breaking them.

I don't have a good way forward, as both have huge pitfalls in the overall health of the results and to users.

@dymurray @jwmatthews @fabianvf @pranavgaikwad @jmle Looking for help and guidance on what our next steps should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Status: 🔖 Ready
Development

No branches or pull requests

4 participants