search

peteroupc / CBOR-Java

A Java implementation of Concise Binary Object Representation (RFC 8949)
The Unlicense
42 stars 10 forks source link

add support of Date type. #14

Closed sbernard31 closed 3 years ago

sbernard31 commented 3 years ago

Maybe I'm wrong but It seems to me currently there is no support for Date type.

Is it something planned to support date like defined in RFC7049§2.4.1 ?

peteroupc commented 3 years ago

The methods CBORObject.FromObject and .ToObject already support converting CBOR objects to and from java.lang.Date.

sbernard31 commented 3 years ago

Good to know. Thx @peteroupc :pray:

sbernard31 commented 3 years ago

(Just an idea, maybe a method asDate() could make this more visible. :thinking:)

sbernard31 commented 3 years ago

I see the default way to encoding date is to use String (tag 0) Any reason why choosing this one instead of the timestamp one (tag 1) which is maybe a bit more straightforward ?

sbernard31 commented 3 years ago

I guess I maybe find a bug (I'm not sure at all) I tried to encode a Date using number instead of string and so I use : 

obj = CBORObject.FromObjectAndTag(date.getTime(), 1);

instead of 

obj = CBORObject.FromObject(date);

Tell me if I'm wrong.

If do that, I face an issue because this can not be decoded using : 

Date date = obj.ToObject(Date.class);

I face : 

com.upokecenter.cbor.CBORException: Not a finite number
    at com.upokecenter.cbor.CBORDateConverter.FromCBORObject(CBORDateConverter.java:33)
    at com.upokecenter.cbor.PropertyMap.TypeToObject(PropertyMap.java:584)
    at com.upokecenter.cbor.CBORObject.ToObject(CBORObject.java:1637)
    at com.upokecenter.cbor.CBORObject.ToObject(CBORObject.java:1301)

I guess maybe the issue is in : 

static boolean IsNumber(CBORObject o) {
      if (IsUntaggedInteger(o)) {
        return true;
      } else if (!o.isTagged() && o.getType() == CBORType.FloatingPoint) {
        return true;
 ... ...

In my case this is a taggedInteger and so isNumber return false. (tested with 4.0.0 and 4.2.0)

peteroupc commented 3 years ago

Yes, this should have been supported better: "If the type is DateTime (or Date in Java) , returns a date/time object if the CBOR object's outermost tag is 0 or 1."

sbernard31 commented 3 years ago

This is pretty much implemented in CBORDateConverter but it seems there is a bug. :grimacing:

Not directly linked to this bug, but my 2 cents about improving Date API. I think maybe adding those method would be great : 

CBORObject.isDate();
CBORObject.asDate();
CBORObject.fromDateAsString(date); 
CBORObject.fromDateAsTimestamp(date);

I think CBORObject.fromObject((Date) date); should maybe encode Date as timestamp by default as I suppose this is easier for a constraint device to deal with a timestamp rather than decode a string Date format.

(Just in case I begin some comparison between Jackson and CBOR-JAVA : https://github.com/eclipse/leshan/issues/939)

peteroupc commented 3 years ago

I have updated this repository to fix the bug in CBORDateConverter.

The suggestion in your last comment might not be implemented until the next major version (which might not happen in a while) for backward compatibility reasons. (To be clear, the next version I expect to release is version 4.3, which is a minor version.)

sbernard31 commented 3 years ago

The suggestion in your last comment might not be implemented until the next major version (which might not happen in a while) for backward compatibility reasons.

No problem, there is no urgency for this. (This is just some improvement ideas)

I have updated this repository to fix the bug in CBORDateConverter.

Thx :pray: for your time !

sbernard31 commented 3 years ago

Should we close this issue and I create a dedicated one for "date API improvement" or we just keep this one open ?

peteroupc commented 3 years ago

Now the CBORDateConverter was made public and expanded.

This issue can now be closed.

sbernard31 commented 3 years ago

I'm not sure if changes proposed at https://github.com/peteroupc/CBOR-Java/issues/14#issuecomment-737114305 was added ? Could you confirm which part of them was added ?

peteroupc commented 3 years ago

This is the new CBORDateConverter: https://github.com/peteroupc/CBOR-Java/blob/master/src/main/java/com/upokecenter/cbor/CBORDateConverter.java . It includes the following new methods:

sbernard31 commented 3 years ago

Ok, this will be available in which version?

peteroupc commented 3 years ago

I expect to include that feature in the next minor version, namely version 4.4.

sbernard31 commented 3 years ago

Ok, thx :pray: