Le 12/10/15 21:03, Stefan Seelmann a écrit : > On 10/12/2015 08:26 PM, Emmanuel Lécharny wrote: >> Hi guys, >> >> last week and most of this week-end, I was working on fixingthe way we >> propagate the SchemaManager in a DN (see conversation on >> [email protected] ML). The thing is that when we parse a DN and >> appky the schemaManager on it, this schemaManager was never propagated >> down to teh AVA and teh Value. >> >> As a reminder : >> >> DN -> RDN+ >> RDN -> AVA+ >> AVA -> AttributeType ':' Value >> >> Where Value can be either a StringValue or a BinaryValue. >> >> In all instances, we keep an UP vakue and a Norm value. The NormValue >> will be created applying the Normalizer fetched from the schema, >> accordingly to the AttributeType's Syntax. >> >> The API was not doing so, except at the RDN level (is, teh AVA and Value >> never get saw the SchemaManager). >> >> I have successfully changed the API to propagate the SchemaManager dow >> to the Ava and Value, which was quite a pain. In the process, I have >> found that *many* tests were simply plain wrong... Having fixed them was >> not the end of my pain :/ >> >> I just created an issue : >> >> https://issues.apache.org/jira/browse/DIRSERVER-1974 >> >> This is about a test in the server which is plain wrong : we try to >> rename an entry, providing a second RDN, expecting the new value being >> present in the entry within the old value (the deleteOldRdn flag is not >> set). And the test *passes*; never mind if the Attribute is a >> Single-Value attribute ! >> >> I'm trying to fix all those issues (and we have many), but that means we >> still have a lot of basic errors all around... >> >> More to come ! >> > Thanks Emmanuel to bear the pain and fixing the mess. I remember that > Dn/Rdn/Ava classes are very large and complex, they are mutable and have > multiple states (normalized=true/false, schemamanager=null/notnull). I > think I tried to simplify that multiple times, but always failed.
FTR, when I started to work on the server, it was all String, binary values weren't even considered. My first change in this area was to use either String or byte[] depending on the H-R flag. It was obviously not enough... At some point later, we had to implement the PrepareString RFC (4518), which was done at the end of 2006 for ApacheDS 1.0.1 : http://svn.apache.org/viewvc/directory/sandbox/elecharny/trunks/shared/ldap/src/main/java/org/apache/directory/shared/ldap/schema/PrepareString.java?view=auto&rev=490270 In an attempt to improve the handling of values, I introduced the StringValue/BinaryValue at the end of 2007 (http://svn.apache.org/viewvc?rev=581113&view=rev) It was a fight to get it done (I think I spent 3 full months on that task), and It took me some extra time to add the schema handling in those classes later on. Anyway, I still consider that this approach is not the best, and it was just an attempt to ease the pain we felt back then... Actually, I have filled a JIRA stating that we should use plain byte[] years ago : https://issues.apache.org/jira/browse/DIRSERVER-1893?jql=project%20%3D%20DIRSERVER%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20key%20ASC%2C%20priority%20DESC This is a pending task, and considering the amount of work involved, and teh other issues we had (and still have!), it was put aside. Yes, it's complicated, and yes, it will take time. FTR, here are a few tests that were (wrongly) successful before my changes : 1) Dn dn1 = new Dn( "ou=A\\ ,ou=system" ); dn1.apply( schemaManager ); assertEquals( "ou=A\\ ,ou=system", dn1.getName() ); assertEquals( "2.5.4.11=a,2.5.4.11=system", dn1.getNormName() ); There is a missing ' ' in the normalized name 2) Dn dn = new Dn( " ou = Ex\\+mple " ); dn.apply( schemaManager ); assertEquals( "Ex+mple", rdn.getValue().getString() ); The '+' should be escaped in the UpValue3) 3) Rdn rdn = new Rdn( schemaManager, " CN =" ); This is simply impossible : CN should always have one char. 4) Dn childDn1 = new Dn( schemaManager, "dc=child1,ou=test,ou=system" ); Rdn newRdn = new Rdn( SchemaConstants.DC_AT + "=" + "renamedChild1" ); RenameOperationContext renameOpCtx = new RenameOperationContext( mockSession, childDn1, newRdn, false ); partition.rename( renameOpCtx ); partition = reloadPartition(); childDn1 = new Dn( schemaManager, "dc=renamedChild1,ou=test,ou=system" ); Entry entry = partition.lookup( new LookupOperationContext( mockSession, childDn1 ) ); assertNotNull( entry ); assertTrue( entry.get( "dc" ).contains( "child1" ) ); This one was actually the worst test... DC is a single value attribute, but no matter what, after the rename, it was holding 2 values ;-) Bottom line, the changes I'm doing are necessary, but the more I fix this code, the more I think we need another approach.
