Webapp changes #61
Webapp changes #61
Conversation
- Changed version from 1.2.1-SNAPSHOT-webapp to 1.2.1-SNAPSHOT - Added dependencyManagement section to lock scanner-core at 6.2.2 - Removed explicit version from scanner-core dependency This allows Jenkins CI to find artifacts in Nexus while maintaining version consistency locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ix deserialization issues Version: - Moving Scanner-Core dependency from scanner-core 5.5.0 to 6.2.3 Fixes: 1. Add Jackson deserialization support to ScanResult - Add @JsonCreator annotation to private constructor - Add @JsonProperty annotations for all constructor parameters - Map 'resultStatus' JSON field to 'jobStatus' Java field - Fixes MongoDB deserialization via MongoJack 2. Fix Javadoc compilation error in IPersistenceProvider - Remove @param limit documentation for non-existent parameter 3. Implement missing methods in DummyPersistenceProvider - Add getScanResultsByTarget() returning empty list - Add getScanResultById() returning null - Fixes test compilation after interface updates
…l_results # Conflicts: # pom.xml # src/main/java/de/rub/nds/crawler/core/BulkScanWorker.java # src/main/java/de/rub/nds/crawler/data/ScanJobDescription.java # src/main/java/de/rub/nds/crawler/data/ScanResult.java # src/test/java/de/rub/nds/crawler/dummy/DummyPersistenceProvider.java
# Conflicts: # src/main/java/de/rub/nds/crawler/core/BulkScanWorker.java
# Conflicts: # src/main/java/de/rub/nds/crawler/core/BulkScanWorker.java
There was a problem hiding this comment.
Pull Request Overview
This PR implements changes to support a new webapp for TLS-Scanner-WS, adding improved error handling, documentation, and retrieval capabilities for scan results. The main changes focus on enhancing data persistence, improving code documentation, and updating dependencies.
Key changes:
- Added new methods to retrieve scan results by target and ID from the persistence layer
- Enhanced Jackson deserialization configuration to handle missing/unknown properties
- Added comprehensive Javadoc documentation across multiple classes and interfaces
- Updated scanner-core dependency from 5.5.0 to 6.2.3
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| MongoPersistenceProvider.java | Implemented getScanResultsByTarget and getScanResultById methods, configured Jackson mapper for flexible deserialization, and added logging |
| IPersistenceProvider.java | Added interface methods for retrieving scan results by target and ID |
| DummyPersistenceProvider.java | Implemented stub methods for the new persistence interface methods |
| ScanResult.java | Added JsonCreator annotation and default constructor for Jackson deserialization |
| ScanJobDescription.java | Added UUID field for unique scan job identification |
| CancellableFuture.java | Added comprehensive Javadoc and renamed parameters for clarity |
| CanceallableThreadPoolExecutor.java | Added comprehensive Javadoc documentation |
| Multiple data/config classes | Added Javadoc documentation for classes, methods, and fields |
| pom.xml | Updated scanner-core dependency version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/de/rub/nds/crawler/persistence/MongoPersistenceProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/de/rub/nds/crawler/persistence/MongoPersistenceProvider.java
Outdated
Show resolved
Hide resolved
XoMEX
left a comment
XoMEX
left a comment
There was a problem hiding this comment.
The docstrings are often mediocre (I've pushed improvements).
Setting the result ID when writing to the DB is odd and should be avoided.
Some minor comments (also check copilot review)
src/main/java/de/rub/nds/crawler/config/WorkerCommandConfig.java
Outdated
Show resolved
Hide resolved
|
|
||
| public class ScanJobDescription implements Serializable { | ||
|
|
||
| private final UUID id = UUID.randomUUID(); |
There was a problem hiding this comment.
What is this UUID used for? The BulkScan already has an ID.
| this.result = result; | ||
| } | ||
|
|
||
| public ScanResult() { |
There was a problem hiding this comment.
IIRC jackson constructors can be private (and in this case should be, as no one else should call this constructor)
| throw new IllegalArgumentException( | ||
| "ScanResult status does not match ScanJobDescription status"); | ||
| } | ||
| scanResult.setId(scanJobDescription.getId().toString()); // <- Add this line |
There was a problem hiding this comment.
it seems super odd to set the id here. IMO the ID should be final (which I do not know why it isn't yet) and set in the constructor.
|
|
||
| @Override | ||
| public List<ScanResult> getScanResultsByTarget( | ||
| String dbName, String collectionName, String target) { |
There was a problem hiding this comment.
Why not use the ScanTarget class here?
| mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); | ||
| mapper.configure(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES, false); | ||
| mapper.configure(DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES, false); |
There was a problem hiding this comment.
Do we need this set? This feels like treating the symptoms of failing deserialization rather than fixing why deserialization fails.
…rovider.java fixed typo error Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rovider.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Implement changes for the new webapp in TLS-Scanner-WS