You are viewing a plain text version of this content. The canonical link for it is here.
Posted to api@directory.apache.org by Emmanuel Lecharny <el...@gmail.com> on 2010/01/25 01:30:00 UTC
About the Control interface
Hi guys,
as I'm working on the messages, I have looked at the Control Interface.
Here is the way it's used in all the different APIs :
ADS :
we used the JNDI interface, and we will change to switch to a LDAP API
Control
jLdap/LDAPSdk
=============
class :
LDAPControl
constructors :
LDAPControl()
LDAPControl(String, boolean byte[]))
methods :
getID()
getValue()
isCritical()
newInstance(byte[])
clone()
JNDI
====
interface :
Control
methods :
getEncodedValue()
getID()
isCritical()
ODS
===
abstract class :
Control
constructors :
Control( String, boolean)
methods :
getOID()
getValue()
hasValue()
isCritical()
UID
===
class :
Control
constructors :
Control(String)
Control(String, boolean)
Control(String, boolean, ASN1OctetString)
methods :
encode()
equals( Object )
getControlName()
getOID()
getValue()
hasValue()
isCritical()
Checking all those implementations, I would suggest that we stick with
something like :
interface Control
<discussion>
Not sure we need a class, when most of the controls will be named, like
VLVControl, PageSearchControl, ...
ODS define an abstract class, which means nobody can create an instance
of it. At this point, I think it's probably better to go for an
interface, hidding the asbtract class to the client.
</discussion>
constructors :
<discussion>
No need of them, if it's an interface. From the server POV, this will be
an issue, as we must be able to create Controls while decoding them and
we don't necessarily know if the control will be supported or not. We
will probably need an intermediate ControlImpl and an AbstractControl
class in order to deal with this, but from the Client POV, this is nt
relevant
</discussion>
methods :
getOid() or getOID().
getValue()
isCritical()
<discussion>
Those three methoda re the bare minimum we need from the client side.
I'm not sure that the hasValue() method is necessary, when the
getValue() returning a null value will provide the same result.
</discussion>
Thoughts ?
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
Re: About the Control interface
Posted by Matthew Swift <Ma...@Sun.COM>.
On 25/01/10 13:48, Emmanuel Lcharny wrote:
> Matthew Swift a écrit :
>>
>>
>> On 25/01/10 13:22, Matthew Swift wrote:
>>> [...]
>>>
>>> For cases where client apps want to use a control for which there is
>>> no existing sub-class implementation we provided them the option of
>>> using the "GenericControl" class instead of being forced to
>>> implement a sub-class. The GenericControl class is pretty
>>> straightforward - it implements Control and provides three
>>> constructors:
>>>
>>> GenericControl(String oid) // non-critical, null value
>>> GenericControl(String oid, boolean isCritical) // null value
>>> GenericControl(String oid, boolean isCritical, ByteString bytes)
>>>
>>> I find the GenericControl name a bit non-obvious.
>>
>>
>> Cool - we're on the same page here. I see that you have described a
>> ControlImpl sub class for exactly this purpose in your WIKI page...
>>
>> I'm not sure I'm a big fan of the ControlImpl name to be honest,
>> since XXXImpl naming is usually associated with internal
>> implementation classes which are not usually exposed to client apps
>> (e.g. PlainSocketImpl in J2SE).
> We have had many discussions about how best name a class which is not
> abstract. At the end, we decided to follow this rule :
> - Interfaces' name is the one we want to identify as the object (in
> this case, Control)
> - Abstract class are named AbstractXXX. I think it's acceptable
> - Implementations' name proposal were : BaseXXX, XXXImpl, or using a
> IXXX for the interface and XXX for the class. Per rule #1, we rejected
> the third proposal, and we decided that XXXImpl was probably better.
>
Fair enough - consistency is the main priority and it sounds like you
already have a consistent naming scheme defined.
:-)
Matt
Re: About the Control interface
Posted by Emmanuel Lcharny <el...@gmail.com>.
Matthew Swift a écrit :
>
>
> On 25/01/10 13:22, Matthew Swift wrote:
>> [...]
>>
>> For cases where client apps want to use a control for which there is
>> no existing sub-class implementation we provided them the option of
>> using the "GenericControl" class instead of being forced to implement
>> a sub-class. The GenericControl class is pretty straightforward - it
>> implements Control and provides three constructors:
>>
>> GenericControl(String oid) // non-critical, null value
>> GenericControl(String oid, boolean isCritical) // null value
>> GenericControl(String oid, boolean isCritical, ByteString bytes)
>>
>> I find the GenericControl name a bit non-obvious.
>
>
> Cool - we're on the same page here. I see that you have described a
> ControlImpl sub class for exactly this purpose in your WIKI page...
>
> I'm not sure I'm a big fan of the ControlImpl name to be honest, since
> XXXImpl naming is usually associated with internal implementation
> classes which are not usually exposed to client apps (e.g.
> PlainSocketImpl in J2SE).
We have had many discussions about how best name a class which is not
abstract. At the end, we decided to follow this rule :
- Interfaces' name is the one we want to identify as the object (in this
case, Control)
- Abstract class are named AbstractXXX. I think it's acceptable
- Implementations' name proposal were : BaseXXX, XXXImpl, or using a
IXXX for the interface and XXX for the class. Per rule #1, we rejected
the third proposal, and we decided that XXXImpl was probably better.
Now, it's purely a convention between us, and we are not found of it. An
alternative is clearly to use a factory, with an helper class
(ControlFactory.newInstance(...))
At this point, I think it's probably a better solution.
>
> Matt
>
>
>
Re: About the Control interface
Posted by Matthew Swift <Ma...@Sun.COM>.
On 25/01/10 13:22, Matthew Swift wrote:
> [...]
>
> For cases where client apps want to use a control for which there is
> no existing sub-class implementation we provided them the option of
> using the "GenericControl" class instead of being forced to implement
> a sub-class. The GenericControl class is pretty straightforward - it
> implements Control and provides three constructors:
>
> GenericControl(String oid) // non-critical, null value
> GenericControl(String oid, boolean isCritical) // null value
> GenericControl(String oid, boolean isCritical, ByteString bytes)
>
> I find the GenericControl name a bit non-obvious.
Cool - we're on the same page here. I see that you have described a
ControlImpl sub class for exactly this purpose in your WIKI page...
I'm not sure I'm a big fan of the ControlImpl name to be honest, since
XXXImpl naming is usually associated with internal implementation
classes which are not usually exposed to client apps (e.g.
PlainSocketImpl in J2SE).
Matt
Re: About the Control interface
Posted by Emmanuel Lcharny <el...@gmail.com>.
Matthew Swift a écrit :
> We're still alive and kicking! ...for now :-)
Yeah, you remain "An open source project"... Not sure what that does mean :?
Re: About the Control interface
Posted by Matthew Swift <Ma...@Sun.COM>.
We're still alive and kicking! ...for now :-)
Matt
On 29/01/10 16:02, Emmanuel Lcharny wrote:
> Some updates :
>
> - I have added the hasValue() method. It's very convenient
> - I have also removed the setOid() method : a Control *must* have an
> OID, and it's already providd in the constructor.
>
> <grin>
> Are the SUN people subscribed to this mailing list still subscribed
> under a sun.com domain ?
> </grin>
Re: About the Control interface
Posted by Emmanuel Lcharny <el...@gmail.com>.
Some updates :
- I have added the hasValue() method. It's very convenient
- I have also removed the setOid() method : a Control *must* have an
OID, and it's already providd in the constructor.
<grin>
Are the SUN people subscribed to this mailing list still subscribed
under a sun.com domain ?
</grin>
Re: About the Control interface
Posted by Matthew Swift <Ma...@Sun.COM>.
On 25/01/10 13:56, Emmanuel Lecharny wrote:
> Matthew Swift a écrit :
>> <snip/>
>> GenericControl(String oid) // non-critical, null value
>> GenericControl(String oid, boolean isCritical) // null value
> Makes sense to add this constructor, but for Control with *no* value
> (semantically more accurate, compared to *null* value).
>> GenericControl(String oid, boolean isCritical, ByteString bytes)
> byte[] fits well, I think.
>> <snip/>
>
>> The Control API must distinguish between null values (i.e. value not
>> present) and empty values (i.e. value present but zero-length).
> Absolutely. I mis-judged this need, so we should add this method :
>
> boolean hasValue();
This is not strictly necessary. If getValue() returns null then you know
there was no value. If getValue().length == 0 then you know that there
was a value but it was empty. Having said that, hasValue is definitely
more readable.
Matt
Re: About the Control interface
Posted by Emmanuel Lecharny <el...@gmail.com>.
Matthew Swift a écrit :
> <snip/>
> GenericControl(String oid) // non-critical, null value
> GenericControl(String oid, boolean isCritical) // null value
Makes sense to add this constructor, but for Control with *no* value
(semantically more accurate, compared to *null* value).
> GenericControl(String oid, boolean isCritical, ByteString bytes)
byte[] fits well, I think.
> <snip/>
> The Control API must distinguish between null values (i.e. value not
> present) and empty values (i.e. value present but zero-length).
Absolutely. I mis-judged this need, so we should add this method :
boolean hasValue();
>
> I don't know if it is out of scope for now, but do we want to support
> extensibility? In particular, how can client applications implement
> their own controls? There are two main issues here that I have
> encountered:
>
> 1. Decoding of response controls: if I have a response control whose
> type is "MyControl" do I want the LDAP SDK to automatically decode
> it to this type? Or will the client application have to do it.
> Here's some pseudo code to illustrate my point:
>
> // Result contains a MyControl response control.
> Result result = connection.add(entry);
>
> // Option #1: Uses an internal map of Control implementation
> classes -> OID + decoder
> MyControl control = result.getControl(MyControl.class);
>
>
>
>
> // Option #2: Uses an internal map of OID -> decoder
> MyControl control = (MyControl)
> result.getControl(MyControl.OID);
>
> // Option #3: No internal map - client has to manually decode
> Control control = result.getControl(MyControl.OID);
> MyControl myControl = new MyControl(control);
>
> I prefer the first approach for simplicity but it requires a
> public API for registering Control implementations (as does option
> #2) or use introspection and require that all implementations
> provide an OID field and a constructor having a single Control
> argument. Option #3 is quite verbose for clients IMO.
>
> I think that it's safer if the request/response API decodes the
> Control each time rather than caching the decoded control. This
> will make it possible to support immutable request/responses.
>
> If it sounds like I getting ahead here, the point of this issue is
> that if we want to provide an simple decoding mechanism like #1
> then we will need to have some way for the SDK to be able to
> decode the Control. This means either having a registration
> process, or using introspection and having a well defined
> constructor and OID field.
>
> The same problem will present itself for the following API features:
>
> * decoding extended responses
>
> * decoding intermediate responses
>
> * decoding request controls (server-side so out of scope)
>
> * decoding extended requests (server-side so out of scope)
>
> 2. Encoding/decoding controls: many control values are encoded using
> ASN1. Do we want to provided ASN1 support? This will also apply
> for new extended operations.
>
> I think that these questions are only applicable if we decide to
> support extensibility.
Well, IMO, from the client side, these issues can occur only if the
provided API does not support some of the server's Controls. In this
case, the user will have to create his own Control, implementing the
Control interface, and do the decoding himself. What the API will
generate is a instance of Control where the ID, criticality and value
are stored as opaque elements - especially for the value -. Up to the
user to translate this instance to its own control.
I'm not sure that providing anything more complex ATM is usefull. Who
develops Controls, anyway ?
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
Re: About the Control interface
Posted by Matthew Swift <Ma...@Sun.COM>.
Hi Emmanuel,
I totally agree with your thoughts regarding the Control API. As per
usual, I prefer getOID to getOid... :-)
Our OpenDS SDK Control package is not yet finished and needs some
serious clean up. In fact, I was planning to turn our Control abstract
class into an interface.
For cases where client apps want to use a control for which there is no
existing sub-class implementation we provided them the option of using
the "GenericControl" class instead of being forced to implement a
sub-class. The GenericControl class is pretty straightforward - it
implements Control and provides three constructors:
GenericControl(String oid) // non-critical, null value
GenericControl(String oid, boolean isCritical) // null value
GenericControl(String oid, boolean isCritical, ByteString bytes)
I find the GenericControl name a bit non-obvious. An alternative
approach is to hide the class (package private) and expose the
constructors using a separate "Controls" utility class. E.g:
public static final class Controls {
public static final Control newControl(String oid) { return new
GenericControl(oid); }
}
I think that we'll probably do this in the OpenDS SDK since we'll also
need at least one other utility method so that we can provide immutable
request/response wrappers:
public static final Control unmodifiableControl(Control control) { ... }
Note that the Control interface is immutable, but that does not stop
implementations from being mutable.
The Control API must distinguish between null values (i.e. value not
present) and empty values (i.e. value present but zero-length).
I don't know if it is out of scope for now, but do we want to support
extensibility? In particular, how can client applications implement
their own controls? There are two main issues here that I have encountered:
1. Decoding of response controls: if I have a response control whose
type is "MyControl" do I want the LDAP SDK to automatically decode
it to this type? Or will the client application have to do it.
Here's some pseudo code to illustrate my point:
// Result contains a MyControl response control.
Result result = connection.add(entry);
// Option #1: Uses an internal map of Control implementation
classes -> OID + decoder
MyControl control = result.getControl(MyControl.class);
// Option #2: Uses an internal map of OID -> decoder
MyControl control = (MyControl) result.getControl(MyControl.OID);
// Option #3: No internal map - client has to manually decode
Control control = result.getControl(MyControl.OID);
MyControl myControl = new MyControl(control);
I prefer the first approach for simplicity but it requires a
public API for registering Control implementations (as does option
#2) or use introspection and require that all implementations
provide an OID field and a constructor having a single Control
argument. Option #3 is quite verbose for clients IMO.
I think that it's safer if the request/response API decodes the
Control each time rather than caching the decoded control. This
will make it possible to support immutable request/responses.
If it sounds like I getting ahead here, the point of this issue is
that if we want to provide an simple decoding mechanism like #1
then we will need to have some way for the SDK to be able to
decode the Control. This means either having a registration
process, or using introspection and having a well defined
constructor and OID field.
The same problem will present itself for the following API features:
* decoding extended responses
* decoding intermediate responses
* decoding request controls (server-side so out of scope)
* decoding extended requests (server-side so out of scope)
2. Encoding/decoding controls: many control values are encoded using
ASN1. Do we want to provided ASN1 support? This will also apply
for new extended operations.
I think that these questions are only applicable if we decide to support
extensibility.
Matt
On 25/01/10 01:30, Emmanuel Lecharny wrote:
> Hi guys,
>
> as I'm working on the messages, I have looked at the Control
> Interface. Here is the way it's used in all the different APIs :
>
> ADS :
> we used the JNDI interface, and we will change to switch to a LDAP API
> Control
>
> jLdap/LDAPSdk
> =============
> class :
> LDAPControl
>
> constructors :
> LDAPControl()
> LDAPControl(String, boolean byte[]))
>
> methods :
> getID()
> getValue()
> isCritical()
> newInstance(byte[])
> clone()
>
>
> JNDI
> ====
> interface :
> Control
>
> methods :
> getEncodedValue()
> getID()
> isCritical()
>
>
> ODS
> ===
> abstract class :
> Control
>
> constructors :
> Control( String, boolean)
>
> methods :
> getOID()
> getValue()
> hasValue()
> isCritical()
>
>
> UID
> ===
> class :
> Control
>
> constructors :
> Control(String)
> Control(String, boolean)
> Control(String, boolean, ASN1OctetString)
>
> methods :
> encode()
> equals( Object )
> getControlName()
> getOID()
> getValue()
> hasValue()
> isCritical()
>
>
> Checking all those implementations, I would suggest that we stick with
> something like :
>
> interface Control
>
> <discussion>
> Not sure we need a class, when most of the controls will be named,
> like VLVControl, PageSearchControl, ...
> ODS define an abstract class, which means nobody can create an
> instance of it. At this point, I think it's probably better to go for
> an interface, hidding the asbtract class to the client.
> </discussion>
>
> constructors :
> <discussion>
> No need of them, if it's an interface. From the server POV, this will
> be an issue, as we must be able to create Controls while decoding them
> and we don't necessarily know if the control will be supported or not.
> We will probably need an intermediate ControlImpl and an
> AbstractControl class in order to deal with this, but from the Client
> POV, this is nt relevant
> </discussion>
>
> methods :
> getOid() or getOID().
> getValue()
> isCritical()
>
> <discussion>
> Those three methoda re the bare minimum we need from the client side.
> I'm not sure that the hasValue() method is necessary, when the
> getValue() returning a null value will provide the same result.
> </discussion>
>
> Thoughts ?
>
>