use core wvware

[andiwand]

No description provided.

[claude]

Claude finished @andiwand's task —— View job

---

Code Review for PR #425: use core wvware

Review Checklist:

• Analyze removed WvwareDocLoader functionality
• Review conanfile.txt dependency changes
• Check LoaderService.java modifications
• Review DocumentFragment.java changes
• Assess MS Word document support after removal
• Check for potential breaking changes
• Look for test coverage implications
• Provide comprehensive review feedback

---

Summary

This PR removes the separate WvwareDocLoader class and the wvware-android dependency, consolidating MS Word document support into the core library. The change simplifies the architecture by removing a dedicated loader class and relying on the native ODR core library's built-in wvware support.

Code Quality and Best Practices ✅

The changes follow good practices:

• Clean removal of unused code (60 lines deleted from WvwareDocLoader.java)
• Simplified loader management in LoaderService.java
• Consistent with the architecture's move toward using the core library for all document processing

Potential Issues 🔍

1. MS Word Support Coverage

The removed WvwareDocLoader specifically handled application/msword MIME type files. However, I notice:

• CoreLoader.isSupported() (lines 59-69 in CoreLoader.java) does NOT include application/msword in its supported types
• It only supports OpenDocument formats and OOXML formats (when doOoxml is true)
• The native core library does include wvware support (DecoderEngine::wvware in core_wrapper.cpp), but the Java layer doesn't recognize MS Word files

Impact: Legacy .doc files will fall back to RawLoader instead of being properly processed.

2. Missing MIME Type Handling

The WvwareDocLoader set the file type to application/msword after conversion (line 45). This metadata is lost with the removal.

3. Error Handling Differences

The WvwareDocLoader had specific error handling for password-related exceptions that mapped to EncryptedDocumentException. The core library has similar support (CoreEncryptedException), but the behavior might differ slightly.

Performance Considerations ⚡

• Positive: Removing a separate loader reduces the overhead of maintaining multiple document processing paths
• Positive: The native core library is likely more performant than the Java-based wvware-android wrapper
• Concern: The core library appears to load documents twice in some cases (see TODO comment in core_wrapper.cpp lines 202 and 342)

Security Concerns 🔒

No new security issues introduced. Password handling remains properly implemented through the core library.

Test Coverage 🧪

No test files were found that reference WvwareDocLoader, which suggests:

• Either tests weren't written for this functionality (concerning)
• Or the tests are integration-based and will continue to work through the core loader

Recommendation: Add explicit tests for MS Word document loading to ensure the functionality isn't regressed.

Recommendations 🛠️

1. Critical Fix: Update CoreLoader.isSupported() to include MS Word MIME types:

   @Override
   public boolean isSupported(Options options) {
       return options.fileType.startsWith("application/vnd.oasis.opendocument") ||
               // ... existing checks ...
               options.fileType.equals("application/msword") || // Add this
               (this.doOoxml && (...));
   }

2. Documentation: Update CLAUDE.md to remove references to WvwareDocLoader and clarify that MS Word support is now handled by CoreLoader

3. Testing: Add integration tests to verify MS Word documents (.doc files) are properly loaded through the core library

4. Validation: Test with various MS Word documents, especially:

   • Password-protected .doc files
   • Complex formatting documents
   • Legacy Word 97-2003 documents

Without fixing the isSupported() method, this PR will break MS Word document support for legacy .doc files.