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

Create a new recipe that adds the missing <?jelly escape-by-default='true'?> line at the beginning of Jelly files #1

Open
gounthar opened this issue Oct 28, 2024 · 0 comments · May be fixed by #2

Comments

@gounthar
Copy link
Owner

What feature do you want to see added?

While working on the AnchorChain plugin, I discovered that several Jelly files were missing the required XML declaration:
<?jelly escape-by-default='true'?>

According to the Jenkins documentation, when migrating the <description> tag from pom.xml to a Jelly file, this declaration is mandatory.

While there is an existing OpenRewrite recipe that handles the file creation, it doesn't address cases where the file exists but lacks the required declaration.

Questions for Discussion:

  1. Which existing PluginModernizer recipe would be most appropriate to incorporate the org.openrewrite.jenkins.CreateIndexJelly recipe?

  2. What would be the best approach to implement the missing declaration fix:

Your guidance on these implementation approaches would be appreciated.

Upstream changes

No response

Are you interested in contributing this feature?

Yes. In the meantime, I will give a try at org.openrewrite.xml.XsltTransformation.

Implementation Update: Handling Jelly Files in OpenRewrite

As discussed in Slack, the solution requires specific configuration for processing Jelly files:

  1. File Processing Configuration:
  • We need to configure additionalPlainTextMasks to treat .jelly files as plain text
  • This is necessary because Jelly files cannot be parsed as Quarks, making them inaccessible to standard recipes
  1. Available Approaches:
  • Use PlainTextVisitor for modifications
  • Create a custom recipe that converts plain text to parsed XML
  1. Limitations:

This configuration requirement explains why standard XML processing approaches aren't applicable in this case.

Proposal

A code not specific to openrewrite but to the plugin modernizer already exist there: gounthar/plugin-modernizer-tool#52

To contribute your AddJellyXmlDeclaration recipe to the OpenRewrite project, follow these steps:
Fork the OpenRewrite Repository: If you haven't already, fork the OpenRewrite repository on GitHub.
Clone Your Fork: Clone your forked repository to your local machine.
git clone https://github.com/your-username/openrewrite.git
cd openrewrite
Create a New Branch: Create a new branch for your feature.
git checkout -b add-jelly-xml-declaration
Add Your Recipe: Add your AddJellyXmlDeclaration recipe to the appropriate module in the OpenRewrite project. Typically, this would be in a module like rewrite-core or rewrite-java.
Recipe Class: Place your recipe class in the appropriate package.
Test Class: Add a test class to verify your recipe.
Here is an example structure:
openrewrite
├── rewrite-core
│ ├── src
│ │ ├── main
│ │ │ └── java
│ │ │ └── org
│ │ │ └── openrewrite
│ │ │ └── jelly
│ │ │ └── AddJellyXmlDeclaration.java
│ │ ├── test
│ │ │ └── java
│ │ │ └── org
│ │ │ └── openrewrite
│ │ │ └── jelly
│ │ │ └── AddJellyXmlDeclarationTest.java
Implement the Recipe: Ensure your recipe class is correctly implemented. Here is your AddJellyXmlDeclaration class:
package org.openrewrite.jelly;

import org.openrewrite.ExecutionContext;
import org.openrewrite.Recipe;
import org.openrewrite.text.PlainText;
import org.openrewrite.text.PlainTextVisitor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**

  • Recipe to add an XML declaration to Jelly files.
    */
    public class AddJellyXmlDeclaration extends Recipe {

    private static final Logger LOG = LoggerFactory.getLogger(AddJellyXmlDeclaration.class);

    @OverRide
    public String getDisplayName() {
    return "Add XML declaration to Jelly files";
    }

    @OverRide
    public String getDescription() {
    return "Ensure the XML declaration <?jelly escape-by-default='true'?> is present in all .jelly files.";
    }

    @OverRide
    public PlainTextVisitor getVisitor() {
    return new PlainTextVisitor() {
    public static final String JELLY_DECLARATION = "";

         @Override
         public PlainText visitText(PlainText text, ExecutionContext executionContext) {
             if (text == null || text.getSourcePath() == null) {
                 return text;
             }
             if (text.getSourcePath().toString().endsWith(".jelly")) {
                 LOG.debug("Processing Jelly file: {}", text.getSourcePath());
                 String content = text.getText();
                 if (content.trim().isEmpty()) {
                     LOG.debug("Adding declaration to empty file");
                     return text.withText(JELLY_DECLARATION);
                 }
                 String lineEnding = content.contains("\r\n") ? "\r\n" : "\n";
                 if (content.trim().toLowerCase().matches("^<\\?jelly\\s+[^>]*>") && !content.startsWith(JELLY_DECLARATION)) {
                     LOG.warn("Found malformed Jelly declaration in {}", text.getSourcePath());
                     LOG.debug("Adding missing declaration");
                     content = content.substring(content.indexOf(lineEnding) + lineEnding.length());
                 }
                 if (!content.startsWith(JELLY_DECLARATION)) {
                     LOG.debug("Declaration already present");
                     content = JELLY_DECLARATION + lineEnding + content;
                     return text.withText(content);
                 }
             }
             return text;
         }
     };
    

    }
    }
    Write Tests: Ensure you have tests to verify your recipe. Here is your AddJellyXmlDeclarationTest class:
    package org.openrewrite.jelly;

