directory-api mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Radovan Semancik <radovan.seman...@evolveum.com>
Subject Re: Value classes refactoring
Date Thu, 24 Mar 2016 08:08:20 GMT
Hi,

Looks good from my point of view. However, now we are getting very close 
to midPoint feature freeze, so I will be able to really test all of this 
only in the next cycle (approx. June). But I think you are doing the 
right thing.

BTW: PrepareString. Have you considered doing it in a lazy fashion? E.g. 
prepare the string only if someone asks for it and then store the 
prepared value for the entire Value lifetime. If Value is immutable then 
this approach should be safe.

-- 
Radovan Semancik
Software Architect
evolveum.com



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