On 2/1/11 4:56 PM, Alex Karasulu wrote:
On Tue, Feb 1, 2011 at 5:37 PM, Emmanuel Lecharny<[email protected]>wrote:
Hi guys,
So we get a building trunk as of today.
We can now move to the next step. There is some cleanup to do in the code
we injected last week, as we were hurrying to get something that works. Here
is a list of tasks to complete :
- Review the CodecService usage
+1
- Remove the CodecService from the DSML grammars
+1 and some other areas:
Where we create new DefaultLdapCodecService instances?
shared-ldap-client-api => LdapNetworkConnection<initialization>
- needed
apacheds-core => DefaultDirectoryService.initialize()
- needed
apacheds-core-api => LdapCoreSessionConnection()<initialization>
- totally unnecessary: the DirectoryService can be used to get a handle
shared-dsmlv2-parser => Dsmlv2Grammar<initialization>
- unnecessary
shared-dsmlv2-parser => Dsmlv2ResponseGrammar<initialization>
shared-ldap => LdapEncoder<initialization>
- review
The encoder should not create a CodecService. It's never called out of
the blue, so the caller can pass it to the encoder.
studio-connection.core => CursorStudioNamingEnumeration<initialization>
- unnecessary
studio-ldapbrowser.core => ExportDsmlRunnable<initialization>
- unnecessary
studio-ldapbrowser.core => ImportDsmlRunnable<initialization>
- unnecessary
Can review again after dust settles to get this organized correctly.
- Rename the *I*xxx interfaces
I don't like this Ixxx interface naming but it did help in many places to my
surprise. It for example narrows searching for an interface to implement a
great deal because of the I in front. I think it should be considered - at
least we need definitive consensus on whether to go with it or not.
I can go either way - I just don't think this should be turned into a "oh no
not again this converstation." Our situation in releasing an API warrants it
one last time at most.
In this case, none of those *I*xxx interfaces are visible from the API.
As we have some inconsistent code atm, I'd suggest we stick to what we
decided years ago and get back to the standard notation (ie Xxx for
interfaces and XxxImpl for implementations).
- Review the contol encoding (we need to avoid a double call to the
computeLength() method)
Yep and we need to make sure we're not double wrapping with decorators.
I had trouble with this piece of code.
Right now, to get it working, I moved the control envelop encoding out
of the ControlDecorator class. The reason was that the encode() methods
was used to encode the Control's value.
The problem with this approach is that we can't anymore stored the
control's length when we have computed it in the control, so when we
encode it, we have to compute it again.
I'd like to add a computeEnvelopLength() and encodeEnvelop() methods in
the ControlDecorator, and move back the code from the LdapEncoder class
down to this class.
wdyt ?
- Add the missing Javadocs
+1
- Relome the duplicated fields
+1
I think it's already done
Once done, we will have some refactoring to do :
- move the controls in one single package (right now, they are spread in
any places)
+1
I can push a separate thread on this matter since it might distract from the
high level discussion we have here.
yes, a separate thread could be good, but I don't think there is much to
discuss here. It's probably more about giving some information.
- check that we have a clear separation between teh API and the SPI (the
extended operation might be a problem here)
+1
What's been done for controls need to be done for extended operations.
Yep.
Last, not least, the PasswordPolicy tests have been ignored, we shoumd move
them to server-integ and make them work.
+1
We need to get all server tests out of core and core-api. There are many of
them.
hmmm, not sure.
We need a list here.
Let's start !
PS : And remember : no trunk breakage, no big refactoring without
previous discussion on the ML, branching could be the way to go.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
