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

🐛 Fix chaining in Java provider #721

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions demo-output.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,20 @@
kind: Class
name: Configuration
package: io.konveyor.demo.ordermanagement.config
java-chaining-01:
description: There should only be one instance of this rule
category: mandatory
incidents:
- uri: file:///examples/customers-tomcat-legacy/src/main/java/io/konveyor/demo/ordermanagement/OrderManagementAppInitializer.java
message: |
Sample message. This rule checks that the chaining conditions are working. Should only get a single issue.
codeSnip: " 3 import javax.servlet.ServletContext;\n 4 import javax.servlet.ServletException;\n 5 import javax.servlet.ServletRegistration;\n 6 \n 7 import org.springframework.web.WebApplicationInitializer;\n 8 import org.springframework.web.context.ContextLoaderListener;\n 9 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;\n10 import org.springframework.web.servlet.DispatcherServlet;\n11 \n12 \n13 public class OrderManagementAppInitializer implements WebApplicationInitializer {\n14 \n15 \t@Override\n16 \tpublic void onStartup(ServletContext container) throws ServletException {\n17 \t\tAnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();\n18 context.setConfigLocation(\"io.konveyor.demo.ordermanagement.config\");\n19 \n20 context.scan(\"io.konveyor.demo.ordermanagement\");\n21 container.addListener(new ContextLoaderListener(context));\n22 \n23 ServletRegistration.Dynamic dispatcher = container"
lineNumber: 13
variables:
file: file:///examples/customers-tomcat-legacy/src/main/java/io/konveyor/demo/ordermanagement/OrderManagementAppInitializer.java
kind: Class
name: OrderManagementAppInitializer
package: io.konveyor.demo.ordermanagement
java-downloaded-maven-artifact:
description: |
This rule tests the application downloaded from maven artifact
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ type referenceCondition struct {
Pattern string `yaml:"pattern"`
Location string `yaml:"location"`
Annotated annotated `yaml:"annotated,omitempty" json:"annotated,omitempty"`
Filepaths []string `yaml:"filepaths"`
}

type annotated struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/konveyor/analyzer-lsp/engine"
"github.com/konveyor/analyzer-lsp/jsonrpc2"
"github.com/konveyor/analyzer-lsp/lsp/protocol"
"github.com/konveyor/analyzer-lsp/provider"
Expand Down Expand Up @@ -53,11 +54,21 @@ func (p *javaServiceClient) Evaluate(ctx context.Context, cap string, conditionI
if err != nil {
return provider.ProviderEvaluateResponse{}, fmt.Errorf("unable to get query info: %v", err)
}
// filepaths get rendered as a string and must be converted
if cond.Referenced.Filepaths != nil {
cond.Referenced.Filepaths = strings.Split(cond.Referenced.Filepaths[0], " ")
}

condCtx := &engine.ConditionContext{}
err = yaml.Unmarshal(conditionInfo, condCtx)
if err != nil {
return provider.ProviderEvaluateResponse{}, fmt.Errorf("unable to get condition context info: %v", err)
}

if cond.Referenced.Pattern == "" {
return provider.ProviderEvaluateResponse{}, fmt.Errorf("provided query pattern empty")
}
symbols, err := p.GetAllSymbols(ctx, cond.Referenced.Pattern, cond.Referenced.Location, cond.Referenced.Annotated)
symbols, err := p.GetAllSymbols(ctx, *cond)
if err != nil {
p.log.Error(err, "unable to get symbols", "symbols", symbols, "cap", cap, "conditionInfo", cond)
return provider.ProviderEvaluateResponse{}, err
Expand Down Expand Up @@ -110,19 +121,19 @@ func (p *javaServiceClient) Evaluate(ctx context.Context, cap string, conditionI
}, nil
}

func (p *javaServiceClient) GetAllSymbols(ctx context.Context, query, location string, annotation annotated) ([]protocol.WorkspaceSymbol, error) {
func (p *javaServiceClient) GetAllSymbols(ctx context.Context, c javaCondition) ([]protocol.WorkspaceSymbol, error) {
// This command will run the added bundle to the language server. The command over the wire needs too look like this.
// in this case the project is hardcoded in the init of the Langauge Server above
// workspace/executeCommand '{"command": "io.konveyor.tackle.ruleEntry", "arguments": {"query":"*customresourcedefinition","project": "java"}}'
argumentsMap := map[string]interface{}{
"query": query,
"query": c.Referenced.Pattern,
"project": "java",
"location": fmt.Sprintf("%v", locationToCode[strings.ToLower(location)]),
"location": fmt.Sprintf("%v", locationToCode[strings.ToLower(c.Referenced.Location)]),
"analysisMode": string(p.config.AnalysisMode),
}

if !reflect.DeepEqual(annotation, annotated{}) {
argumentsMap["annotationQuery"] = annotation
if !reflect.DeepEqual(c.Referenced.Annotated, annotated{}) {
argumentsMap["annotationQuery"] = c.Referenced.Annotated
}

if p.includedPaths != nil && len(p.includedPaths) > 0 {
Expand Down Expand Up @@ -152,6 +163,19 @@ func (p *javaServiceClient) GetAllSymbols(ctx context.Context, query, location s
}
}

if c.Referenced.Filepaths != nil {
// filter according to the given filepaths
var filteredRefs []protocol.WorkspaceSymbol
for _, ref := range refs {
for _, fp := range c.Referenced.Filepaths {
if strings.HasSuffix(ref.Location.Value.(protocol.Location).URI, fp) {
filteredRefs = append(filteredRefs, ref)
}
}
}
return filteredRefs, nil
}

return refs, nil
}

Expand Down
2 changes: 1 addition & 1 deletion provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ func templateCondition(condition []byte, ctx map[string]engine.ChainTemplate) ([
s := strings.ReplaceAll(string(condition), `'{{`, "{{")
s = strings.ReplaceAll(s, `}}'`, "}}")

s, err := mustache.Render(s, true, ctx)
s, err := mustache.RenderRaw(s, true, ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why render raw vs render just for my knowledge :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were calling the Render method passing true as a second parameter, but the signature of Render doesn´t have a bool parameter. RenderRaw has, and I thought there was some reason in the past to use RenderRaw but the invocation was done mistakenly by calling Render instead...

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! that makes sense

if err != nil {
return nil, err
}
Expand Down
17 changes: 17 additions & 0 deletions rule-example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,20 @@
elements:
- name: basePackages
value: "io.konveyor.demo.ordermanagement.repository"
- ruleID: java-chaining-01
category: mandatory
description: "There should only be one instance of this rule"
message: |
Sample message. This rule checks that the chaining conditions are working. Should only get a single issue.
when:
and:
- java.referenced:
pattern: java.lang.Override
location: ANNOTATION
as: class
ignore: true
- java.referenced:
pattern: org.springframework.web.WebApplicationInitializer
location: IMPLEMENTS_TYPE
filepaths: "{{class.Filepaths}}"
jmle marked this conversation as resolved.
Show resolved Hide resolved
from: class
Loading