You are viewing a plain text version of this content. The canonical link for it is here.
Posted to api@directory.apache.org by Radovan Semancik <ra...@evolveum.com> on 2015/03/04 17:17:35 UTC

isHumanReadable in LdapSyntax

Hi,

The current code of class LdapSyntax, method isHumanReadable seems to 
return false by default (line 145).  However the comment indicates that 
it should default to string which is human readable. So the line should 
be probably "return true" and not "return false". This seemed to me as 
an obvious bug.

But when I've changed it to "return true" then the 
SchemaObjectRendererTest fails ... in a quite strange way (NPE). I do 
not feel to be very strong here and I'm not really sure what I'm doing. 
Could anyone please have a look and help me out?

Thanks.

-- 
Radovan Semancik
Software Architect
evolveum.com


Re: isHumanReadable in LdapSyntax

Posted by Radovan Semancik <ra...@evolveum.com>.
Hi,

Yes, of course. That was exactly my goal.

... and I thank you!

-- 

                                            Radovan Semancik
                                           Software Architect
                                              evolveum.com



On 03/08/2015 06:17 PM, Emmanuel Lécharny wrote:
> Radovan,
>
> just for the sake of clarity : the code you put on github was meant to
> be injected into the Apache LDAP API, at some pointj, right ?
>
> or to be more explicit, now that I have taken it, updated it and made it
> a part of the API, do you agree that it was your intended move ?
>
> Many thanks !



Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Radovan,

just for the sake of clarity : the code you put on github was meant to
be injected into the Apache LDAP API, at some pointj, right ?

or to be more explicit, now that I have taken it, updated it and made it
a part of the API, do you agree that it was your intended move ?

Many thanks !

Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Okie, a complete VLV implementation has been pushed into trunk.

I still have to add the junit tests for it.

Many thanks for your contribution, Radovan !

Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 06/03/15 07:50, Radovan Semancik a écrit :
> Hi,
>
> Thanks for comments. I'll have a look at the  *Tags enums. But I guess
> it will be a bit inconvenient for me to work on my github fork and you
> working on svn trunk ...
>
> I would suggest this: let me finish my rudimentary implementation. 

I'm already adding the missing parts, and it would not take long to be
done. I'm quite sure it will be completed before the end of this
week-end, based on all the work you have already done, it's going fast
(although I have to do that on my spare time atm).

> Now I have to speed things up a bit, so it probably won't take longer
> than a week or two. I'm using VLV and schema in my LDAP connector for
> midPoint. I'm also working on automated tests that will use
> midPoint+connector+directory API with OpenLDAP and 389ds (I already
> have tests for OpenDJ). Once I have that then I will create a pull
> request so it can get to the directory API project trunk. The code
> will probably be a bit ugly ... but once it is in trunk you are free
> to clean it up. And as I will have the tests I can check that it still
> works after your cleanup ...
>
> I was thinking about how to make unit tests for this directly in the
> directory API project. Simple tests for VLV controls parse and encode
> will be probably a good idea. But this is not really enough to make
> sure that the whole thing really works. Do you have any kind of test
> suite for directory API that is using real LDAP servers?

We do test the API using ApacheDS, which does not currently support VLV
itself, so I guess the best for us is to implement VLV in ApacheDS ;-)

Regardless, what we do usually is that we add unit tests for the encoder
and decoder, in a way we cover all the possible use cases - which is
quite painfull, as we may have many possibilities -. For the VLV codec,
it's not that hard.


Re: isHumanReadable in LdapSyntax

Posted by Radovan Semancik <ra...@evolveum.com>.
Hi,

Thanks for comments. I'll have a look at the  *Tags enums. But I guess 
it will be a bit inconvenient for me to work on my github fork and you 
working on svn trunk ...

I would suggest this: let me finish my rudimentary implementation. Now I 
have to speed things up a bit, so it probably won't take longer than a 
week or two. I'm using VLV and schema in my LDAP connector for midPoint. 
I'm also working on automated tests that will use 
midPoint+connector+directory API with OpenLDAP and 389ds (I already have 
tests for OpenDJ). Once I have that then I will create a pull request so 
it can get to the directory API project trunk. The code will probably be 
a bit ugly ... but once it is in trunk you are free to clean it up. And 
as I will have the tests I can check that it still works after your 
cleanup ...

I was thinking about how to make unit tests for this directly in the 
directory API project. Simple tests for VLV controls parse and encode 
will be probably a good idea. But this is not really enough to make sure 
that the whole thing really works. Do you have any kind of test suite 
for directory API that is using real LDAP servers?

