2014-09-15 23:01 GMT+04:00 Romain Manni-Bucau <rmannibu...@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?

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?

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?

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.

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).



>     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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to