Consolidate more XML parsing, FileLike usages #929
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates XML parsing and FileLike usage across the LabKey platform by standardizing XML document builder creation and migrating from File to FileLike objects for better virtual file system support.
- Replace custom DocumentBuilderFactory instantiation with centralized XmlBeansUtil.DOCUMENT_BUILDER_FACTORY
- Migrate File-based operations to FileLike for improved virtual file system compatibility
- Update zip extraction and file handling to use FileLike APIs consistently
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| protein/src/org/labkey/protein/ProteinServiceImpl.java | Replaces local DocumentBuilderFactory with XmlBeansUtil.DOCUMENT_BUILDER_FACTORY |
| flow/src/org/labkey/flow/controllers/WorkspaceData.java | Migrates File to FileLike for temporary directory handling |
| flow/enginesrc/org/labkey/flow/persist/AnalysisSerializer.java | Updates extractArchive method to use FileLike for zip extraction operations |
| flow/enginesrc/org/labkey/flow/analysis/model/CompensationMatrix.java | Consolidates XML document builder creation using XmlBeansUtil |
| flow/enginesrc/org/labkey/flow/Main.java | Migrates zip extraction logic from File to FileLike |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // TODO: with `setNamespaceAware(true)`, is the following still needed? | ||
| // DocumentBuilderFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.NAMESPACES_FEATURE, true) |
There was a problem hiding this comment.
The TODO comment should be properly formatted as a Javadoc comment or regular comment block, and the question should be resolved or tracked in a proper issue tracker rather than left as inline code comments.
| // TODO: with `setNamespaceAware(true)`, is the following still needed? | |
| // DocumentBuilderFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.NAMESPACES_FEATURE, true) | |
| /* | |
| * The following line was previously used to enable namespace awareness: | |
| * DocumentBuilderFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.NAMESPACES_FEATURE, true) | |
| * If you need to revisit namespace handling, see the relevant documentation or issue tracker. | |
| */ |
There was a problem hiding this comment.
Based on my quick analysis, seems like setNamespaceAware() is sufficient and will stomp over the setFeature() value anyway
| } | ||
|
|
||
| public static File extractArchive(File file, File tempDir) | ||
| public static File extractArchive(File file, FileLike tempDir) |
There was a problem hiding this comment.
The method signature is inconsistent - it accepts a File parameter but returns a File while using FileLike for tempDir. Consider updating the return type to FileLike or the input parameter to FileLike for API consistency.
| // TODO: with `setNamespaceAware(true)`, is the following still needed? | ||
| // DocumentBuilderFactory.setFeature(Constants.SAX_FEATURE_PREFIX + Constants.NAMESPACES_FEATURE, true) |
There was a problem hiding this comment.
Based on my quick analysis, seems like setNamespaceAware() is sufficient and will stomp over the setFeature() value anyway
Rationale
Related Pull Requests
Changes