directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Seelmann <m...@stefan-seelmann.de>
Subject Re: Value classes refactoring
Date Thu, 24 Mar 2016 20:49:02 GMT
Thanks Emmanuel for the effort, makes all sense to me.

On 03/24/2016 12:44 AM, Emmanuel L├ęcharny wrote:
> Hi guys,
> 
> following the long mails I sent 2 weeks ago, I started my experiment in
> a branch (the 'value' branch). The first step I completed last week-end
> is about removing the AbstractValue, StringValue and BinaryValue
> classes, and making the Value interface a class that gathers all the
> features spread in the implementations.
> 
> Moving the code was not too complex, I just had issues with the tests,
> and the lack of time :-)
> 
> I have all the LDAP API tests passing now. I haven't tested yet with
> ApacheDS, but obviously it will break, because ApacheDS (and Studio too)
> is still using the old classes.
> 
> 
> There are a few things that need to be reviewed though :
> - I made the classe immutable, but there is still some work to do.
> Typically, the apply(AttributeType) method is still visible, and needs
> to be made private. It's visible because I need it for deserialization,
> but I think we can use a factory for that purpose.
> - I decided that binary values (those containing a byte[], or which AT
> is not HR) will not have a normalized value. In other words, there is no
> mean to normalize them
> - a side effect is that serialization is way simpler, and more compact :
> I'm not saving as much data as we were in the previous classes. The
> consequence is that we should have smaller entries and smaller DN. This
> is good for the server. It should also be faster
> 
> 
> What remain to be done :
> - the apply(AttributeType) has to be removed
> - we have to make the DN, RDN and AVA classes immutable
> - I have not yet talked about String Preparation : it has to be done,
> and I guess the right place for that is to add this feature to the Value
> class. I still don't know if it's a good idea to apply the PrepareString
> function to every Value (assuming it's holding a String, of course), or
> if we could spare us the CPU, as not all the values will be compared -
> typically, this is ok for indexed values, not so much for other values
> -. I'm not setup my mind yet...
> 
> As a side effect, I found a few bugs in the API :
> - an issue in the ACI handling, where some tests are computing a
> hashcode on a set of Value. The tests are working by pure luck. (see
> https://issues.apache.org/jira/browse/DIRAPI-279)
> - there is a serialize(byte[])/deserialize(byte[]) method in the
> StringValue class, which is absolutely bugged. It's not use at all,
> except by the Ava class, which is invoqued by the Rdn class, itselef
> invoqued by the Dn class. Bottom line, the Mavibot project is using this
> feature for bulkLoading data. This has to be fixed, or to be removed.
> 
> This is a work in progress, but I'm confident that would bring us to a
> better and more stable API.
> 


Mime
View raw message