Re: [ApacheDS] Merge jdbm-store into jdbm-partition
On Fri, Jun 4, 2010 at 1:16 AM, Emmanuel Lecharny elecha...@gmail.comwrote: On 6/4/10 12:01 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/3/10 10:51 PM, Stefan Seelmann wrote: Hi dev, another easy refactoring is to merge the modules jdbm-store and jdbm-partiton. +1. Which one will you keep ? jdbm-store ? To me, a partition is associated with a naming context, not with an underlying store. That implies we should get rid of those XXX-partition to just keep xxx-store, and keep the partitions at a upper layer (ie, core) I wanted to keep the jdbm-partition module. To me the partition is the concept that the core knows. The core knows nothing about stores. We also define partitions in the configuration, not stores. This is how I understand the architecture: 1. The core defines the Partition interface +1 Yes. 2. XDBM provides an abstract implemementation of the Partition interface and additionally defines the Store interface and search engine. Yes. Also eventually with global identifiers (UUID) acting as primary keys for the XDBM db scheme we will be able to pull the search functionality out of the partition and place it above the partition nexus. This will make the store interface/concept less important as well. 3. The JDBM partition is a concrete implementation of the XDBM partition. It contains a Store implementation because this is forced by XDBM. Here, I disagree. JDBM is a store, not a partition. XDBM = XXX-Data Base Manager, nothing connected to the idea of Partition. We could probably say that XDBM and Store is the same concept. I think we're overloading too much meaning into Store here. Treat it as a simple interface and forget about trying to draw more meaning out of it such as Database Manager etc. Search happens on this Store which exposes all the methods needed to perform various operations. It's just an interface coalescing all these functions together into a single place so for example we can hand off a store to different parts of the XDBM implementation and have it operate on that object. But let's discuss this aspect further, I may perfectly be wrong, I'm just trying to manipulate concepts here. I think we're trying to draw more meaning from this concept which we do not need to. -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
On Fri, Jun 4, 2010 at 12:27 AM, Stefan Seelmann seelm...@apache.orgwrote: Felix Knecht wrote: On 06/03/10 23:04, Stefan Seelmann wrote: Hi dev, the core-mock module includes some mock implementations of ApacheDS core-api classes (CoreSession, DirectoryService, etc.). It is only used as test dependency in ldif-partition. To get rid of the core-mock module I'd like to suggest to move those classes to src/test/java in core-api. To be able to use the test classes outside of the core-api module we just need to deploy the test jar of core-api. The using module then needs to specify a dependency with classifier tests, see [1] for details. Thoughts? +1 Regards Felix Should we declare the deployment of test jar for the core-api module only or should we deploy the test jars of all modules? In the latter case we can declare the maven-jar-module in the project pom, similar to the maven-source-plugin. I like the idea of deploying some module tests because they can actually be made to operate on different kinds of LDAP API providers or Partition implementations as modular tests. Unfortunately JUnit is not the best for running these tests on multiple implementations. Regardless overall this is something that will be worth while for us once we tame our testing approach. So yes I like this idea of enabling deployment of test jars. -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: Exception Logging
On Fri, Jun 4, 2010 at 12:41 AM, Emmanuel Lecharny elecha...@gmail.comwrote:
On 6/3/10 11:15 PM, Felix Knecht wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
We have a lot of following constructs:
log.error( I18n.err( I18n.ERR_04007 ) );
throw new DecoderException( I18n.err( I18n.ERR_04007 ) );
What about logging the exception within the exception itself like
public DecoderException(String message)
{
super( message );
log.error( message );
}
This will avoid having log.error all over the place and the translation
must be done only once instead of twice like above.
I would not favor such code pattern. The reason is that we may not want to
log in all cases, but only from time to time. Also the log can contain a
different message.
To avoid a double translation, I would rather suggest something like :
String message =
String message = I18n.err( I18n.ERR_04007 );
log.error( message );
throw new DecoderException( message );
+1
Also although perhaps unrelated ... can't we get eclipse to show the error
message when doing a mouse over or something on the I18n error code?
--
Alex Karasulu
My Blog :: http://www.jroller.com/akarasulu/
Apache Directory Server :: http://directory.apache.org
Apache MINA :: http://mina.apache.org
To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: [ApacheDS] Merge module avl-partition into xdbm-search and rename xdbm-search to xdbm-partition
+1 On Fri, Jun 4, 2010 at 12:32 AM, Emmanuel Lecharny elecha...@gmail.comwrote: On 6/3/10 10:47 PM, Stefan Seelmann wrote: Hi dev, I'd like to move the only remaining class AvlPartition from module avl-partition into module xdbm-search. Additional I'd like to rename xdbm-search to xdbm-partition. The xdbm-partition module than contains everything that is related to XDBM partitions: - the Store interfaces and a default AVL implementation - the XDBM search engine interfaces and the default implementation - the XDBM partition interfaces and a default AVL implementation Any objections? Go for it. We already have a xdbm.impl.avl package in xdbm-search, no reason to have a separate module for AVL. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: CachingNormalizer - where does it caches?
On Thu, Jun 3, 2010 at 8:36 PM, Emmanuel Lecharny elecha...@gmail.comwrote: On 6/3/10 7:08 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 The constructor accepts a cachesize which is never used. Apart of this I really can't see, where the caching happens. Any ideas? Ok, in real world, this cache is just useless. I mean, it's a good idea to think about having a cache, but it has to be implemented correctly, which is not the case here. The discussions we had a while back about the caches (and there is a JIRA about it) demonstrated that we need a global cache, based on some global system like JCS (apache commons-jcs). It should be linked to the DirectryService, and mus not be a singleton. Right now, the cache we have are associated with instances of the interceptors, so they are not shared. They are just valid during an user session, AFAICT. Not efficient... (I would ask you to take this with cautious, I may be wrong). It really would be nice if we can get this cache mess sorted out and have a solid entry cache in place before releasing 2.0. It's a critical part of the server and should not be difficult to do with ehcache or something. If we can get this JDBM serialization hook working to check the cache first that's another plus. Thoughts? -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: Interceptor Partition methods will now return a LdapException
On Thu, Jun 3, 2010 at 4:49 PM, Emmanuel Lecharny elecha...@gmail.comwrote: Hi guys, just to inform you that in order to get rid of NamingException from the core server, I'm replacing it by LdapException, and that will lead to some modification in the core server : every method were throwing a Exception, and will now throw a LdapException. Not a big change from the server POV, but that impact a lot of code. +1 One question: Is LdapException shared with the ldap-api as well? Wondering this because it would be nice for us in several respect if they are both the same exception class hierarchy root. -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Useless if else or missing something?
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
The if {} does the same like the else {} - no matter if it's
'overlapping' or not. If it's not a bug (doing the same) we can skip the
if else.
http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/ldif/LdifRevertor.html#502
Felix
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkwIrkwACgkQ2lZVCB08qHGfWwCg6TbJxtAyDFRVCF0jvCUVpJvn
3mEAoIPaiasluVLqAxDTFaPP5cCLDDGE
=rAdJ
-END PGP SIGNATURE-
Re: Interceptor Partition methods will now return a LdapException
On 6/4/10 9:08 AM, Alex Karasulu wrote: On Thu, Jun 3, 2010 at 4:49 PM, Emmanuel Lecharnyelecha...@gmail.comwrote: Hi guys, just to inform you that in order to get rid of NamingException from the core server, I'm replacing it by LdapException, and that will lead to some modification in the core server : every method were throwing a Exception, and will now throw a LdapException. Not a big change from the server POV, but that impact a lot of code. +1 One question: Is LdapException shared with the ldap-api as well? Yes Wondering this because it would be nice for us in several respect if they are both the same exception class hierarchy root. They are. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Useless if else or missing something?
On 6/4/10 9:42 AM, Felix Knecht wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
The if {} does the same like the else {} - no matter if it's
'overlapping' or not. If it's not a bug (doing the same) we can skip the
if else.
This is very strange...
I have to review this part of the code to see why I have added this
if... else... part.
Give me a couple of hour so that I move to office and get the cafeine
spreaded into my brain...
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
Re: svn commit: r951314 - /directory/shared/trunk/ldap/src/main/java/org/apache/directory/shared/ldap/util/StringTools.java
On 6/4/10 9:50 AM, fel...@apache.org wrote: Author: felixk Date: Fri Jun 4 07:50:56 2010 New Revision: 951314 URL: http://svn.apache.org/viewvc?rev=951314view=rev Log: Don't catch RuntimeExceptions accidentally Good 'catch' :-) btw, there are many places where we throw NullPointerExceptions (mainly when checking the methods arguments, which is wrong : we should throw a IllegalArgumentException instead). -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
Emmanuel Lecharny wrote: On 6/4/10 1:03 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/3/10 11:04 PM, Stefan Seelmann wrote: Hi dev, the core-mock module includes some mock implementations of ApacheDS core-api classes (CoreSession, DirectoryService, etc.). It is only used as test dependency in ldif-partition. To get rid of the core-mock module I'd like to suggest to move those classes to src/test/java in core-api. To be able to use the test classes outside of the core-api module we just need to deploy the test jar of core-api. The using module then needs to specify a dependency with classifier tests, see [1] for details. We also had a discussion with Pierre-Arnaud about those unit tests. Once upon a time, we have had some core-unti and server-unit modules. They have been renamed core-intger and server-integ. So far, so good, but we still have another module called apacheds-test-framework. We do think that the classes we have in core/server-integ - I mean, the classes in java/main, not the tests - should be moved to apacheds-test-framework. That being said, we could also move the core-mock into this apacheds-test-framework module. Does it make sense ? I'm afraid that won't be possible because this would cause cyclic dependencies. The integ tests and the test framework start up a real directory service with real partitions, including an LDIF based config and schema partition, so they depend on the ldif-partition. The mocks are used because we can't start a real directory service in ldif-partion module for unit tests. But we can run the integration tests against the LDIF partition. Ok, so we have the exact same problem than with the LDAP API : we had to extract the api tests and put them in ApacheDS just to be able to launch a real server. What about doing the same thing and move all the Ldif-Partition tests out of the ldif-partition module and put them where they can use the real DS ? I mean, tests are not to be close to the part they are testing? thoughts ? I think we should distinguish unit tests and integration tests. Unit tests should be close to the part they are testing. And they should not test other components around. Thus it is valid to use mocks. The tests in ldif-partition are unit tests because they just test that the ldif-partition code works, that is that files in the file system are created or deleted. So I think these tests are at the right place. Kind Regards, Stefan
[jira] Created: (DIRSERVER-1515) Remove the 'throw new NullPointerException' in the code
Remove the 'throw new NullPointerException' in the code --- Key: DIRSERVER-1515 URL: https://issues.apache.org/jira/browse/DIRSERVER-1515 Project: Directory ApacheDS Issue Type: Bug Affects Versions: 1.5.7 Reporter: Emmanuel Lecharny Fix For: 2.0.0-RC1 There are 43 places where we throw a NullPointerException explicitely. This is very worng. If we do that when a method argument is null, we should instead throw a IllegalArgumentException. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
How strict must we check in util methods
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Do we leave it up to third party developers to take care that only
correct values are passed or do we have to take care that otherwise an
exception is thrown? To give an example from shared AttributUtils:
We never check if the object to clone is a String, we just presume that
any other developer using this class respects the convention given by
javadoc. Can we do so or should an exception be thrown in such cases?
This question will appear for sure for other methods as well.
/**
* Clone the value. An attribute value is supposed to be either a String
* or a byte array. If it's a String, then we just return it ( as String
* is immutable, we don't need to copy it). If it's a bu=yte array, we
* create a new byte array and copy the bytes into it.
*
* @param value The value to clone
* @return The cloned value
*/
public static Object cloneValue( Object value )
{
// First copy the value
Object newValue = null;
if ( value instanceof byte[] )
{
newValue = ( ( byte[] ) value ).clone();
}
else
{
newValue = value;
}
return newValue;
}
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkwIxz4ACgkQ2lZVCB08qHHdxgCfdoxVZnY6OLr33zaD+p0+Dz6T
LU8AnisthtKmNWYd3faao3mBgsrF4IK9
=2x2R
-END PGP SIGNATURE-
Re: How strict must we check in util methods
On Fri, Jun 4, 2010 at 12:28 PM, Felix Knecht fe...@otego.com wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Do we leave it up to third party developers to take care that only correct values are passed or do we have to take care that otherwise an exception is thrown? To give an example from shared AttributUtils: We never check if the object to clone is a String, we just presume that any other developer using this class respects the convention given by javadoc. Can we do so or should an exception be thrown in such cases? think it is ok to assume the other value as string, IMHO a throws clause makes the caller code cluttered with try/catch blocks (assuming that we are throwing a checked exception) P.S:- hmm not sure if throwing a IllegalArgumentException is the right idea here, but is a good choice thanks Felix Kiran Ayyagari
Re: How strict must we check in util methods
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 11:36, Kiran Ayyagari wrote: On Fri, Jun 4, 2010 at 12:28 PM, Felix Knecht fe...@otego.com wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Do we leave it up to third party developers to take care that only correct values are passed or do we have to take care that otherwise an exception is thrown? To give an example from shared AttributUtils: We never check if the object to clone is a String, we just presume that any other developer using this class respects the convention given by javadoc. Can we do so or should an exception be thrown in such cases? think it is ok to assume the other value as string, IMHO a throws clause makes the caller code cluttered with try/catch blocks (assuming that we are throwing a checked exception) P.S:- hmm not sure if throwing a IllegalArgumentException is the right idea here, but is a good choice Hehe ... Sounds like the answer of a politician :-) thanks Felix Kiran Ayyagari -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwIyzcACgkQ2lZVCB08qHG6dwCfQLkHMgKNTWMpGN/hhtcJFs63 i3kAn0LzOKCH+aWo60OQWLbvLUX6SbdL =nWpg -END PGP SIGNATURE-
Logical Operators '' vs Conditional-And Operator ''
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 To me it would look more logical use everywhere conditional operator '' in the if restriction [1]. I don't think it's a false positive of findbugs NS: Potentially dangerous use of non-short-circuit logic (NS_DANGEROUS_NON_SHORT_CIRCUIT) This code seems to be using non-short-circuit logic (e.g., or |) rather than short-circuit logic ( or ||). In addition, it seem possible that, depending on the value of the left hand side, you might not want to evaluate the right hand side (because it would have side effects, could cause an exception or could be expensive. Non-short-circuit logic causes both sides of the expression to be evaluated even when the result can be inferred from knowing the left-hand side. This can be less efficient and can result in errors if the left-hand side guards cases when evaluating the right-hand side can generate an error. [1] http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/schema/syntaxCheckers/UuidSyntaxChecker.html#85 PS: Hope there's still enough coffee left ;-) -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwIy24ACgkQ2lZVCB08qHFYDgCePA6XE35v7gLoyhMUVAWeOILR jjgAnA+CIdT9Ot856YW3h9Huxl0fhgdO =K9BW -END PGP SIGNATURE-
Re: Interceptor Partition methods will now return a LdapException
On Fri, Jun 4, 2010 at 10:45 AM, Emmanuel Lécharny elecha...@apache.orgwrote: On 6/4/10 9:08 AM, Alex Karasulu wrote: On Thu, Jun 3, 2010 at 4:49 PM, Emmanuel Lecharnyelecha...@gmail.com wrote: Hi guys, just to inform you that in order to get rid of NamingException from the core server, I'm replacing it by LdapException, and that will lead to some modification in the core server : every method were throwing a Exception, and will now throw a LdapException. Not a big change from the server POV, but that impact a lot of code. +1 One question: Is LdapException shared with the ldap-api as well? Yes Wondering this because it would be nice for us in several respect if they are both the same exception class hierarchy root. They are. Excellent thanks. -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
On 6/4/10 10:10 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/4/10 1:03 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/3/10 11:04 PM, Stefan Seelmann wrote: Hi dev, the core-mock module includes some mock implementations of ApacheDS core-api classes (CoreSession, DirectoryService, etc.). It is only used as test dependency in ldif-partition. To get rid of the core-mock module I'd like to suggest to move those classes to src/test/java in core-api. To be able to use the test classes outside of the core-api module we just need to deploy the test jar of core-api. The using module then needs to specify a dependency with classifier tests, see [1] for details. We also had a discussion with Pierre-Arnaud about those unit tests. Once upon a time, we have had some core-unti and server-unit modules. They have been renamed core-intger and server-integ. So far, so good, but we still have another module called apacheds-test-framework. We do think that the classes we have in core/server-integ - I mean, the classes in java/main, not the tests - should be moved to apacheds-test-framework. That being said, we could also move the core-mock into this apacheds-test-framework module. Does it make sense ? I'm afraid that won't be possible because this would cause cyclic dependencies. The integ tests and the test framework start up a real directory service with real partitions, including an LDIF based config and schema partition, so they depend on the ldif-partition. The mocks are used because we can't start a real directory service in ldif-partion module for unit tests. But we can run the integration tests against the LDIF partition. Ok, so we have the exact same problem than with the LDAP API : we had to extract the api tests and put them in ApacheDS just to be able to launch a real server. What about doing the same thing and move all the Ldif-Partition tests out of the ldif-partition module and put them where they can use the real DS ? I mean, tests are not to be close to the part they are testing? thoughts ? I think we should distinguish unit tests and integration tests. Unit tests should be close to the part they are testing. And they should not test other components around. Thus it is valid to use mocks. Absolutely The tests in ldif-partition are unit tests because they just test that the ldif-partition code works, that is that files in the file system are created or deleted. So I think these tests are at the right place. So where should we put the mocks ? Core-API as you suggested ? (IMO, if that solve the problem, then +1) Maybe what I'm suggesting is a bit too convoluted anyway. Kind Regards, Stefan -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Should we use assert ?
Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: How strict must we check in util methods
On 6/4/10 11:28 AM, Felix Knecht wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Do we leave it up to third party developers to take care that only
correct values are passed or do we have to take care that otherwise an
exception is thrown? To give an example from shared AttributUtils:
We never check if the object to clone is a String, we just presume that
any other developer using this class respects the convention given by
javadoc. Can we do so or should an exception be thrown in such cases?
This question will appear for sure for other methods as well.
/**
* Clone the value. An attribute value is supposed to be either a String
* or a byte array. If it's a String, then we just return it ( as String
* is immutable, we don't need to copy it). If it's a bu=yte array, we
* create a new byte array and copy the bytes into it.
*
* @param value The value to clone
* @return The cloned value
*/
public static Object cloneValue( Object value )
{
// First copy the value
Object newValue = null;
if ( value instanceof byte[] )
{
newValue = ( ( byte[] ) value ).clone();
}
else
{
newValue = value;
}
return newValue;
}
Hi Felix,
I think I replied by sending another mail (I wrote it 2 hours ago while
I ddn't have internet connection ...)
I think that this is exacty what 'assert' is good for :
public static Object cloneValue( Object value )
{
assert ( value != null );
assert ( ( value instanceof String ) || ( value instanceof byte[] ) )
// First copy the value
Object newValue = null;
...
--
Regards,
Cordialement,
Emmanuel Lécharny
www.nextury.com
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
Emmanuel Lecharny schrieb: On 6/4/10 10:10 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/4/10 1:03 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/3/10 11:04 PM, Stefan Seelmann wrote: Hi dev, the core-mock module includes some mock implementations of ApacheDS core-api classes (CoreSession, DirectoryService, etc.). It is only used as test dependency in ldif-partition. To get rid of the core-mock module I'd like to suggest to move those classes to src/test/java in core-api. To be able to use the test classes outside of the core-api module we just need to deploy the test jar of core-api. The using module then needs to specify a dependency with classifier tests, see [1] for details. We also had a discussion with Pierre-Arnaud about those unit tests. Once upon a time, we have had some core-unti and server-unit modules. They have been renamed core-intger and server-integ. So far, so good, but we still have another module called apacheds-test-framework. We do think that the classes we have in core/server-integ - I mean, the classes in java/main, not the tests - should be moved to apacheds-test-framework. That being said, we could also move the core-mock into this apacheds-test-framework module. Does it make sense ? I'm afraid that won't be possible because this would cause cyclic dependencies. The integ tests and the test framework start up a real directory service with real partitions, including an LDIF based config and schema partition, so they depend on the ldif-partition. The mocks are used because we can't start a real directory service in ldif-partion module for unit tests. But we can run the integration tests against the LDIF partition. Ok, so we have the exact same problem than with the LDAP API : we had to extract the api tests and put them in ApacheDS just to be able to launch a real server. What about doing the same thing and move all the Ldif-Partition tests out of the ldif-partition module and put them where they can use the real DS ? I mean, tests are not to be close to the part they are testing? thoughts ? I think we should distinguish unit tests and integration tests. Unit tests should be close to the part they are testing. And they should not test other components around. Thus it is valid to use mocks. Absolutely The tests in ldif-partition are unit tests because they just test that the ldif-partition code works, that is that files in the file system are created or deleted. So I think these tests are at the right place. So where should we put the mocks ? Core-API as you suggested ? (IMO, if that solve the problem, then +1) Yes, this would solve the problem. Another option that also works: as the mocks are only used by the LDIF partition tests we can also move them to src/test/java of ldif-partition. I think that's the better option.
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Another option that also works: as the mocks are only used by the LDIF partition tests we can also move them to src/test/java of ldif-partition. I think that's the better option. +1 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwI7I0ACgkQ2lZVCB08qHHvjgCghKyfA3uDYeTg32ck0UOKbJlY K6QAoObgzUi1VzuaW0HPsVAGyqKe2+EG =qIZR -END PGP SIGNATURE-
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
On 6/4/10 2:01 PM, Stefan Seelmann wrote: So where should we put the mocks ? Core-API as you suggested ? (IMO, if that solve the problem, then +1) Yes, this would solve the problem. Another option that also works: as the mocks are only used by the LDIF partition tests we can also move them to src/test/java of ldif-partition. I think that's the better option. What if we want to use those mocks in another module in the near (or not so near) future ? Wouldn't it be better to just let the mock be accessible by moving them in core-api? (playing devil's advocate here...) -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Should we use assert ?
Emmanuel Lecharny wrote: Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? I never used them before. For me assert is a bit magic because they are disabled at runtime and the assert statement isn't evaluated unless they are enabled. I prever to throw a IllegalArgumentException instead. My 2 cents, Stefan
Re: Should we use assert ?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 12:15, Emmanuel Lecharny wrote: Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? +1 Just make sure, that assert isn't used in places where it's shared/apacheds/... specific as an AssertationError is thrown in case. What about error messages? Do we need them here? If so it can blow up the code, just imagine something like assert ( ( value instanceof String ) || ( value instanceof byte[] ) ) : String or byte[] expected instead of + value.getClass() -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwI7rUACgkQ2lZVCB08qHEFLQCfSzuVCIOGenudn7TExxLO/2EL 7JIAoIOuHtV2fqafEsVQLnkTjo+mFEpZ =QAuM -END PGP SIGNATURE-
Re: Architecture Diagram for Object LDAP persistence Tooling : gsoc2010
Stefan Seelmann wrote:
Kasun Lakpriya wrote:
Hi all,
I have added some description about each and every component to make
the diagram descriptive. Please add your suggestions and modifications
need to added to this. It can be found in the same link [1] below.
[1] -
http://code.google.com/p/dirstudio-ldap-tooling/wiki/ArchitectureDiagram
Looks very great!
I'm not sure if it is necessary to distinguish between Apache Directory
Studio and Eclipse. The LDAP Persistence Tooling makes only sense
when the Studio plugins are installed within the Eclipse IDE.
For the Code Generator you should use a template engine because I think
it allows great flexibility.
The Schema Creator part is a bit unclear. What do you mean with the
second bullet Directly using the API?
I think it's time to make your hands dirty :-) . You already mentioned in
IRC that you'd like to start with path A, that's great. I'd recommend to
create a first prototype:
- Create an new UI plugin that adds a new menu item to the LDAP Browser
context menu used to select an entry and to call the analyzer
- Create a simple version of LDAP entry and schema Analyzer that just
extracts the structural object class from the entry and all user
attributes from the schema
- Select a template engine for the code generator
- Create a simple template that just generates a Java class named like
the structural object class (capitalize the first letter) and with
attributes (type Object) for all user attributes. No DAO yet.
So for example, when selecting an inetOrgPerson entry the generated
Java class looks like this:
public InetOrgPerson
{
private Object objectClass
private Object cn;
private Object givenName;
private Object telephoneNumber;
}
I think it is important to get some working code very early. Then it is
much easier to discuss improvements and to add new features.
Just fyi: I created the initial project structure and maven project, see
[1].
Kind Regards,
Stefan
[1]http://dirstudio-ldap-tooling.googlecode.com/svn/trunk/persistence-tooling/
Re: Should we use assert ?
On 6/4/10 2:10 PM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? I never used them before. For me assert is a bit magic because they are disabled at runtime and the assert statement isn't evaluated unless they are enabled. I prever to throw a IllegalArgumentException instead. It's funny because I was an heavy user of asserts when I was writing C code, and it saved me a hell lot of time. Here, we should make a distinction between using asserts and if(). Let me take an exemple. Suppose we are writing an User API, with a method taking a parameter that should not be null. Clearly, using an assert in this case is *not* valid, as it might be disabled art runtime. Now, if we are writing a method used internally, then there is no need to do a if( condition not met ) then throw new IllegalArgumentException, as it's unlikely to happen, if we have tests covering those cases. However, I won't push to much if you feel more comfortable using an if+ IllegalArgumentException. In any case, what is important is to cover the case of bad arguments being used... My 2 cents, Stefan -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Should we use assert ?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Reading http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html#usage I have to withdraw my +1 and agree with Stefans objections On 06/04/10 14:16, Felix Knecht wrote: On 06/04/10 12:15, Emmanuel Lecharny wrote: Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? +1 Just make sure, that assert isn't used in places where it's shared/apacheds/... specific as an AssertationError is thrown in case. What about error messages? Do we need them here? If so it can blow up the code, just imagine something like assert ( ( value instanceof String ) || ( value instanceof byte[] ) ) : String or byte[] expected instead of + value.getClass() -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwI77AACgkQ2lZVCB08qHGVDACfdHyhixrHw/gfZqQXxxiWdok7 3TEAn1o3pr6VinMwDhEt5/G/lNTbQMBK =zjLR -END PGP SIGNATURE-
Re: Should we use assert ?
On 6/4/10 2:16 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 12:15, Emmanuel Lecharny wrote: Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? +1 Just make sure, that assert isn't used in places where it's shared/apacheds/... specific as an AssertationError is thrown in case. See my other mail about when to use asserts or not (ie, User API should not use them) What about error messages? Do we need them here? If so it can blow up the code, just imagine something like assert ( ( value instanceof String ) || ( value instanceof byte[] ) ) : String or byte[] expected instead of + value.getClass() Yes, assert must be used with a message. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwI7rUACgkQ2lZVCB08qHEFLQCfSzuVCIOGenudn7TExxLO/2EL 7JIAoIOuHtV2fqafEsVQLnkTjo+mFEpZ =QAuM -END PGP SIGNATURE- -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: [ApacheDS] Merge module avl-partition into xdbm-search and rename xdbm-search to xdbm-partition
Stefan Seelmann wrote: Hi dev, I'd like to move the only remaining class AvlPartition from module avl-partition into module xdbm-search. Additional I'd like to rename xdbm-search to xdbm-partition. The xdbm-partition module than contains everything that is related to XDBM partitions: - the Store interfaces and a default AVL implementation - the XDBM search engine interfaces and the default implementation - the XDBM partition interfaces and a default AVL implementation Done. Please make sure to delete the old avl-partition and xdmb-search folders in your file system in case they are not deleted by svn up. You also need to import the new xdbm-partition module into your eclipse workspace. Kind Regards, Stefan
Re: Should we use assert ?
On 6/4/10 2:21 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Reading http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html#usage I have to withdraw my +1 and agree with Stefans objections Ok, not a big deal :) -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
Emmanuel Lecharny wrote: On 6/4/10 2:01 PM, Stefan Seelmann wrote: So where should we put the mocks ? Core-API as you suggested ? (IMO, if that solve the problem, then +1) Yes, this would solve the problem. Another option that also works: as the mocks are only used by the LDIF partition tests we can also move them to src/test/java of ldif-partition. I think that's the better option. What if we want to use those mocks in another module in the near (or not so near) future ? Wouldn't it be better to just let the mock be accessible by moving them in core-api? (playing devil's advocate here...) Should I bring another option into the game: Using a mock framework like EaseMock or Mockito? Let's keep it simple. It isn't a big difference to move them to core-api or ldif-partition. The latter option is just a bit simpler to solve the current problem. But I agree with you, moving them to core-api makes it easier to reuse the mocks. Kind Regards, Stefan
Re: Exception Logging
On Jun 3, 2010, at 2:15 PM, Felix Knecht wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
We have a lot of following constructs:
log.error( I18n.err( I18n.ERR_04007 ) );
throw new DecoderException( I18n.err( I18n.ERR_04007 ) );
What about logging the exception within the exception itself like
public DecoderException(String message)
{
super( message );
log.error( message );
}
This will avoid having log.error all over the place and the translation
must be done only once instead of twice like above.
This is not a very good pattern for a number of reasons.
First, you cannot control logging at the source of the error. Admittedly this
is an error message but I have run into times where I want to turn off the
klaxon to see what the real problem is.
Second, constructors should not have side effects. It's never a good idea.
Third, I never log an error if I am throwing an exception. It just adds noise.
I will, however, log additional useful information that is not in the
exception message. Just parroting what's in the exception is of little value.
Finally, what the heck is ERR_04007? :) I thought there already was a
discussion and community consensus about how there is little to negative value
in using numbers as error messages. Maybe I missed the conversation where this
opinion was reversed. If so, ignore this bit. :)
Regards,
Alan
[Shared] AddRequestCodec
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 A NPE may be thrown at line 280 if 'entry==null'. As the method doesn't declares a thrown exception I suppose that a default length (like -1) should be returned if the coumputing fails? The method endProtocolOp lacks the same problem (entry==null) but declares a thrown exception, so the NPE can be avoided and a specific error can be thrown. http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/codec/add/AddRequestCodec.html#280 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwI+GsACgkQ2lZVCB08qHFB/wCdFu5zXAjz+avMycSZ2B7T5lnu fBMAoJxb//IVhqPCMuIiAebZcwNR0jaZ =xQQc -END PGP SIGNATURE-
[jira] Assigned: (DIRSERVER-1515) Remove the 'throw new NullPointerException' in the code
[ https://issues.apache.org/jira/browse/DIRSERVER-1515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Emmanuel Lecharny reassigned DIRSERVER-1515: Assignee: Emmanuel Lecharny Remove the 'throw new NullPointerException' in the code --- Key: DIRSERVER-1515 URL: https://issues.apache.org/jira/browse/DIRSERVER-1515 Project: Directory ApacheDS Issue Type: Bug Affects Versions: 1.5.7 Reporter: Emmanuel Lecharny Assignee: Emmanuel Lecharny Fix For: 2.0.0-RC1 There are 43 places where we throw a NullPointerException explicitely. This is very worng. If we do that when a method argument is null, we should instead throw a IllegalArgumentException. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
On 6/4/10 2:53 PM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/4/10 2:01 PM, Stefan Seelmann wrote: So where should we put the mocks ? Core-API as you suggested ? (IMO, if that solve the problem, then +1) Yes, this would solve the problem. Another option that also works: as the mocks are only used by the LDIF partition tests we can also move them to src/test/java of ldif-partition. I think that's the better option. What if we want to use those mocks in another module in the near (or not so near) future ? Wouldn't it be better to just let the mock be accessible by moving them in core-api? (playing devil's advocate here...) Should I bring another option into the game: Using a mock framework like EaseMock or Mockito? If it works, then totally +1 Let's keep it simple. It isn't a big difference to move them to core-api or ldif-partition. The latter option is just a bit simpler to solve the current problem. But I agree with you, moving them to core-api makes it easier to reuse the mocks. Here, I would say it's up to you. No need to spend more time on discussing on the mailing list about the pros and cons ad nauseam, it's not changing the way the server works :) As soon as we know what are our options, and as it's now just a choice to make, then let's go for what you think is the best. Thanks ! -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Should we use assert ?
On Jun 4, 2010, at 5:10 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: Hi guys, currently (and probably because nobody uses them, due to some Java 1.3 habits we have), we don't use asserts to do simple things like checking methods parameters (pre-conditions). Should we start using them ? I never used them before. For me assert is a bit magic because they are disabled at runtime and the assert statement isn't evaluated unless they are enabled. I prever to throw a IllegalArgumentException instead. I like asserts because they have the dual function of performing useful sanity checking when turned on and also provide implicit documentation to other developers as to what my fundamental assumptions are when I designed my code. Take for example assert Thread.holdsLock(foo) : I should be protected before you call me; This is a nice bit of code that explains my assumptions to other developers when they use/extend my code with the added benefit that it gets turned off once the code is deployed in production. Slavish use of asserts can lead to trouble; they are not a panacea. Frontline argument and state checking at the client API end should never use asserts for obvious reasons. However, performing the same kind of heavy weight checking in my internal classes where I have complete control is a bit of overkill; here asserts fit nicely. They also serve to warn the developer that they are now troweling inside the bowels of a codebase. Just my 2 cents. Regards, Alan
Re: [Shared] AddRequestCodec
Emmanuel Lecharny wrote: On 6/4/10 2:58 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 A NPE may be thrown at line 280 if 'entry==null'. As the method doesn't declares a thrown exception I suppose that a default length (like -1) should be returned if the coumputing fails? Good point. I think we can test that the entry is not null, and if so, throw an IllegalArgumentException (IAE in the next ten mails, as i'm fedup of typing it everytime in extenso ;) Isn't there a mail plugin for Eclipse that provides code completion?
Re: Exception Logging
On 6/4/10 2:55 PM, Alan D. Cabrera wrote: Finally, what the heck is ERR_04007? :) I thought there already was a discussion and community consensus about how there is little to negative value in using numbers as error messages. Maybe I missed the conversation where this opinion was reversed. If so, ignore this bit. :) No, nobody missed this valid concern. We are just modifying them one by one when fixing issues using those error code : ERR_02014_UNSUPPORTED_OPERATION( ERR_02014_UNSUPPORTED_OPERATION ), ... ERR_04219_ARGUMENT1_NULL( ERR_04219_ARGUMENT1_NULL ), ERR_04220_ARGUMENT2_NULL( ERR_04220_ARGUMENT2_NULL ), ... ERR_04442_NULL_AT_NOT_ALLOWED( ERR_04442_NULL_AT_NOT_ALLOWED ), ... ERR_04457_NULL_ATTRIBUTE_ID( ERR_04457_NULL_ATTRIBUTE_ID ), ... ERR_04460_ATTRIBUTE_TYPE_NULL_NOT_ALLOWED( ERR_04460_ATTRIBUTE_TYPE_NULL_NOT_ALLOWED ), ... ERR_04477_NO_VALID_AT_FOR_THIS_ID( ERR_04477_NO_VALID_AT_FOR_THIS_ID ), ERR_04478_NO_VALUE_NOT_ALLOWED( ERR_04478_NO_VALUE_NOT_ALLOWED ), ERR_04479_INVALID_SYNTAX_VALUE( ERR_04479_INVALID_SYNTAX_VALUE ), ... ERR_09001_DIRECTORY_CREATION_FAILED( ERR_09001_DIRECTORY_CREATION_FAILED ), ... ERR_254_ADD_EXISTING_VALUE(ERR_254_ADD_EXISTING_VALUE), ... ERR_272_MODIFY_LEAVES_NO_STRUCTURAL_OBJECT_CLASS(ERR_272_MODIFY_LEAVES_NO_STRUCTURAL_OBJECT_CLASS), ... ERR_539_BAD_BLOCK_ID(ERR_539_BAD_BLOCK_ID), We have something like one thousands of them, it will take time, but this approach works. Be patient :) -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Exception Logging
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 No worries. :) Why do they all have numbers in them? Because there's also a request to build a knowledgebase [1]. It's easier having numbers to lookup something in a knowledgebase than having a text which may occur multiple times and for different exceptions. Felix [1] https://issues.apache.org/jira/browse/DIRSERVER-886 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJBloACgkQ2lZVCB08qHExZQCcDe2ah3Ecqcgi4AduLeeh0ghN 6hQAoNnlAKU3ly+4ejuZ42TSxaaMw3mj =1Y7i -END PGP SIGNATURE-
Re: Exception Logging
On 6/4/10 3:44 PM, Alan D. Cabrera wrote: On Jun 4, 2010, at 6:13 AM, Emmanuel Lecharny wrote: On 6/4/10 2:55 PM, Alan D. Cabrera wrote: Finally, what the heck is ERR_04007? :) I thought there already was a discussion and community consensus about how there is little to negative value in using numbers as error messages. Maybe I missed the conversation where this opinion was reversed. If so, ignore this bit. :) No, nobody missed this valid concern. We are just modifying them one by one when fixing issues using those error code : ERR_02014_UNSUPPORTED_OPERATION( ERR_02014_UNSUPPORTED_OPERATION ), ... ERR_04219_ARGUMENT1_NULL( ERR_04219_ARGUMENT1_NULL ), ERR_04220_ARGUMENT2_NULL( ERR_04220_ARGUMENT2_NULL ), ... ERR_04442_NULL_AT_NOT_ALLOWED( ERR_04442_NULL_AT_NOT_ALLOWED ), ... ERR_04457_NULL_ATTRIBUTE_ID( ERR_04457_NULL_ATTRIBUTE_ID ), ... ERR_04460_ATTRIBUTE_TYPE_NULL_NOT_ALLOWED( ERR_04460_ATTRIBUTE_TYPE_NULL_NOT_ALLOWED ), ... ERR_04477_NO_VALID_AT_FOR_THIS_ID( ERR_04477_NO_VALID_AT_FOR_THIS_ID ), ERR_04478_NO_VALUE_NOT_ALLOWED( ERR_04478_NO_VALUE_NOT_ALLOWED ), ERR_04479_INVALID_SYNTAX_VALUE( ERR_04479_INVALID_SYNTAX_VALUE ), ... ERR_09001_DIRECTORY_CREATION_FAILED( ERR_09001_DIRECTORY_CREATION_FAILED ), ... ERR_254_ADD_EXISTING_VALUE(ERR_254_ADD_EXISTING_VALUE), ... ERR_272_MODIFY_LEAVES_NO_STRUCTURAL_OBJECT_CLASS(ERR_272_MODIFY_LEAVES_NO_STRUCTURAL_OBJECT_CLASS), ... ERR_539_BAD_BLOCK_ID(ERR_539_BAD_BLOCK_ID), We have something like one thousands of them, it will take time, but this approach works. Be patient :) No worries. :) Why do they all have numbers in them? Grep is your friend when you have accurate elements to work on :) Numbers are less versatile than textual descriptions. /me remember a 5 days debugging session on a production serveur generating 5Gb of logs a day, using grep to grab the errors I was looking for... -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
[jira] Resolved: (DIRSERVER-1515) Remove the 'throw new NullPointerException' in the code
[ https://issues.apache.org/jira/browse/DIRSERVER-1515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Emmanuel Lecharny resolved DIRSERVER-1515. -- Resolution: Fixed Fixed with http://svn.apache.org/viewvc?rev=951408view=rev Remove the 'throw new NullPointerException' in the code --- Key: DIRSERVER-1515 URL: https://issues.apache.org/jira/browse/DIRSERVER-1515 Project: Directory ApacheDS Issue Type: Bug Affects Versions: 1.5.7 Reporter: Emmanuel Lecharny Assignee: Emmanuel Lecharny Fix For: 2.0.0-RC1 There are 43 places where we throw a NullPointerException explicitely. This is very worng. If we do that when a method argument is null, we should instead throw a IllegalArgumentException. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
Is 'null' ever valid syntax?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I suppose this is either always true or false, right? if ( attributeType.getSyntax().getSyntaxChecker().isValidSyntax( null ) ) http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#916 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJCTkACgkQ2lZVCB08qHElmQCguqsL1rbCuaO6RGpcyedAfEqq RYMAoJ4cX/HAqZy7fILR18aGcPLedmII =hXAi -END PGP SIGNATURE-
Code is never executed
This will always match for the else clause - we now the attributeType is null (line 1469). Probably dead code? http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#1475
Re: Is 'null' ever valid syntax?
On 6/4/10 4:10 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I suppose this is either always true or false, right? No. It depends on the AttributeType. Some accept null values, some don't. This is the reason we have such a test. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Code is never executed
On 6/4/10 4:14 PM, Felix Knecht wrote: This will always match for the else clause - we now the attributeType is null (line 1469). Probably dead code? http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#1475 Dohhh ! Good catch again :) I will soon think that code reviews should be mandatory :) -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Is 'null' ever valid syntax?
Felix Knecht wrote: I suppose this is either always true or false, right? if ( attributeType.getSyntax().getSyntaxChecker().isValidSyntax( null ) ) http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#916 No. For example the directory string syntax doesn't allow an empty string, so in that case the syntax checker returns false. But the IA5 string syntax allows an empty string, in that case the syntax checker returns false. Kind Regards, Stefan
Re: Is 'null' ever valid syntax?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 16:32, Stefan Seelmann wrote: Felix Knecht wrote: I suppose this is either always true or false, right? if ( attributeType.getSyntax().getSyntaxChecker().isValidSyntax( null ) ) http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#916 No. For example the directory string syntax doesn't allow an empty string, so in that case the syntax checker returns false. But the IA5 string syntax allows an empty string, in that case the syntax checker returns false. I have no idea about the checkers, but an empty string isn't a null string. Regards Felix -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJD8IACgkQ2lZVCB08qHEKmQCdEhuBtJn4Z51HTMQ//dUOm7vz w5IAn13NQhOfhJWOY2879a4ILMPzWEPm =SUik -END PGP SIGNATURE-
Re: Code is never executed
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 16:32, Emmanuel Lecharny wrote: On 6/4/10 4:14 PM, Felix Knecht wrote: This will always match for the else clause - we now the attributeType is null (line 1469). Probably dead code? http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#1475 Dohhh ! Good catch again :) Well, is it dead code now? I will soon think that code reviews should be mandatory :) The tools are already doing this for us. I just seem to be the one ATM ( ... as I have still not much knowledge of the code ...) looking at the reports which are generated. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJEEcACgkQ2lZVCB08qHF11gCgpw0dWygqevU3sos8m5kqDR1j l5cAn2o553g8lgUE4KXoVjj130LK+n23 =NSVr -END PGP SIGNATURE-
[jira] Commented: (DIRSERVER-1252) Server tools dump command broken due to use of old paths
[ https://issues.apache.org/jira/browse/DIRSERVER-1252?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12875614#action_12875614 ] Emmanuel Lecharny commented on DIRSERVER-1252: -- It's even a bit more complicated than that : not only the file paths layout has changed, but we now handle many different kind of partitions (Ldif, Hbase, Oracle...) We have to see if we want to keep the tools in the current distribution... Server tools dump command broken due to use of old paths Key: DIRSERVER-1252 URL: https://issues.apache.org/jira/browse/DIRSERVER-1252 Project: Directory ApacheDS Issue Type: Bug Affects Versions: 1.5.5, 1.5.4 Reporter: Alex Karasulu Assignee: Alex Karasulu Priority: Critical Fix For: 2.0.0-RC1 When running the dump command I noticed that it is trying to validate the old layout of file paths after giving the base path using the -i switch. For example if I give it /foo/bar it will try to validate that the following structure exists: /foo/bar/bin /foo/bar/lib /foo/bar/lib/ext /foo/bar/var /foo/bar/var/run /foo/bar/var/partition /foo/bar/var/partition/schema /foo/bar/var/partition/system This is wrong since we no longer use this schema. This whole layout thingy needs to be seriously revisited along with the code in ApacheDS.java. A mini revamp is in order to avoid these issues and let the commands work properly. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
What should we do with apacheds-server-tools ?
Hi guys, this module is broken since, pfeww, september 11 2008 (what a coincidence ! Bad things always happen on 11/09...) The question is : should we keep going and fix this module or should we move it to the deceased projects ? A thrd option would be to make this module a separate project we can release separately from ADS 2.0. Wdyt is the best ? -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Exception Logging
On Jun 4, 2010, at 6:58 AM, Emmanuel Lecharny wrote: No worries. :) Why do they all have numbers in them? Grep is your friend when you have accurate elements to work on :) Numbers are less versatile than textual descriptions. /me remember a 5 days debugging session on a production serveur generating 5Gb of logs a day, using grep to grab the errors I was looking for... On Jun 4, 2010, at 6:57 AM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 No worries. :) Why do they all have numbers in them? Because there's also a request to build a knowledgebase [1]. It's easier having numbers to lookup something in a knowledgebase than having a text which may occur multiple times and for different exceptions. All good reasons. Regards, Alan
Re: What should we do with apacheds-server-tools ?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 The question is : should we keep going and fix this module or should we move it to the deceased projects ? A thrd option would be to make this module a separate project we can release separately from ADS 2.0. - From an administrators POV (who doesn't like GUIs anyway) I'd say we still need them. How to make a e.g. a dump on a server machine? Using studio (hopefully the requested admin function exists in studio) and then doing an ssh X-Forward tunnel and get the dump on the local machine instead on the server? I'd prefer the third option. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJFBYACgkQ2lZVCB08qHHDigCgx4UATiEwRKP2g6JH90EOFahy E0kAoOYXgSgFuXgYY8nY6ZJ8Hbh255k8 =U8DX -END PGP SIGNATURE-
Re: Is 'null' ever valid syntax?
On 6/4/10 4:37 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 16:32, Stefan Seelmann wrote: Felix Knecht wrote: I suppose this is either always true or false, right? if ( attributeType.getSyntax().getSyntaxChecker().isValidSyntax( null ) ) http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#916 No. For example the directory string syntax doesn't allow an empty string, so in that case the syntax checker returns false. But the IA5 string syntax allows an empty string, in that case the syntax checker returns false. I have no idea about the checkers, but an empty string isn't a null string. I think ( to be double checked) that we allow a Value to old either or null. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Code is never executed
On 6/4/10 4:40 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 16:32, Emmanuel Lecharny wrote: On 6/4/10 4:14 PM, Felix Knecht wrote: This will always match for the else clause - we now the attributeType is null (line 1469). Probably dead code? http://people.apache.org/~felixk/shared-docs/xref/org/apache/directory/shared/ldap/entry/DefaultEntryAttribute.html#1475 Dohhh ! Good catch again :) Well, is it dead code now? I guess so. I will soon think that code reviews should be mandatory :) The tools are already doing this for us. I just seem to be the one ATM ( ... as I have still not much knowledge of the code ...) looking at the reports which are generated. I *do* look at the reports, but not as frenquently as you do. Probably because I consider that we have many other serious issues to fix in the server, and I wrongly assume that they are more important than the one you are pointing out. The key is : there is nothing such as frivolous errors. An error is an error, and has to be fixed. So keep going, and trust me, those reports are really important, or all of us. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: What should we do with apacheds-server-tools ?
On 6/4/10 4:56 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 The question is : should we keep going and fix this module or should we move it to the deceased projects ? A thrd option would be to make this module a separate project we can release separately from ADS 2.0. - From an administrators POV (who doesn't like GUIs anyway) I'd say we still need them. How to make a e.g. a dump on a server machine? Using studio (hopefully the requested admin function exists in studio) and then doing an ssh X-Forward tunnel and get the dump on the local machine instead on the server? It all depends on which command we are talking about here. On any linux box, you have many tools like ldapadd or ldapsearch existing. No need to rewite those guys (just because starting a command which spawns a JVM is too freaking slow). But when it comes to those admin commands, then yes, we should offer a non GUI tool for the admin. One interesting idea would have been to use a tool like ldapvi, or even better, writing our own GUI. The only problm I have right now with the current approach, is that we don't fix the existing issues, we just postpone them over and over. At this point, if we don't move the project to the attic, then I really think it should be a side project, like the Ldap API. One good thing about having this as a side project : it's less intimidating for someone who want to become a committer to start by playing around such a piece of code, as it won't break the main server. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: Code is never executed
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I *do* look at the reports, but not as frenquently as you do. Probably because I consider that we have many other serious issues to fix in the server, and I wrongly assume that they are more important than the one you are pointing out. I didn't want to blame anyone. I just do what I'm able to ;-). Both things are needed: Development and code review. As I started with this I continue and I really appreciate that you guys are going ahead with the development (and are sometimes interrupted with some 'odd' questions). We need both! Go ahead and I try to read the reports :-) Just one question Is it ok to do this via dev list or shall I add jira entries which seems to me an overkill? -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJFzMACgkQ2lZVCB08qHFRWgCg1UxGgMAuLzw1iSnsvhGEHGHN 45cAoLXQQLPe6QQxi0ekkUWy7ZmTczOB =onsP -END PGP SIGNATURE-
Re: Code is never executed
On 6/4/10 5:09 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I *do* look at the reports, but not as frenquently as you do. Probably because I consider that we have many other serious issues to fix in the server, and I wrongly assume that they are more important than the one you are pointing out. I didn't want to blame anyone. I just do what I'm able to ;-). Both things are needed: Development and code review. As I started with this I continue and I really appreciate that you guys are going ahead with the development (and are sometimes interrupted with some 'odd' questions). We need both! Go ahead and I try to read the reports :-) Absolutely :) Just one question Is it ok to do this via dev list or shall I add jira entries which seems to me an overkill? For such QR, the ML is plain ok. Fill a JIRA if you feel like the question is a bit to complicated, or if you think there is a serious bug that will need love, or if you don't have time to fix it and you don't want to see the problem vanishing under a pile of mails. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: What should we do with apacheds-server-tools ?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 It all depends on which command we are talking about here. On any linux box, you have many tools like ldapadd or ldapsearch existing. No need to rewite those guys (just because starting a command which spawns a JVM is too freaking slow). Aren't those tools mainly part of an OpenLDAP installation? Why should I use ApacheDS if I need to install OpenLDAP anyway? But when it comes to those admin commands, then yes, we should offer a non GUI tool for the admin. Yep. One good thing about having this as a side project : it's less intimidating for someone who want to become a committer to start by playing around such a piece of code, as it won't break the main server. +1 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJGPMACgkQ2lZVCB08qHGTtgCgpdYX1zNUpIuIU7RQDBnB4BHH xgYAoLZnwEZsNylUTXt1XGrhr9o3zHX6 =2PuG -END PGP SIGNATURE-
Re: What should we do with apacheds-server-tools ?
On 6/4/10 5:17 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 It all depends on which command we are talking about here. On any linux box, you have many tools like ldapadd or ldapsearch existing. No need to rewite those guys (just because starting a command which spawns a JVM is too freaking slow). Aren't those tools mainly part of an OpenLDAP installation? Why should I use ApacheDS if I need to install OpenLDAP anyway? The Ldap-utils package is not installing OpenLDAP. Just the CL tools. -- Regards, Cordialement, Emmanuel Lécharny www.nextury.com
Re: What should we do with apacheds-server-tools ?
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/04/10 17:25, Emmanuel Lecharny wrote: On 6/4/10 5:17 PM, Felix Knecht wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 It all depends on which command we are talking about here. On any linux box, you have many tools like ldapadd or ldapsearch existing. No need to rewite those guys (just because starting a command which spawns a JVM is too freaking slow). Aren't those tools mainly part of an OpenLDAP installation? Why should I use ApacheDS if I need to install OpenLDAP anyway? The Ldap-utils package is not installing OpenLDAP. Just the CL tools. God save the queen! -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwJG+YACgkQ2lZVCB08qHH15wCgkFgMPYd2PqSQ9t/78wuKmGHN OVcAoI4wJK7iYdgKMyqn8afl1JwUGhqV =mcWG -END PGP SIGNATURE-
Re: [ApacheDS] Move classes of module core-mock to src/test/java of core-api
Done. Emmanuel Lecharny wrote: On 6/4/10 2:53 PM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/4/10 2:01 PM, Stefan Seelmann wrote: So where should we put the mocks ? Core-API as you suggested ? (IMO, if that solve the problem, then +1) Yes, this would solve the problem. Another option that also works: as the mocks are only used by the LDIF partition tests we can also move them to src/test/java of ldif-partition. I think that's the better option. What if we want to use those mocks in another module in the near (or not so near) future ? The future is now :-) Some tests in apacheds-core defined their own mock classes. It was easy to remove those duplicate mock classes. Kind Regards, Stefan
[jira] Created: (DIRSHARED-62) Improve antlr generated code [shared-ldap]
Improve antlr generated code [shared-ldap] -- Key: DIRSHARED-62 URL: https://issues.apache.org/jira/browse/DIRSHARED-62 Project: Directory Shared Issue Type: Improvement Environment: All Reporter: Felix Knecht The generated code by antlr could be improved regarding performance. Listed classes below use the construct new Integer(...) a lot of times in their constructor. If these classes are created many times it's an improvement to have Integer.valueOf(...). Looking at the generated classes almost all values for new Integer are in a range of [-128,128] what matches the criterias for more performance [1]. Classes: /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/aci/AntlrACIItemCheckerLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/aci/AntlrACIItemLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/subtree/AntlrSubtreeSpecificationCheckerLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/subtree/AntlrSubtreeSpecificationLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/trigger/AntlrTriggerSpecificationLexer.java [1] http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster. Values between -128 and 127 are guaranteed to have corresponding cached instances and using valueOf is approximately 3.5 times faster than using constructor. For values outside the constant range the performance of both styles is the same. Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Long, Integer, Short, Character, and Byte. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[ApacheDS] maven-source-plugin and maven-jar-plugin
Hi dev, the following POMs define the maven-source-plugin and create a source jar during a local build: apacheds/http-integration/pom.xml apacheds/core-api/pom.xml apacheds/protocol-shared/pom.xml apacheds/xbean-spring/pom.xml apacheds/core/pom.xml apacheds/protocol-changepw/pom.xml apacheds/syncrepl/pom.xml apacheds/protocol-ntp/pom.xml apacheds/core-jndi/pom.xml apacheds/protocol-dns/pom.xml apacheds/server-jndi/pom.xml apacheds/kerberos-test/pom.xml apacheds/ldif-partition/pom.xml apacheds/jdbm-partition/pom.xml apacheds/protocol-ldap/pom.xml apacheds/interceptor-kerberos/pom.xml apacheds/protocol-kerberos/pom.xml apacheds/jdbm-store/pom.xml Additional in project/pom.xml the maven-source-plugin is defined in two profiles: release and full. So the source jars are generated and deployed during the release, that is required, so far so good. My questions: What is the full profile used for? Can we remove the definiton of the maven-source-plugin from all the POMs listed above? I think during the local build those are not requried. Kind Regards, Stefan
Re: [ApacheDS] Merge jdbm-store into jdbm-partition
So I'm going to keep the jdbm-partition module. Alex Karasulu wrote: On Fri, Jun 4, 2010 at 1:16 AM, Emmanuel Lecharny elecha...@gmail.com mailto:elecha...@gmail.com wrote: On 6/4/10 12:01 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/3/10 10:51 PM, Stefan Seelmann wrote: Hi dev, another easy refactoring is to merge the modules jdbm-store and jdbm-partiton. +1. Which one will you keep ? jdbm-store ? To me, a partition is associated with a naming context, not with an underlying store. That implies we should get rid of those XXX-partition to just keep xxx-store, and keep the partitions at a upper layer (ie, core) I wanted to keep the jdbm-partition module. To me the partition is the concept that the core knows. The core knows nothing about stores. We also define partitions in the configuration, not stores. This is how I understand the architecture: 1. The core defines the Partition interface +1 Yes. 2. XDBM provides an abstract implemementation of the Partition interface and additionally defines the Store interface and search engine. Yes. Also eventually with global identifiers (UUID) acting as primary keys for the XDBM db scheme we will be able to pull the search functionality out of the partition and place it above the partition nexus. This will make the store interface/concept less important as well. 3. The JDBM partition is a concrete implementation of the XDBM partition. It contains a Store implementation because this is forced by XDBM. Here, I disagree. JDBM is a store, not a partition. XDBM = XXX-Data Base Manager, nothing connected to the idea of Partition. We could probably say that XDBM and Store is the same concept. I think we're overloading too much meaning into Store here. Treat it as a simple interface and forget about trying to draw more meaning out of it such as Database Manager etc. Search happens on this Store which exposes all the methods needed to perform various operations. It's just an interface coalescing all these functions together into a single place so for example we can hand off a store to different parts of the XDBM implementation and have it operate on that object. But let's discuss this aspect further, I may perfectly be wrong, I'm just trying to manipulate concepts here. I think we're trying to draw more meaning from this concept which we do not need to. -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: [ApacheDS] maven-source-plugin and maven-jar-plugin
Hi Stefan, I have absolutely no idea what it is used for. Looking at the SVN history, it has been introduced by David Djenks at revision 643768 [1] on April 2008. Personally, I've never used it and I thing it can be removed. Now, about the definiton of the maven-source-plugin from all the POMs you listed, I think it's important to let it even during the local build. It is actually required by the xbean-spring project to generate correct mapping file based on annotations found in sources. But, since we're not using anymore the server-xml file, I think we don't need spring and xbean-spring anymore... So, my understanding is that as soon as we get rid of the server-xml project, we would be able to remove the definition. Regards, Pierre-Arnaud [1] - http://svn.apache.org/viewvc?rev=643768view=rev On 4 juin 2010, at 17:58, Stefan Seelmann wrote: Hi dev, the following POMs define the maven-source-plugin and create a source jar during a local build: apacheds/http-integration/pom.xml apacheds/core-api/pom.xml apacheds/protocol-shared/pom.xml apacheds/xbean-spring/pom.xml apacheds/core/pom.xml apacheds/protocol-changepw/pom.xml apacheds/syncrepl/pom.xml apacheds/protocol-ntp/pom.xml apacheds/core-jndi/pom.xml apacheds/protocol-dns/pom.xml apacheds/server-jndi/pom.xml apacheds/kerberos-test/pom.xml apacheds/ldif-partition/pom.xml apacheds/jdbm-partition/pom.xml apacheds/protocol-ldap/pom.xml apacheds/interceptor-kerberos/pom.xml apacheds/protocol-kerberos/pom.xml apacheds/jdbm-store/pom.xml Additional in project/pom.xml the maven-source-plugin is defined in two profiles: release and full. So the source jars are generated and deployed during the release, that is required, so far so good. My questions: What is the full profile used for? Can we remove the definiton of the maven-source-plugin from all the POMs listed above? I think during the local build those are not requried. Kind Regards, Stefan
Re: [ApacheDS] maven-source-plugin and maven-jar-plugin
On Fri, Jun 4, 2010 at 7:21 PM, Pierre-Arnaud Marcelot p...@marcelot.net wrote: Hi Stefan, I have absolutely no idea what it is used for. Looking at the SVN history, it has been introduced by David Djenks at revision 643768 [1] on April 2008. Personally, I've never used it and I thing it can be removed. Now, about the definiton of the maven-source-plugin from all the POMs you listed, I think it's important to let it even during the local build. It is actually required by the xbean-spring project to generate correct mapping file based on annotations found in sources. But, since we're not using anymore the server-xml file, I think we don't need spring and xbean-spring anymore... So, my understanding is that as soon as we get rid of the server-xml project, we would be able to remove the definition. ahh, ok, Seelmann was about to delete this but I asked him to keep this for some more days but I think we should remove it right now so that we eliminate any potential issues like this if they are lurking behind based on these modules. so I would say let us delete the xbean-spring and server-xml modules, wdot? Kiran Ayyagari
[jira] Commented: (DIRSHARED-62) Improve antlr generated code [shared-ldap]
[ https://issues.apache.org/jira/browse/DIRSHARED-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12875658#action_12875658 ] Emmanuel Lecharny commented on DIRSHARED-62: I'm afraid we won't be able to do anything about it, as this code is generated. We would have to fix the generator :/ Improve antlr generated code [shared-ldap] -- Key: DIRSHARED-62 URL: https://issues.apache.org/jira/browse/DIRSHARED-62 Project: Directory Shared Issue Type: Improvement Environment: All Reporter: Felix Knecht The generated code by antlr could be improved regarding performance. Listed classes below use the construct new Integer(...) a lot of times in their constructor. If these classes are created many times it's an improvement to have Integer.valueOf(...). Looking at the generated classes almost all values for new Integer are in a range of [-128,128] what matches the criterias for more performance [1]. Classes: /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/aci/AntlrACIItemCheckerLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/aci/AntlrACIItemLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/subtree/AntlrSubtreeSpecificationCheckerLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/subtree/AntlrSubtreeSpecificationLexer.java /shared-ldap/target/generated-sources/antlr/org/apache/directory/shared/ldap/trigger/AntlrTriggerSpecificationLexer.java [1] http://findbugs.sourceforge.net/bugDescriptions.html#DM_NUMBER_CTOR Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster. Values between -128 and 127 are guaranteed to have corresponding cached instances and using valueOf is approximately 3.5 times faster than using constructor. For values outside the constant range the performance of both styles is the same. Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Long, Integer, Short, Character, and Byte. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
Re: svn commit: r947413 - in /directory/shared/trunk/ldap: pom.xml src/main/java/org/apache/directory/shared/ldap/codec/LdapTransformer.java
Felix Knecht wrote: Hi Pierre-Arnaud I'm currently reviewing the dependency tree of each project in Apache DS. When generating the installers I've found a lot of new dependencies (since the last release) in the generated lib folder, and especially this one: findbugs:annotations. Are you sure of the scope of this dependency? Thanks for heads up! No. I thought it wouldn't be taken when having scope compile It's used to avoid false positives when generating the findbugs reports. provided seems to be the better solution. The problem with provided is now that the build fails with Java5 [1]. Seems like the dependent modules have problems when the annotation class is missing. As a workaround I added the findbugs annotations dependency with scope provided to each the shared, apacheds, and ldap-api parent pom. Any other idea? Kind Regards, Stefan [1]http://vmbuild.apache.org/continuum/buildResult.action?buildId=346851projectGroupId=139projectId=2675projectName=ApacheDS+1.5+Build+With+Dependencies
Re: svn commit: r947413 - in /directory/shared/trunk/ldap: pom.xml src/main/java/org/apache/directory/shared/ldap/codec/LdapTransformer.java
Hi Stefan, Kiran had the same build problem. Strangely, the problem disappeared when he removed his maven repository and tried again. Kiran, can you confirm? Maybe we could ask artifacts under org/apache/directory to be removed on the Continuum machine. Regards, Pierre-Arnaud On 4 juin 2010, at 18:48, Stefan Seelmann wrote: Felix Knecht wrote: Hi Pierre-Arnaud I'm currently reviewing the dependency tree of each project in Apache DS. When generating the installers I've found a lot of new dependencies (since the last release) in the generated lib folder, and especially this one: findbugs:annotations. Are you sure of the scope of this dependency? Thanks for heads up! No. I thought it wouldn't be taken when having scope compile It's used to avoid false positives when generating the findbugs reports. provided seems to be the better solution. The problem with provided is now that the build fails with Java5 [1]. Seems like the dependent modules have problems when the annotation class is missing. As a workaround I added the findbugs annotations dependency with scope provided to each the shared, apacheds, and ldap-api parent pom. Any other idea? Kind Regards, Stefan [1]http://vmbuild.apache.org/continuum/buildResult.action?buildId=346851projectGroupId=139projectId=2675projectName=ApacheDS+1.5+Build+With+Dependencies
Re: svn commit: r947413 - in /directory/shared/trunk/ldap: pom.xml src/main/java/org/apache/directory/shared/ldap/codec/LdapTransformer.java
On Fri, Jun 4, 2010 at 7:54 PM, Pierre-Arnaud Marcelot p...@marcelot.net wrote: Hi Stefan, Kiran had the same build problem. Strangely, the problem disappeared when he removed his maven repository and tried again. Kiran, can you confirm? yeah the same problem went away when I tried building after removing the local maven repo Kiran Ayyagari
Re: svn commit: r947413 - in /directory/shared/trunk/ldap: pom.xml src/main/java/org/apache/directory/shared/ldap/codec/LdapTransformer.java
Kiran Ayyagari wrote: On Fri, Jun 4, 2010 at 7:54 PM, Pierre-Arnaud Marcelot p...@marcelot.net wrote: Hi Stefan, Kiran had the same build problem. Strangely, the problem disappeared when he removed his maven repository and tried again. Kiran, can you confirm? yeah the same problem went away when I tried building after removing the local maven repo I cleaned my local repo, but without success. I have no problem when using Java6. But with Java5 I get those errors: [ERROR] Failure executing javac, but could not parse the error: An exception has occurred in the compiler (1.5.0_14). Please file a bug at the Java Developer Connection (http://java.sun.com/webapps/bugreport) after checking the Bug Parade for duplicates. Include your program and the following diagnostic in your report. Thank you. com.sun.tools.javac.code.Symbol$CompletionFailure: file edu/umd/cs/findbugs/annotations/SuppressWarnings.class not found
Re: Architecture Diagram for Object LDAP persistence Tooling : gsoc2010
Hi Stefan,
I got it last week ya I have checked out and looked at it. but did not
work on that last whole week as I got to complete some tasks in my
training place. Its over now and I'm now starting working on this.
And also just now I got my fixed internet connection :).
Thanks,
Kasun
On Fri, Jun 4, 2010 at 5:47 PM, Stefan Seelmann seelm...@apache.org wrote:
Stefan Seelmann wrote:
Kasun Lakpriya wrote:
Hi all,
I have added some description about each and every component to make
the diagram descriptive. Please add your suggestions and modifications
need to added to this. It can be found in the same link [1] below.
[1] -
http://code.google.com/p/dirstudio-ldap-tooling/wiki/ArchitectureDiagram
Looks very great!
I'm not sure if it is necessary to distinguish between Apache Directory
Studio and Eclipse. The LDAP Persistence Tooling makes only sense
when the Studio plugins are installed within the Eclipse IDE.
For the Code Generator you should use a template engine because I think
it allows great flexibility.
The Schema Creator part is a bit unclear. What do you mean with the
second bullet Directly using the API?
I think it's time to make your hands dirty :-) . You already mentioned in
IRC that you'd like to start with path A, that's great. I'd recommend to
create a first prototype:
- Create an new UI plugin that adds a new menu item to the LDAP Browser
context menu used to select an entry and to call the analyzer
- Create a simple version of LDAP entry and schema Analyzer that just
extracts the structural object class from the entry and all user
attributes from the schema
- Select a template engine for the code generator
- Create a simple template that just generates a Java class named like
the structural object class (capitalize the first letter) and with
attributes (type Object) for all user attributes. No DAO yet.
So for example, when selecting an inetOrgPerson entry the generated
Java class looks like this:
public InetOrgPerson
{
private Object objectClass
private Object cn;
private Object givenName;
private Object telephoneNumber;
}
I think it is important to get some working code very early. Then it is
much easier to discuss improvements and to add new features.
Just fyi: I created the initial project structure and maven project, see
[1].
Kind Regards,
Stefan
[1]http://dirstudio-ldap-tooling.googlecode.com/svn/trunk/persistence-tooling/
Re: svn commit: r951512 - /directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/filtering/CursorList.java
On Fri, Jun 4, 2010 at 8:56 PM, fel...@apache.org wrote:
Author: felixk
Date: Fri Jun 4 17:56:57 2010
New Revision: 951512
URL: http://svn.apache.org/viewvc?rev=951512view=rev
Log:
Probably vice versa
Modified:
directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/filtering/CursorList.java
Modified:
directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/filtering/CursorList.java
URL:
http://svn.apache.org/viewvc/directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/filtering/CursorList.java?rev=951512r1=951511r2=951512view=diff
==
---
directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/filtering/CursorList.java
(original)
+++
directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/filtering/CursorList.java
Fri Jun 4 17:56:57 2010
@@ -452,11 +452,11 @@ public class CursorList implements Entry
{
if ( reason != null )
{
- c.close();
+ c.close( reason );
}
else
{
- c.close( reason );
+ c.close();
}
}
catch ( Exception e )
yeap, my fault, thanks for finding and fixing it Felix
Kiran Ayyagari
Re: svn commit: r951513 - /directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultDirectoryService.java
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Can please some cross check this? It doesn't has a testcase to verify it.
Thanks
Felix
On 06/04/10 20:07, fel...@apache.org wrote:
Author: felixk
Date: Fri Jun 4 18:07:07 2010
New Revision: 951513
URL: http://svn.apache.org/viewvc?rev=951513view=rev
Log:
I don't believe that this ever has work that way
Modified:
directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultDirectoryService.java
Modified:
directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultDirectoryService.java
URL:
http://svn.apache.org/viewvc/directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultDirectoryService.java?rev=951513r1=951512r2=951513view=diff
==
---
directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultDirectoryService.java
(original)
+++
directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultDirectoryService.java
Fri Jun 4 18:07:07 2010
@@ -82,6 +82,7 @@ import org.apache.directory.shared.ldap.
import org.apache.directory.shared.ldap.entry.Entry;
import org.apache.directory.shared.ldap.entry.EntryAttribute;
import org.apache.directory.shared.ldap.entry.Modification;
+import org.apache.directory.shared.ldap.entry.Value;
import org.apache.directory.shared.ldap.exception.LdapException;
import org.apache.directory.shared.ldap.exception.LdapNoPermissionException;
import org.apache.directory.shared.ldap.exception.LdapOperationException;
@@ -1343,17 +1344,9 @@ public class DefaultDirectoryService imp
adminDn.normalize( schemaManager.getNormalizerMapping() );
Entry adminEntry = partitionNexus.lookup( new
LookupOperationContext( adminSession, adminDn ) );
-Object userPassword = adminEntry.get(
SchemaConstants.USER_PASSWORD_AT ).get();
+Value? userPassword = adminEntry.get(
SchemaConstants.USER_PASSWORD_AT ).get();
+needToChangeAdminPassword = Arrays.equals(
PartitionNexus.ADMIN_PASSWORD_BYTES, userPassword.getBytes() );
-if ( userPassword instanceof byte[] )
-{
-needToChangeAdminPassword = Arrays.equals(
PartitionNexus.ADMIN_PASSWORD_BYTES, ( byte[] ) userPassword );
-}
-else if ( userPassword.toString().equals(
PartitionNexus.ADMIN_PASSWORD_STRING ) )
-{
-needToChangeAdminPassword =
PartitionNexus.ADMIN_PASSWORD_STRING.equals( userPassword.toString() );
-}
-
if ( needToChangeAdminPassword )
{
LOG.warn( You didn't change the admin password of directory
service + instance ' + instanceId + '.
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkwJQqYACgkQ2lZVCB08qHFwcwCgt857kk5bHlU2kpKLL1e+BhFC
R9wAniBVfsmAhLTbgoVGB/vktEC5Mxjq
=XZBU
-END PGP SIGNATURE-
Re: [ApacheDS] Merge jdbm-store into jdbm-partition
Done. So I'm going to keep the jdbm-partition module. Alex Karasulu wrote: On Fri, Jun 4, 2010 at 1:16 AM, Emmanuel Lecharny elecha...@gmail.com mailto:elecha...@gmail.com wrote: On 6/4/10 12:01 AM, Stefan Seelmann wrote: Emmanuel Lecharny wrote: On 6/3/10 10:51 PM, Stefan Seelmann wrote: Hi dev, another easy refactoring is to merge the modules jdbm-store and jdbm-partiton. +1. Which one will you keep ? jdbm-store ? To me, a partition is associated with a naming context, not with an underlying store. That implies we should get rid of those XXX-partition to just keep xxx-store, and keep the partitions at a upper layer (ie, core) I wanted to keep the jdbm-partition module. To me the partition is the concept that the core knows. The core knows nothing about stores. We also define partitions in the configuration, not stores. This is how I understand the architecture: 1. The core defines the Partition interface +1 Yes. 2. XDBM provides an abstract implemementation of the Partition interface and additionally defines the Store interface and search engine. Yes. Also eventually with global identifiers (UUID) acting as primary keys for the XDBM db scheme we will be able to pull the search functionality out of the partition and place it above the partition nexus. This will make the store interface/concept less important as well. 3. The JDBM partition is a concrete implementation of the XDBM partition. It contains a Store implementation because this is forced by XDBM. Here, I disagree. JDBM is a store, not a partition. XDBM = XXX-Data Base Manager, nothing connected to the idea of Partition. We could probably say that XDBM and Store is the same concept. I think we're overloading too much meaning into Store here. Treat it as a simple interface and forget about trying to draw more meaning out of it such as Database Manager etc. Search happens on this Store which exposes all the methods needed to perform various operations. It's just an interface coalescing all these functions together into a single place so for example we can hand off a store to different parts of the XDBM implementation and have it operate on that object. But let's discuss this aspect further, I may perfectly be wrong, I'm just trying to manipulate concepts here. I think we're trying to draw more meaning from this concept which we do not need to. -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
[ApacheDS] Modules core-constants and core-avl
Hi, two other small modules we have are core-constants and core-avl. I think we can move the 4 relevant files of core-contants to core-api and delete core-constants. For core-avl I'm not sure if we should merge it into another module or keep it as its own module. It is only used by the AVL store which is now part of xdbm-partition, so the AVL classes could be moved there. Another option is to move the AVL classes to core-api. WDYT? Kind Regards, Stefan
Re: [ApacheDS] Modules core-constants and core-avl
On Sat, Jun 5, 2010 at 1:19 AM, Stefan Seelmann seelm...@apache.org wrote: Hi, two other small modules we have are core-constants and core-avl. I think we can move the 4 relevant files of core-contants to core-api and delete core-constants. For core-avl I'm not sure if we should merge it into another module or keep it as its own module. It is only used by the AVL store which is now part of xdbm-partition, so the AVL classes could be moved there. Another option is to move the AVL classes to core-api. I would prefer to keep core-avl separate, just in case if somebody wants to reuse or want to work/tweak this code Kiran Ayyagari
