Community
Participate
Working Groups
1. Create a Web 3.0 project with no DD 2. Open the New Filter wizard (right click on the project -> New -> Filter ) 3. Type in the package and class name, click next 4. In the filter mappings section, click Add to add a new mapping 5. Select at least one check box for dispatchers, click OK 6. Note that the dispatchers are included in the filter mappings table. Click Finish Problem: The created class does not include the dispatcherTypes element in the WebFilter annotation
Assigning to Dimitar for initial investigation. If this can be quickly fixed, we would love to see it fixed in WTP 3.2.1 The problem here comes from org.eclipse.jst.j2ee.web/templates/filter_classHeader.template There is a section wrapped by: if (model.isAnnotated()) { That section contains the proper code for handling the older style dispatchers. However, in the section wrapped by: if ("3.0".equals(model.getJavaEEVersion())) { There is no equivalent section for the Web 3.0 dispatcherTypes
Created attachment 174039 [details] fix for dispatcher annotation parameter
Here is the fix. I think we can push it in 3.2.1 Additionally Carl can you review the fix and the behavior, I had a talk with our run-time colleagues and we came up with this implementation. In the specification the information is really scarce. Basically we took the aggregation approach, where all dispatcher types are aggregated in the annotation without duplication. @Chuck can you review the fix.
Patch looks good
Carl can you review the fix, so I can ask for PMC review.
Dimitar, the fix is incomplete (see the end of this comment for the main reason). I apologize for the delay, but I am still working to verify what the proper fix should be. So far, here is what I am certain of: As of the Servlet 2.5 specification, a filter mapping can have one set of dispatcher types, and then it can have one or more URL patterns and/or Servlet mappings. Our current UI limits a filter mapping to the Servlet 2.4 limitations- each filter mapping has one set of dispatcher types, and only one of either a URL pattern or a Servlet mapping. I will open a separate enhancement to address that limitation, but we can live with the current UI for now. Our current UI allows for multiple filter mappings on a filter. I am investigating how that should be put into an @WebFilter, but do not have a good answer yet. Your current patch adds in dispatcherType support for URL patterns. It does not add in dispatcherType support for Servlet mappings. And the way it is coded in CreateFilterTemplateModel.getClassAnnotationParams(), it worries about the type of the mapping first. The dispatcherType should be handled first, and then the type (URL pattern/Servlet name) should then be considered.
I opened bug 391745 to cover the missing spec coverage. I am still investigating the @WebFilter annotations specifics.
Created attachment 174261 [details] processing of servletmapping added Here is the new patch. I will spare you the long comment that I thought that I should check this and completely forgot about it:) This looks like a good short term solution for now. Is it already too late for 3.2.1?
It still doesn't work for the servlet name only case- the following code block stops it: if (urlMappings.size() > 0) { result.put(ATT_URL_PATTERNS, urlMappings); if (dispatcherTypes.size() > 0) { result.put(ATT_DISPATCHER_TYPES, dispatcherTypes); } } Today is the last day for non-critical changes to WTP 3.2.1.
Created attachment 174389 [details] version 3 @Chuck, Carl - Can you review the latest patch.
I approve of this patch.
approved
Dear PMC members: Description: Because of this bug the dispatcherTypes stated in the Filter wizard are completely ignored and not added to the Filter annotations. Workaround: To workaround this issue is adding the dispatcherTypes manually in the annotation Testing: The fix has been tested manually Reviewers: The fix was reviewed by: Chuck and Carl What is the risk associated with this fix? - Low.
Pretty big code change (in terms of number of lines) but they seem relatively "routine", and does sound like "missing function" for this area, so I'm fine if this can get into this week's build.
Committed and released. Thanks to all.
