We have been talking recently about making our APIs more test-friendly. One sore point is that the Filesystems API
documents that XML MIME resolvers are available, and even supplies their DTD, yet does not actually make them work if
you add them to your layer. (I don't think this is analogous to other API/SPI situations where there is a documented
pluggable SPI implemented in some other module and the API guarantees nothing. The API here offers an SPI - MIMEResolver
- yet also declares that a given XML format may be used instead.) Moving the resolver impl to Filesystems would solve
the issue of writing unit tests involving XML subtypes (cf. issue #138173).
Created attachment 63840 [details]
Proposed patch (still needs testing)
Please review this proposed change.
Note changes from current behavior (theoretically incompatible):
1. The old impl would have interpreted a resolver (with the correct doctype) in any location beneath Services. (Actually
a resolver would offer an InstanceCookie<MIMEResolver> in any location, but outside Services they would not have been
used in MIME resolution.) The new impl expects resolvers to be Services/MIMEResolver/*.xml, as in practice they always
are, and does not bother to verify that the public ID is set to the expected value in the doctype.
2. The old impl would have inserted the declarative resolvers in default lookup (assuming you were running with the
module system started and thus FolderLookup[Services included). The new impl does not.
The current patch does not attempt to reuse the parse of a FileObject between refreshes of the resolver list cache,
though this should be simple enough to add. It should also listen to changes in the resolver folder (new files, etc.)
and refresh the cache.
Y01 Do not use org/openide/filesystems/declmime package, use org.nb.modules.openide.fs.declmime
Y02 This is not really compatible change. If you want to sacrify compatibility, please give something important back.
For example, speed: change the code to create only one MimeResolverImpl and make the decision process in the impl
Y03 Eliminate public static ArrayList<String> getExtensionsAndMIMETypes(FileObject fo) somehow, please (core.xyz has
impl dependency on openide.fs and could access such method?)
Y04 Fix apisupport "File Type" template to use this
To Y04 - just means no unit test template is necessary any more; see issue #139253.
Y01 - can do.
Y02 - the advantage is clear: better testability. If I see any opportunity for easy optimization I will take it, but I
doubt it will have any real benefit, since we already order resolvers according to expected frequency of matches.
Y03 - no one has an impl dep on filesystems currently. The new method is needed from core.ui (for the new Options dialog
UI). Could perhaps duplicate a sufficient portion of the parser in core.ui to extract this information.
Created attachment 64115 [details]
Y01 and Y03 addressed; also fixing FileAssociationsModel's expectation that Lookup<MIMEResolver> will be useful here
Now reusing parses, and some other impl changes. core-main #d65ff2111b86
Integrated into 'main-golden', available in NB_Trunk_Production #324 build
User: Jesse Glick <email@example.com>
Log: Issue #138846: move the implementation of declarative MIME resolvers to the Filesystems API.