Conversation
- Updated Maven compiler plugin from 3.8.1 to 3.11.0 - Changed Java release from 11 to 21 - Modernized switch statement in setPolicyId() to use switch expressions - Replaced string concatenations with text blocks for better readability - Used String.formatted() instead of String.format() and concatenation - Implemented pattern matching in extractUserId() method - Added pattern matching with instanceof in headersAndResponseSummaryTable() - Improved code readability and maintainability using Java 21 features Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace String.formatted() calls with String.format() for better compatibility - Keep text blocks and other Java 21 features that work correctly - Maintain all modernization improvements while ensuring compilation success Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Updated maven-compiler-plugin from 3.11.0 to 3.13.0 for proper Java 21 support - Added explicit maven.compiler.source and maven.compiler.target properties - Verified successful compilation and packaging with Java 21 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Changed from <release>21</release> to <source>21</source> and <target>21</target> - Successfully compiles and packages with Java 21 - All Java 21 language features working correctly - Build output shows 'debug target 21' confirming proper Java 21 usage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Updated zap-clientapi from 1.16.0 to 1.17.0 - Verified successful compilation and packaging with Java 21 - Ensures latest security updates and improved compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Changed Maven compiler source/target from 21 to 17 - Reverted Java 21-specific pattern matching to Java 17 compatible code - Maintained all other modern features (switch expressions, text blocks) - Successfully compiles and packages with Java 17 - Java 17 is LTS and more widely supported Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR upgrades the project from Java 11 to Java 17, leveraging modern Java features including text blocks, pattern matching for instanceof, and switch expressions. The changes modernize the codebase while maintaining the same functionality.
Changes:
- Upgraded Java version from 11 to 17 and updated Maven compiler plugin to 3.13.0
- Refactored string concatenation to use text blocks for improved HTML template readability
- Replaced traditional switch statements with modern switch expressions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/scanner/ScannerMethods.java | Modernized code to use Java 17 features: text blocks for HTML strings, pattern matching instanceof, switch expressions, and improved String.format usage |
| pom.xml | Updated Java source/target to 17, upgraded Maven compiler plugin to 3.13.0, and updated zap-clientapi to 1.17.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| null, null, null, null, String.format("%s%s - %d", reportName, date, seconds), null, | ||
| String.format("%s/%s", System.getProperty("user.dir"), dir), null); |
There was a problem hiding this comment.
The report output path is built with a hardcoded '/' separator (String.format("%s/%s", ...)), which is not portable across OSes. Prefer dir.getAbsolutePath() or Paths.get(System.getProperty("user.dir"), dir.getPath()).toString() / File(base, child) to construct the path safely.
| null, null, null, null, String.format("%s%s - %d", reportName, date, seconds), null, | |
| String.format("%s/%s", System.getProperty("user.dir"), dir), null); | |
| null, null, null, null, String.format("%s%s - %d", reportName, date, seconds), null, | |
| new File(System.getProperty("user.dir"), dir.getPath()).getAbsolutePath(), null); |
| <maven.compiler.version>3.13.0</maven.compiler.version> | ||
| <maven.compiler.source>17</maven.compiler.source> | ||
| <maven.compiler.target>17</maven.compiler.target> | ||
| <github.url>https://maven.pkg.github.com/dvsa/vol-app-security-lib</github.url> |
There was a problem hiding this comment.
<maven.compiler.source> and <maven.compiler.target> are defined as properties, but the maven-compiler-plugin config below hardcodes 17 instead of referencing these properties. Consider wiring the plugin values to the properties (or removing the unused properties) to avoid configuration drift.
| <configuration> | ||
| <release>11</release> | ||
| <source>17</source> | ||
| <target>17</target> | ||
| </configuration> |
There was a problem hiding this comment.
Consider using 17 instead of separate
| </tr> | ||
| <p></p> | ||
| <p></p> | ||
| <p></p> | ||
| <p></p>""", this.reportURL, this.reportURL); |
There was a problem hiding this comment.
This summary table HTML inserts multiple
elements directly inside a (outside any /| ) and doesn’t close the table, which is invalid markup and can render inconsistently. Consider using CSS spacing or spacer |
| rows, and ensure the table is properly closed. |
| <table width=45%% border=0> | ||
| <tr bgcolor=#666666> | ||
| <h3>Server Requests and Responses</h3><div class=spacer></div> | ||
| <td width=50%%>Request</td> | ||
| <td width=50%%><p>Response</p></td>"""); |
There was a problem hiding this comment.
The header markup is not valid table structure:
Description
Related issue: JIRA_TICKET_NUMBER
Before submitting (or marking as "ready for review")