Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support of Date type. #14

Closed
sbernard31 opened this issue Nov 26, 2020 · 16 comments
Closed

add support of Date type. #14

sbernard31 opened this issue Nov 26, 2020 · 16 comments

Comments

@sbernard31
Copy link

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
Copy link
Owner

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

@sbernard31
Copy link
Author

Good to know. Thx @peteroupc 🙏

@sbernard31
Copy link
Author

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

@sbernard31
Copy link
Author

sbernard31 commented Nov 27, 2020

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
Copy link
Author

sbernard31 commented Dec 1, 2020

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
Copy link
Owner

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."

@peteroupc peteroupc reopened this Dec 1, 2020
@sbernard31
Copy link
Author

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

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 : eclipse-leshan/leshan#939)

@peteroupc
Copy link
Owner

peteroupc commented Dec 4, 2020

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
Copy link
Author

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 🙏 for your time !

@sbernard31
Copy link
Author

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

@peteroupc
Copy link
Owner

Now the CBORDateConverter was made public and expanded.

This issue can now be closed.

@sbernard31
Copy link
Author

I'm not sure if changes proposed at #14 (comment) was added ?
Could you confirm which part of them was added ?

@peteroupc
Copy link
Owner

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:

  • TryGetDateTimeFields
  • DateTimeFieldsToCBORObject
  • Support for tag 0 and tag 1 date/times as well as untagged date/times.

@sbernard31
Copy link
Author

Ok, this will be available in which version?

@peteroupc
Copy link
Owner

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

@sbernard31
Copy link
Author

Ok, thx 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants