tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: adding an internal filter
Date Thu, 18 Sep 2014 18:41:21 GMT
changed to:

private void addFilterConfig(final Context context, final FilterDef filterDef) {
        // hack to force filter to get a config otherwise it is
ignored in the http routing
        try {
            final Constructor<ApplicationFilterConfig> cons =
ApplicationFilterConfig.class.getDeclaredConstructor(Context.class,
FilterDef.class);
            if (!cons.isAccessible()) {
                cons.setAccessible(true);
            }
            final ApplicationFilterConfig config =
cons.newInstance(context, filterDef);
            ((Map<String, ApplicationFilterConfig>)
Reflections.get(context,
"filterConfigs")).put(filterDef.getFilterName(), config);
        } catch (final Exception e) {
            throw new IllegalStateException(e);
        }
    }

works fine even if not sexy

Thanks Konstantin for the feedback


Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau


2014-09-16 15:50 GMT+02:00 Romain Manni-Bucau <rmannibucau@gmail.com>:
> 2014-09-16 15:16 GMT+02:00 Konstantin Kolinko <knst.kolinko@gmail.com>:
>> 2014-09-15 23:01 GMT+04:00 Romain Manni-Bucau <rmannibucau@gmail.com>:
>>> Hi guys,
>>>
>>> in TomEE I'd like to add a filter but after context filter are started
>>> (means filterStart() from StandardContext is called).
>>>
>>
>> Why? What is the use case?
>>
>
> we add a filter with a tomee service (passing after tomee started)
>
>> You code adds filter definitions. A filter definitions just associates
>> filter with a name. A filter is unreachable if it has no mappings for
>> it.
>> Where are filter mappings for your filter? How do you add filter mappings?
>>
>
> full code is:
>
>         final FilterDef filterDef = new FilterDef();
>         filterDef.setAsyncSupported("true");
>         filterDef.setDescription(description);
>         filterDef.setFilterName(description);
>         filterDef.setDisplayName(description);
>         filterDef.setFilter(new CXFJAXRSFilter(cxfRsHttpListener,
> context.findWelcomeFiles()));
>         filterDef.setFilterClass(CXFJAXRSFilter.class.getName());
>         context.addFilterDef(filterDef);
>
>         String mapping = completePath;
>         if (!completePath.endsWith("/*")) {
>             if (completePath.endsWith("*")) {
>                 mapping = completePath.substring(0, completePath.length() - 1);
>             }
>             mapping = mapping + "/*";
>         }
>
>         final String urlPattern = removeWebContext(webContext, mapping);
>         cxfRsHttpListener.setUrlPattern(urlPattern.substring(0,
> urlPattern.length() - 1));
>
>         final FilterMap filterMap = new FilterMap();
>         filterMap.addURLPattern(urlPattern);
>         filterMap.setFilterName(filterDef.getFilterName());
>         context.addFilterMap(filterMap);
>
>         addFilterConfig(context, filterDef); // the hack I sent previously
>
>
>> When do you add your new filter? Is context already started and
>> serving requests, or it has not started yet?  Why cannot you add
>> filter definition before a filterStart() call?
>>
>> Do you use "StandardContext" class directly, or you can subclass it?
>>
>
> directly, tomee doesn't change StandardContext (one big goal)
>
>> The proper solutions should be to add new API, but I would like the
>> answers on the above questions first, as
>> a) There shall be some API for filter mappings as well.
>> b) If context has already been started, implementation would be more
>> complicated.
>>
>>> What I do today is:
>>>
>>
>> Where do I start with this?
>>
>> Just several notes on the code.
>>
>> 1. Missing synchronization. filterStart() uses "synchronized
>> (filterConfigs)" Your code does not use synchronization.
>>
>
> Actually we don't care that much here since that's still in the start
> context (app is not yet fully usable) so multithreading shouldnt
> occur, does it in tomcat?
>
>> 2. If you using "setAccessible(true)",
>>
>> A simpler way would be to mark ApplicationFilterConfig constructor as
>> accessible and call it directly instead of playing with filterStart().
>>
>> Playing with filterStart() without accompanying filterStop() call is broken
>>
>> filterStart() does "filterConfigs.clear();" as one of the first steps.
>> A lot of code in your method are a workaround against that clear()
>> call. (You copy the filterConfigs into a temporary map to preserve the
>> map entries from being cleared). Essentially this clear() call throws
>> away filter instances (wrapped in ApplicationFilterConfig classes)
>> without properly destroying them.
>>
>> You code might break if "filterConfigs" field were marked as"final"
>> (as it should be).
>>
>
> we already got this before in webapp classloader and until tomcat
> supports our feature we forced by reflection the field to not be
> final. That said I agree (and why I ask for a simpler API) that it is
> mainly workarounds but I don't see why StandardContext is not exposing
> it (what would be blocking).
>
>>
>>
>>>     try {
>>>             final Field def =
>>> StandardContext.class.getDeclaredField("filterDefs");
>>>             if (!def.isAccessible()) {
>>>                 def.setAccessible(true);
>>>             }
>>>             final Field config =
>>> StandardContext.class.getDeclaredField("filterConfigs");
>>>             if (!config.isAccessible()) {
>>>                 config.setAccessible(true);
>>>             }
>>>             final Object oldDefs = def.get(context);
>>>             final Map<String, ApplicationFilterConfig> oldConfig = new
>>> HashMap<>((Map<String, ApplicationFilterConfig>) config.get(context));
>>>             try {
>>>                 final Map<String, FilterDef> tempDef = new HashMap<>();
>>>                 tempDef.put(filterDef.getFilterName(), filterDef);
>>>                 def.set(context, tempDef);
>>>
>>>                 StandardContext.class.cast(context).filterStart();
>>
>> Why such code style? The following is shorter.
>>                   ((StandardContext)context).filterStart();
>>
>>
>>>                 // update configs
>>>                 oldConfig.putAll((Map<String,
>>> ApplicationFilterConfig>) config.get(context));
>>>             } finally {
>>>                 def.set(context, oldDefs);
>>>                 def.set(context, oldConfig);
>>>             }
>>>         } catch (final Exception e) {
>>>             throw new IllegalStateException(e);
>>>         }
>>>
>>> but it is really hacky just to get a ApplicationFilterConfig. Is there
>>> an easier way I miss or is it possible to make it a bit more easy?
>>>
>>>
>>> Romain Manni-Bucau
>>> Twitter: @rmannibucau
>>> Blog: http://rmannibucau.wordpress.com/
>>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>>> Github: https://github.com/rmannibucau
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message