On Mon, Nov 7, 2011 at 3:55 PM, Emmanuel Lecharny <[email protected]> wrote: > Hi guys, > > I played around some of the ideas we discussed last week on the mailing > list, about the way we handle the chain and the interceptors. > > What I did was pretty simple : I modified the way we process the > getRootDse() operation. This impact many classes : > o OperationContext : > This interface exposes 4 more methods : > List<String> getInterceptors() which returns the list of interceptors to > execute for this operation > void setInterceptors( List<String> interceptors ) which sets the list of > interceptors to execute for this operation I would suggest we remove this mutable operation and fill the interceptor list with whatever there in the DS automatically And I think(purely random thinking) that exposing this interceptor list before lead to the convenient idea of 'let us use bypass list' to get past the re-entry issue > int getCurrentInterceptor() : returns the current interceptor for this > context > void nextInterceptor() : move the current interceptor to the next > interceptor > +1 > o AbstractOperationContext : > This is where we implement the four added methods. > > ... > /** The interceptors to call for this operation */ > protected List<String> interceptors; > I would prefer not to expose this to the subclasses > /** The current interceptor position */ > protected int currentInterceptor; > ... > public AbstractOperationContext( CoreSession session ) > { > this.session = session; > currentInterceptor = 0; > } > ... > public List<String> getInterceptors() > { > return interceptors; > } > > public void setInterceptors( List<String> interceptors ) > { > this.interceptors = interceptors; > } > and removing the above setter > public int getCurrentInterceptor() > { > return currentInterceptor; > } > > public void nextInterceptor() > { > currentInterceptor++; > } > > Nothing big here, just accessors. > > > o ServerContext.doGetRootDSEOperation : > the GetRootDseOperationContext now contains the list of interceptors to call > : > > protected Entry doGetRootDSEOperation( Dn target ) throws Exception > { > GetRootDSEOperationContext getRootDseContext = new > GetRootDSEOperationContext( session, target ); > getRootDseContext.addRequestControls( convertControls( true, > requestControls ) ); > > --> // We should get this list from the DS !!! +1 > --> List<String> interceptors = new ArrayList<String>(); > --> interceptors.add( AuthenticationInterceptor.class.getSimpleName() ); > --> getRootDseContext.setInterceptors( interceptors ); > > // do not reset request controls since this is not an external > // operation and not do bother setting the response controls either > OperationManager operationManager = service.getOperationManager(); > > return operationManager.getRootDSE( getRootDseContext ); > } > > Here, it's hard wired, but the logic would be to ask the DS to provide the > interceptor list, which will be copied and stored into the OperationContext. > Otherwise, the code is the same. > > o DefaultOperationManager : > The first main difference : > public Entry getRootDSE( GetRootDSEOperationContext getRootDseContext ) > throws LdapException > { > ... > try > { > Interceptor head = directoryService.getInterceptor( > getRootDseContext.getInterceptors().get( 0 ) ); > getRootDseContext.nextInterceptor(); > > return head.getRootDSE( getRootDseContext ); > ... > > Here, we compute the first interceptor to call by getting the first of the > list from the OperationContext, and we call it. As we can see, we don't use > the InterceptorChain anymore. (Note that we can improve the code by adding a > method in the OperationContext to return the Interceptor directly.) > this is a great idea > o Interceptor : > The second main difference. We don't pass anymore the NextInterceptor I think we can still achieve the behavior of next.add() with next(opContext) and continue executing (definitely possible now, but just saying) > parameter : > Entry getRootDSE( GetRootDSEOperationContext getRootDseContext ) throws > LdapException; > > o The inherited classes (here, AuthenticatorInterceptor) : > We have to change the getRootDSE method in order to call the next > interceptor. This is done by substituing this line : > return next.getRootDSE( getRootDseContext ); > by this one : > return next( getRootDseContext ); > > where the next() method is a final method in the BaseInterceptor class > > o BaseInterceptor : > This is where we compute the next interceptor to call : > > protected final Entry next( GetRootDSEOperationContext getRootDseContext > ) throws LdapException > { > Interceptor interceptor = getNextInterceptor( getRootDseContext ); > > return interceptor.getRootDSE( getRootDseContext ); > } > > and > > private Interceptor getNextInterceptor( OperationContext operationContext > ) > { > int currentInterceptor = operationContext.getCurrentInterceptor(); > > if ( currentInterceptor == operationContext.getInterceptors().size() > ) > { > return FINAL_INTERCEPTOR; > } > > operationContext.nextInterceptor(); > > Interceptor interceptor = directoryService.getInterceptor( > operationContext.getInterceptors().get( currentInterceptor ) ); > > return interceptor; > } > > > The FINAL_INTERCEPTOR interceptor is the one which reroute calls to the > nexus (It's the same class we have in the InterceptorChain). > > > That's it, the code works, and when it comes to debug it, it's just easy : > we don't have to go through multiple classes when stepping through, and we > don't have to go through the BaseInterceptor for each disabled or bypassed > interceptor. > cool, no more break points in InterceptorChain.Element class > It's not totally finished though : the list creation is just horrible in the > given code. It must be generated in the DirectoryService by using > introspection over all the activated Interceptors. > > Thoughts ? > I liked this approach, thanks Emmanuel for experimenting and sharing > -- > Regards, > Cordialement, > Emmanuel Lécharny > www.iktek.com > >
-- Kiran Ayyagari