import static org.openrewrite.test.SourceSpecs.text;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.openrewrite.Recipe;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.text.PlainTextParser;

/**

  • Test class for the AddJellyXmlDeclaration recipe.
    */
    public class AddJellyXmlDeclarationTest implements RewriteTest {

    @OverRide
    public Recipe getRecipe() {
    return new AddJellyXmlDeclaration();
    }

    @test
    void addXmlDeclarationToJellyFile(@tempdir Path tempDir) throws IOException {
    Path inputFile = tempDir.resolve("example.jelly");
    Files.writeString(inputFile, """
    <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define">
    <st:contentType value="text/html"/>

    Hello, World!


    </j:jelly>
    """);

     Path expectedFile = tempDir.resolve("expected.jelly");
     Files.writeString(expectedFile, """
         <?jelly escape-by-default='true'?>
         <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define">
             <st:contentType value="text/html"/>
             <h1>Hello, World!</h1>
         </j:jelly>
     """);
    
     rewriteRun(
             spec -> spec.recipe(new AddJellyXmlDeclaration())
                     .parser(PlainTextParser.builder())
                     .expectedCyclesThatMakeChanges(1)
                     .cycles(1),
             text(Files.readString(inputFile), Files.readString(expectedFile)));
    

    }

    @test
    void doNotAddXmlDeclarationIfAlreadyPresent(@tempdir Path tempDir) throws IOException {
    Path inputFile = tempDir.resolve("example.jelly");
    Files.writeString(inputFile, """

    <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define">
    <st:contentType value="text/html"/>

    Hello, World!


    </j:jelly>
    """);

     rewriteRun(
             spec -> spec.recipe(new AddJellyXmlDeclaration())
                     .parser(PlainTextParser.builder())
                     .expectedCyclesThatMakeChanges(1)
                     .cycles(0),
             text(Files.readString(inputFile)));
    

    }
    }
    Build and Test: Build the project and run the tests to ensure everything works correctly.
    mvn clean install
    Commit and Push: Commit your changes and push the branch to your fork.
    git add .
    git commit -m "Add recipe to add XML declaration to Jelly files"
    git push origin add-jelly-xml-declaration
    Create a Pull Request: Go to your fork on GitHub and create a pull request to the main OpenRewrite repository.
    Follow Up: Monitor the pull request for any feedback or requested changes from the maintainers.
    By following these steps, you can propose your new feature to the OpenRewrite project.

gounthar added a commit that referenced this issue Oct 28, 2024
Fixes #1

Implement the `AddJellyXmlDeclaration` recipe to add the XML declaration to Jelly files.

* **AddJellyXmlDeclaration.java**
  - Implement the `AddJellyXmlDeclaration` class to add the XML declaration to Jelly files.
  - Use `PlainTextVisitor` to modify the content of `.jelly` files.
  - Ensure the XML declaration `<?jelly escape-by-default='true'?>` is present in all `.jelly` files.

* **AddJellyXmlDeclarationTest.java**
  - Implement the `AddJellyXmlDeclarationTest` class to test the `AddJellyXmlDeclaration` recipe.
  - Write tests to verify the XML declaration is added to Jelly files.
  - Ensure tests cover cases where the declaration is already present and where it is missing.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/gounthar/rewrite-jenkins/issues/1?shareId=XXXX-XXXX-XXXX-XXXX).
@gounthar gounthar linked a pull request Oct 28, 2024 that will close this issue
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 a pull request may close this issue.

1 participant