Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 351882 - ValidFileExtensions annotation should support el expression to compute file extension
Summary: ValidFileExtensions annotation should support el expression to compute file e...
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Sapphire (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Konstantin Komissarchik CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-12 13:56 EDT by Shenxue Zhou CLA
Modified: 2021-11-19 09:21 EST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Shenxue Zhou CLA 2011-07-12 13:56:45 EDT
Currently, you can supply a list of strings to the ValidFileExtensions annotation. My use case requires the file extension to be dynamic depending on some other properties in the model. As an example, I'd like to use the following expression to compute file extensions:

parent().UsePageFragment ? "jsp, jspx" : "jsff"

Ideally, we should also support FileExtensionService, similiar to RelativePathService to make it really extensible.
Comment 1 Konstantin Komissarchik CLA 2011-07-13 14:00:16 EDT
Enhancement implemented. 

In 0.3.1, there is a new @FileExtensions annotation (with support for EL) alongside the now legacy @ValidFileExtensions annotation. Both are backed by the new FileExtensionsService, which can be implemented directly.

As a companion to this feature, I found that it would be highly useful for EL to automatically convert comma-separated strings into lists. That would allow one to write the following:

@FileExtensions( expr = "png,gif,jpeg" )
@FileExtensions( expr = "${ LossyCompression ? 'jpeg' : 'png,gif' }" )

To enable this, I have added the following new automatic conversion rules:

String to List
List to String
Set to String
Array to String

All of these conversions utilize comma-separated encoding. See all conversion rules in documentation under Expression Language.

In 0.4, the @ValidFileExtensions annotation has been removed.

See documentation under services section (either version), the what's new guide for 0.3.1 and the migration guide for 0.4. 

Unit test coverage in TestExpr0007 and in TestServices0002.

Shenxue, please verify.
Comment 2 Shenxue Zhou CLA 2011-07-13 16:39:34 EDT
I've tried this new enhancement and it works mostly. There is only one thing for the file extension expression: it needs to listen to the changes of the expression properties and re-validate.
Comment 3 Shenxue Zhou CLA 2011-07-13 16:46:02 EDT
(In reply to comment #2)
> I've tried this new enhancement and it works mostly. There is only one thing
> for the file extension expression: it needs to listen to the changes of the
> expression properties and re-validate.

Another issue (probably related to comment #2): after changes are made to the properties referenced in the expression, and then bring up the "browse" dlg, the "browse" dlg still list the old file extensions.
Comment 4 Konstantin Komissarchik CLA 2011-07-14 18:28:51 EDT
I have reproduced and fixed the issue described in Comment #2. The TestServices0002.testValidation unit test now covers that case. 

I will take a look at the problem described in Comment #3 next. It should not have any relation to the validation problem that I just fixed. In fact existing TestServices0002.testFileExtensionExpr shows that file extension service itself does correctly react to expression changes. The browse dialog simply asks the service for extensions each time it is launched. The only thing that I can think of is that your expr uses Parent() function. Perhaps there is a bug with that function not propagating changes. Will try to setup a similar scenario.
Comment 5 Konstantin Komissarchik CLA 2011-07-14 19:56:12 EDT
I was not able to reproduce the problem described in Comment #3. The test cases involving Parent() function that I set up all appear to be working correctly. See TestServices0002.testFileExtensionsExpr1-4. 

Give it another try, if you are still seeing this issue, compare what you are doing to the above test case and re-open with further details.
Comment 6 Shenxue Zhou CLA 2011-07-15 13:28:16 EDT
(In reply to comment #5)
> I was not able to reproduce the problem described in Comment #3. The test cases
> involving Parent() function that I set up all appear to be working correctly.
> See TestServices0002.testFileExtensionsExpr1-4. 
> 
> Give it another try, if you are still seeing this issue, compare what you are
> doing to the above test case and re-open with further details.

Here is my file extension expression:

@FileExtensions( expr = "${ parent().UsePageFragments ? 'jsff' : 'jsf, jsp, jspx' }")

In the form editor, if I toggle "UsePageFragments" property, and try to bring up the file browse dlg, I do see the file extensions change accordingly. So I don't think there is anything wrong with my expression.

The problem only happens in the diagram properties sheet. I first need to go to the properties sheet of the diagram page to toggle the "UsePageFragments" property, then I need to select a view node and in the properties sheet for the node, I bring up the file browse dlg. The result is not consistent. For some view nodes, the file browse dlg has correct file extensions. But for other view nodes, the file browse dlg still contains old file extensions.
Comment 7 Konstantin Komissarchik CLA 2011-07-15 13:42:29 EDT
Since the scenario seems to be so specialized to what you are doing, you are going to need to debug this yourself. There is no difference for FileExtensionsService codepath between property page and the form page. The services are maintained at the model (not UI) level and its the same service instance for given element/property combination regardless of context. 

Maybe you aren't editing the properties you think you are editing (or editing them on wrong elements). Happened to me once as I was writing the unit tests. Took a bit of head scratching to figure out.

Try placing a breakpoint in DeclarativeFileExtensionsService. 

Resolving as there is no new information that points to a problem at the framework level.
Comment 8 Shenxue Zhou CLA 2011-07-15 16:13:49 EDT
(In reply to comment #7)
> Since the scenario seems to be so specialized to what you are doing, you are
> going to need to debug this yourself. There is no difference for
> FileExtensionsService codepath between property page and the form page. The
> services are maintained at the model (not UI) level and its the same service
> instance for given element/property combination regardless of context. 
> 
> Maybe you aren't editing the properties you think you are editing (or editing
> them on wrong elements). Happened to me once as I was writing the unit tests.
> Took a bit of head scratching to figure out.
> 
> Try placing a breakpoint in DeclarativeFileExtensionsService. 
> 
> Resolving as there is no new information that points to a problem at the
> framework level.

After more digging, I think I found the problem. In my form editor, my activities are displayed in a list. When a list entry is selected, it'll display its properties including the property with the FileExtensionsService expression. And RelativePathBrowseActionHandler gets initialized. Inside the init() method of RelativePathBrowseActionHandler, the file extensions computed by the FileExtensionsService are assigned. So here everything works correctly.

But in the case of diagram properties sheet, the browse button next to the property involved with the FileExtensionsService expression aren't re-generated. So the corresponding RelativePathBrowseActionHandler's init() method isn't called again when the parent property changes. So RelativePathBrowseActionHandler's file extensions are not updated.

Having file extensions set in init() method of RelativePathBrowseActionHandler doesn't seem to be a good solution...
Comment 9 Konstantin Komissarchik CLA 2011-07-15 19:21:36 EDT
Thanks for debugging this further. I fixed this problem in the validation code path, but didn't think the same caching issue would be present in browse action handlers. Fixed RelativePathBrowseActionHandler and two other places with similar problem.
Comment 10 Shenxue Zhou CLA 2011-07-15 19:44:12 EDT
(In reply to comment #9)
> Thanks for debugging this further. I fixed this problem in the validation code
> path, but didn't think the same caching issue would be present in browse action
> handlers. Fixed RelativePathBrowseActionHandler and two other places with
> similar problem.

Just tried your latest fix and it works great! Thanks!