sbcgua / ajson

Yet another json parser serializer for ABAP
MIT License
49 stars 15 forks source link

time conversion not correctly converted from json to abap? #171

Closed oblomov-dev closed 5 months ago

oblomov-dev commented 5 months ago

Hi @sbcgua,

started to play around with ajson, looks very nice, thank you for this project!

Did i find a bug here? Seems that the field with type t is not correctly converted back to the structure.

Best regards.

  TYPES:
      BEGIN OF ty_struc,
        time    TYPE t,
        string  TYPE string,
        true    TYPE abap_bool,
        false   TYPE abap_bool,
        date    TYPE d,
        tstamp  TYPE timestamp,
        tstampl TYPE timestampl,
        i       TYPE i,
        p       TYPE p LENGTH 2 DECIMALS 2,
      END OF ty_struc.

    DATA(ls_test2) = VALUE ty_struc(
        string  = `string`
        true    = abap_true
        false   = abap_false
        date    = sy-datum
        time    = sy-uzeit
        i       = 123
        p       = 1 / 3
    ).

    DATA(ls_test_result) = VALUE ty_struc( ).

    GET TIME STAMP FIELD ls_test2-tstamp.
    GET TIME STAMP FIELD ls_test2-tstampl.

    DATA ajson TYPE REF TO zif_ajson.
    ajson = zcl_ajson=>create_empty( ).

    ajson->set( iv_path = '/' iv_val  = ls_test2 ).
    lv_stringified = ajson->stringify( ).

    DATA ajson_result TYPE REF TO zif_ajson.
    ajson_result = zcl_ajson=>parse( lv_stringified ).

    ajson_result->to_abap(
      IMPORTING
        ev_container     = ls_test_result
    ).

    IF ls_test2 <> ls_test_result.
      ASSERT 1 = 0.
    ENDIF.
image
mbtools commented 5 months ago

looks like a to_time method is missing in this class:

https://github.com/sbcgua/ajson/blob/4c0e2f26f2dd33d0c338e8e5ff7c7a96d2febb69/src/core/zcl_ajson.clas.locals_imp.abap#L634

also unit tests cover only "date" and "timestamp" but not "time"

so there's room for improvement 😉

oblomov-dev commented 5 months ago

thank your for the info, i give it a try: https://github.com/sbcgua/ajson/pull/172

sbcgua commented 5 months ago

Thank you both. A good catch. I added a couple of comments to the PR, and will merge it as soon as it is fixed. And thank you for the fix as well of course.

sbcgua commented 5 months ago

Can be closed?

oblomov-dev commented 5 months ago

yes thanks for merging!