-- 

                                            Radovan Semancik
                                           Software Architect
                                              evolveum.com



On 03/05/2015 07:08 PM, Emmanuel Lécharny wrote:
> We declare the tags in a XXXTags file, like in :
>
> public enum PasswordPolicyTags
> {
>      PPOLICY_WARNING_TAG(0xA0), // warning [0]
>      PPOLICY_ERROR_TAG(0x81), // error [1]
>      TIME_BEFORE_EXPIRATION_TAG(0x80), // timeBeforeExpiration [0]
>      GRACE_AUTHNS_REMAINING_TAG(0x81); // graceAuthNsRemaining [1]
>      ...
>
> One thing that is missing is the choice between byteOffset and an
> assertionValue : we should have a flag in the impl to determinate which
> of the two is in use.
>
> the contextID might be empty, and might be absent from the PDU. Those
> are 2 different use cases. We should cover both.
>
>
> The VLV control will probably be put into the codec-extras module.
>
> I'll check that tonite !
>
>
>



Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 05/03/15 18:39, Radovan Semancik a écrit :
> On 03/05/2015 06:18 PM, Emmanuel Lécharny wrote:
>> Woot ! You went through the intricacy of teh ASN/1 encoder/decoder on
>> your own ! Taht's impressive ! L'll have a look and probably add it
>> into the API. 
>
> Thanks. But never underestimate the power of copy&paste :-)
>
> It is not hard to figure it out. I've already implemented VLV controls
> for JNDI before. And other controls implemented in the directory API
> are quite a good examples to get started. None of them used the ASN.1
> choice, though, so I had no place to copy this piece from. I've used a
> literal (0xa0), but I assume that there is a nicer way to do that. I
> mean the encoding of this part of VLV request control:
>
>           target       CHOICE { ...
>
>
We declare the tags in a XXXTags file, like in :

public enum PasswordPolicyTags
{
    PPOLICY_WARNING_TAG(0xA0), // warning [0]
    PPOLICY_ERROR_TAG(0x81), // error [1]
    TIME_BEFORE_EXPIRATION_TAG(0x80), // timeBeforeExpiration [0]
    GRACE_AUTHNS_REMAINING_TAG(0x81); // graceAuthNsRemaining [1]
    ...

One thing that is missing is the choice between byteOffset and an
assertionValue : we should have a flag in the impl to determinate which
of the two is in use.

the contextID might be empty, and might be absent from the PDU. Those
are 2 different use cases. We should cover both.


The VLV control will probably be put into the codec-extras module.

I'll check that tonite !




Re: isHumanReadable in LdapSyntax

Posted by Radovan Semancik <ra...@evolveum.com>.
On 03/05/2015 06:18 PM, Emmanuel Lécharny wrote:
> Woot ! You went through the intricacy of teh ASN/1 encoder/decoder on 
> your own ! Taht's impressive ! L'll have a look and probably add it 
> into the API. 

Thanks. But never underestimate the power of copy&paste :-)

It is not hard to figure it out. I've already implemented VLV controls 
for JNDI before. And other controls implemented in the directory API are 
quite a good examples to get started. None of them used the ASN.1 
choice, though, so I had no place to copy this piece from. I've used a 
literal (0xa0), but I assume that there is a nicer way to do that. I 
mean the encoding of this part of VLV request control:

           target       CHOICE { ...


-- 

                                            Radovan Semancik
                                           Software Architect
                                              evolveum.com


Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 05/03/15 17:43, Radovan Semancik a écrit :
> On 03/05/2015 05:16 PM, Emmanuel Lécharny wrote:
>> Le 05/03/15 16:32, Radovan Semancik a écrit :
>>> On 03/05/2015 02:50 PM, Emmanuel Lécharny wrote:
>>>> Ok, I created a JIRA for this one :
>>>> https://issues.apache.org/jira/browse/DIRAPI-223
>>>>
>>>> And I fixed it too.
>>>>
>>>> It should now work as expected.
>>> It looks like it really does work. Thanks a lot!
>>>
>>> BTW, I have notices that there is no support for VirtualListView (VLV)
>>> request and response controls in the API. Therefore I'm implementing
>>> it now.
>> That's right, we don't support that, I guess we started it a long time
>> ago. I can lend some hands here (specifically for the encoded/decoder).
>>
>> Just let me know.
>
> I think I have figured out the basics. But it might require some
> review/cleanup. E.g. I haven't found constant or any other nice way
> for ASN.1 choice encoding (but I haven't looked that hard).
>
> The code is available here:
> https://github.com/Evolveum/directory-shared
> ... but it is still work in progress. Only VLV request is done yet,
> and it only has "by offset" option. But it seems to work for me.
> Tested with OpenDJ. Testing with OpenLDAP and 389ds will come later.
>
Woot ! You went through the intricacy of teh ASN/1 encoder/decoder on
your own ! Taht's impressive !

L'll have a look  and probably add it into the API.

Re: isHumanReadable in LdapSyntax

Posted by Radovan Semancik <ra...@evolveum.com>.
On 03/05/2015 05:16 PM, Emmanuel Lécharny wrote:
> Le 05/03/15 16:32, Radovan Semancik a écrit :
>> On 03/05/2015 02:50 PM, Emmanuel Lécharny wrote:
>>> Ok, I created a JIRA for this one :
>>> https://issues.apache.org/jira/browse/DIRAPI-223
>>>
>>> And I fixed it too.
>>>
>>> It should now work as expected.
>> It looks like it really does work. Thanks a lot!
>>
>> BTW, I have notices that there is no support for VirtualListView (VLV)
>> request and response controls in the API. Therefore I'm implementing
>> it now.
> That's right, we don't support that, I guess we started it a long time
> ago. I can lend some hands here (specifically for the encoded/decoder).
>
> Just let me know.

I think I have figured out the basics. But it might require some 
review/cleanup. E.g. I haven't found constant or any other nice way for 
ASN.1 choice encoding (but I haven't looked that hard).

The code is available here:
https://github.com/Evolveum/directory-shared
... but it is still work in progress. Only VLV request is done yet, and 
it only has "by offset" option. But it seems to work for me. Tested with 
OpenDJ. Testing with OpenLDAP and 389ds will come later.

-- 
Radovan Semancik
Software Architect
evolveum.com


Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 05/03/15 16:32, Radovan Semancik a écrit :
> On 03/05/2015 02:50 PM, Emmanuel Lécharny wrote:
>> Ok, I created a JIRA for this one :
>> https://issues.apache.org/jira/browse/DIRAPI-223
>>
>> And I fixed it too.
>>
>> It should now work as expected.
>
> It looks like it really does work. Thanks a lot!
>
> BTW, I have notices that there is no support for VirtualListView (VLV)
> request and response controls in the API. Therefore I'm implementing
> it now.

That's right, we don't support that, I guess we started it a long time
ago. I can lend some hands here (specifically for the encoded/decoder).

Just let me know.


Re: isHumanReadable in LdapSyntax

Posted by Radovan Semancik <ra...@evolveum.com>.
On 03/05/2015 02:50 PM, Emmanuel Lécharny wrote:
> Ok, I created a JIRA for this one :
> https://issues.apache.org/jira/browse/DIRAPI-223
>
> And I fixed it too.
>
> It should now work as expected.

It looks like it really does work. Thanks a lot!

BTW, I have notices that there is no support for VirtualListView (VLV) 
request and response controls in the API. Therefore I'm implementing it now.

-- 
Radovan Semancik
Software Architect
evolveum.com


Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 04/03/15 17:17, Radovan Semancik a écrit :
> Hi,
>
> The current code of class LdapSyntax, method isHumanReadable seems to
> return false by default (line 145).  However the comment indicates
> that it should default to string which is human readable. So the line
> should be probably "return true" and not "return false". This seemed
> to me as an obvious bug.
>
> But when I've changed it to "return true" then the
> SchemaObjectRendererTest fails ... in a quite strange way (NPE). I do
> not feel to be very strong here and I'm not really sure what I'm
> doing. Could anyone please have a look and help me out?
>
> Thanks.
>
Ok, I created a JIRA for this one :
https://issues.apache.org/jira/browse/DIRAPI-223

And I fixed it too.

It should now work as expected.



Re: isHumanReadable in LdapSyntax

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 04/03/15 17:17, Radovan Semancik a écrit :
> Hi,
>
> The current code of class LdapSyntax, method isHumanReadable seems to
> return false by default (line 145).  However the comment indicates
> that it should default to string which is human readable. So the line
> should be probably "return true" and not "return false". This seemed
> to me as an obvious bug.

Indeed.

It's even worse : we check that the extensions contain the
MetaSchemaConstants.X_NOT_HUMAN_READABLE_AT value, which is
"x-not-humane-readable" (lower case) when the flag is itself uper case :/

We have to make this part case-insensitive ...
>
> But when I've changed it to "return true" then the
> SchemaObjectRendererTest fails ... in a quite strange way (NPE). 

Hmmm, not for me.

> I do not feel to be very strong here and I'm not really sure what I'm
> doing. Could anyone please have a look and help me out?

Sure, I'm having a look at this mess atm.