ActionProcessor, OptionsPanelControllerProcessor, and maybe others have custom code to check for the existence of a named resource. This should be consolidated into some properly tested utility method in LayerGeneratingProcessor. For example:
protected final byte validateResource(Element originatingElement, String resource, boolean searchClasspath) throws LayerBuildingException;
SOURCE_PATH should always be checked, and CLASS_OUTPUT currently as well (to work around a javac bug affecting Maven projects). Optionally, also check CLASS_PATH and perhaps PLATFORM_CLASS_PATH (but definitely not ANNOTATION_PROCESSOR_PATH or SOURCE_OUTPUT). Should throw LBE if not found.
Can return byte contents in case the processor wishes to perform additional checks on the content (as e.g. bundlevalue does now). Returning an InputStream is slightly more efficient, though it forces processors which do not wish to perform any further checks to remember to close it. (@CheckReturnValue, if openide.filesystems could depend on that.)
May be useful to also add a parameter for the responsible annotation (or annotation type?), if bug #194545 is also implemented.
In principle it could be useful to have a boolean flag to permit the resource to be either absolute or relative to the package of the originatingElement, as e.g. Class.getResource (as opposed to e.g. ClassLoader.getResource). Currently I do not know of any NB annotations which permit relative resource paths, however. Even if they did, something would probably need to first compute the absolute resource path for emitting to a layer, so that code could pass the absolute path to the validate method.
(In reply to comment #0)
> Optionally, also check CLASS_PATH
Unfortunately I have found that this does not work in JDK 6 javac - even if the JAR containing the icon is moved to the front of the classpath (ahead of build/classes and so on), the filer produces
java.io.FileNotFoundException: .../release/modules/ext/whatever.jar/path/to/icon.gif (Not a directory)
resulting in a "Cannot find iconBase file..." error from the processor. The same command invocation works in JDK 7 javac. Have not yet tracked down the javac bug, but this is a potentially blocking issue when using @ActionRegistration or any other annotation which permits a resource to be found on the classpath and not just in the sourcepath. (Encountered in a customer module including jlfgr-1_0.jar on the classpath for standard icons, which is a reasonable use case.)
Found the fix in JDK 7. Probably need to escalate a backport.
Filed this problem as bug #196933.
Created attachment 109497 [details]
'hg pdi validateResource_196452' to get current patch.
Please review. Tests are pending, though ActionProcessorTest does some of the testing.
I don't like absolutizeResource much. Perhaps this is a way to avoid it:
Y01 Rather than having validateResource on LGP, you could have new method on LayerBuilder.File:
public void stringFromResourceValue(String attrName, String path, boolean classpath, boolean canBeRelative, ...)
The method would validate if the resource exists (either relative or absolute) and would generate the proper form of the attribute (absolutized by default I guess). In this system absolutizeResource method is not needed, as it is handled by the processor under the cover.
There is a lot of somevalue in LayerBuilder.File anyway, and new ones are appearing everyday, so I guess this would be fine approach.
(In reply to comment #6)
> Y01 ... would generate the proper form of the attribute
> (absolutized by default I guess).
This style was my initial thought (see comment #0), but it did not work out well: the processor may need to do various things with the absolute result, including constructing URLs. Rather than trying to shoehorn two independent tasks - absolutizing a path, and validating a resource - into a single method, it seemed better to keep them separate and leave it up to the caller to combine them where and how desired.
Another thing to note is that validating a resource with classpath=true cannot work in JDK 6 javac (the impl will silently accept any value), so it is not in general possible to detect whether an ambiguous path format is in fact relative or absolute.
A variant method I have considered (but which is not proposed in this patch) would treat "o/n/m/x/resources/x.png" as absolute, but would also permit "./resources/x.png" or "../resources/x.png" as relative paths. This would provide a compatible enhancement for the many annotations which require an absolute resource path for e.g. iconBase, but where a relative path would be more convenient in some cases, since the forms with ./ or ../ segments would not be permitted by the current processors. Again it would be clearest for this to be a simple added utility method which could be used with validateResource, rather than complicating vR's signature with such an option.
(In reply to comment #7)
> (In reply to comment #6)
> A variant method I have considered (but which is not proposed in this patch)
> would treat "o/n/m/x/resources/x.png" as absolute, but would also permit
> "./resources/x.png" or "../resources/x.png" as relative paths.
This way you don't need an option to ask whether stringFromAResource method accepts relative or absolute values. It would always accept both.
> rather than complicating vR's signature with such an option.
The only option needed now would be whether the value should always be converted to absolute or can be left relative.
Btw. it would be more OS-like and Class.getResource()-like if absolute paths started with / and relative would not.
(In reply to comment #8)
> This way you don't need an option to ask whether stringFromAResource method
> accepts relative or absolute values. It would always accept both.
Not good enough - some annotations specify that "resources/x.png" is permitted as a relative path. And as previously mentioned, it is not always possible to infer whether this is absolute or relative.
> The only option needed now would be whether the value should always be
> converted to absolute or can be left relative.
Not necessarily, because some processors need to do other things with the result than store it as a stringvalue, as previously mentioned.
> it would be more [...] Class.getResource()-like if absolute paths
> started with / and relative would not.
That is what absolutizeResource does.
Integrated into 'main-golden'
User: Jesse Glick <firstname.lastname@example.org>
Log: #196452: way for a LayerGeneratingProcessor to validate a resource path.
*** Bug 212077 has been marked as a duplicate of this bug. ***