TicketValidator implementations are not easily extensible

Description

I want to apply a few modifications to some concrete TicketValidator implementations
so I inherit this class. But most of the interesting methods are often marked as final.

Moreover the java beans properties do not have getter so i need to override the setter method
to store a duplicated local properties. This allow me to keep the same properties.
Example:
public class CustomXXXTicketValidator extends XXXTicketValidator {
private boolean myProperty;

public void setMyProperty(final String myProperty) {
super.setMyProperty(myProperty);
this.myProperty = myProperty;
}

// Needed for potential subclasses
public String getMyProperty() { {
return this.myProperty;
}
}

Sometimes the setter are also final and therefore I'm stuck with two solutions:

  • duplicate the code but I will lose maintainability and easy upgrade

  • use introspection to access private field but this is dirty

I understand that final keyword is necessary to ensure a restricted execution flow
like blocking a method using final and use new non final protected methods for
potential extension.

Environment

None

Activity

Show:
Sébastien Launay
December 18, 2009, 4:30 AM

Instead of telling us what we should be doing to fix code that isn't a problem, it generally helps us if we understand what you're trying to do. You say you're trying to extend the ticket validators. To do what?

Sure, in my case i was trying to add dynamic request parameters when calling serviceValidator using Cas20ServiceTicketValidator.
But the populateUrlAttributeMap() method is final therefore I can not provide additional parameters here.
The AbstractUrlBasedTicketValidator provides a customParameters properties for this purpose but the parameters are static and modifying the map at each request will not be thread safe.

Therefore I was forced to inherit from AbstractCasProtocolUrlBasedTicketValidator where populateUrlAttributeMap is still overridable and to duplicate the Cas20ServiceTicketValidator code.

ScottS
December 18, 2009, 4:36 AM

Thanks! That makes it a bit easier to help you I'll take a look at opening up that API.

ScottS
December 18, 2009, 4:37 AM

You're adding the parameters on demand? I.e. they're different per request?

Sébastien Launay
December 18, 2009, 5:31 AM

You're adding the parameters on demand? I.e. they're different per request?

This parameter is mandatory and I though at first that it can be different for each request.
Actually I can retrieve the parameter value at startup but I need to compute it in Java.

Basically, I need to provide a different logout URL to the CAS server because of a DMZ network and load balancing issue.
By adding an additional localService parameter while validating a ticket I benefits from
the secure channel to avoid exposing this internal logout URL.
This URL can be different at each request (DNS, port) but in the current environment it will not changed after startup.

I have the same kind of problems while extending some components on the CAS server
to retrieve and use this URL and I will certainly create the same request of enhancement for
this project .

I am a bit disappointed because the interface and abstract classes are very neat and open
but concrete implementation are often locked (on client and server projects).
I understand that changing some components is an error sometimes because
they are not meant to be and can bring bad design.
OTOH provide extensibility by default allow to meet more use cases without changing the code each time.

ScottS
September 12, 2011, 2:11 PM

Actually for both the client and server we provide generous hierarchies and we open up the implementations in particular ways that won't cause your logic to mess with required logic. For example, if the populateUrlAttributeMap were just easily over-rideable then you could override it in a way that doesn't allow the pgtUrl to be set.

We are constantly creating new extension points when people come to us with concrete use cases.

Regardless, it doesn't look, from your comment, that you need that feature. I'm going to go ahead with some of the other changes (i.e. the protected getters to help you out).

If you need the ability to generate DYNAMIC parameters, please open a request specifically for that (as a new feature) and we'll work with you to create a template method that you can override that will satisfy your use case. (we love template methods )

Cheers,
Scott

Fixed

Assignee

ScottS

Reporter

Sébastien Launay

Labels

None

Estimated End Date

None

Components

Fix versions

Affects versions

Priority

Minor