sbcgua / ajson

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

set_string with custom_mapping does rename field #110

Closed albertmink closed 2 years ago

albertmink commented 2 years ago

Intention is to override JSON data. We read the JSON data, then calculate a new value which then shall be stored in the JSON data.

When setting a string value, its fieldname is changed. Here from fieldData to fielddata. Is this a bug?

DATA ajson   TYPE REF TO zcl_ajson.
DATA path TYPE string.

ajson = zcl_ajson=>parse( iv_json = `{"fieldData":"field_value"}`
                          ii_custom_mapping = zcl_ajson_mapping=>create_camel_case( iv_first_json_upper = abap_false ) ).

" /fieldata = E
ajson->set_string( iv_path = `/fieldData`  iv_val = 'E' ).

Without custom_mapping the field name stays the same.

sbcgua commented 2 years ago

hmm, interesting case, and a bit controversial.

What behavior would you expect ? (in particular in the step 1) ? @jrodriguez-rc What do you think ?

P.S. There is a bug though. If you add ajson->set_string( iv_path = '/fielddata' iv_val = '...' ). (lowercased field name) - it will dump. I think due to the step 1 issue.

albertmink commented 2 years ago

I'm not sure, whether I understand all technical aspects properly. But I noticed, that I didn't provided the entire use case. The to_abap part was missing, see below.

Challenge: The JSON data encodes language in ISO 639 and the ABAP logic is based on the char[1] type. I would be a natural choice, if the AJSON could do the conversion. Otherwise, we need to find a workaround in ABAP coding.

So, I expected (overall) that the set_string sets the value and does NOT modify the field name.

DATA:
  ajson TYPE REF TO zcl_ajson,

  BEGIN OF ls_result,
    original_language TYPE c LENGTH 1,
  END OF ls_result.

ajson = zcl_ajson=>parse( iv_json = `{"originalLanguage":"en"}`  " language code encoded  ISO 639
                          ii_custom_mapping = zcl_ajson_mapping=>create_camel_case( ) ).

*ajson->set_string( iv_path = `/originalLanguage`  iv_val = 'E' ).

ajson->to_abap( IMPORTING ev_container = ls_result ).
jrodriguez-rc commented 2 years ago

It seems that the "SET" method always calls to mapper

image

The SET is calling to the "to_json" transformer, so the attribute is converted to lowercase and there is no underscore.

With the first code, if I try to extract the content of the json to an ABAP structure with the field "field_data" there is a "Path not found" exception because it is trying to find the field "fielddata".

it should be:

This way, if you convert JSON to JSON or ABAP to ABAP the format doesn't change.

But now, when calls to the "SET" method, even if you add a value to a node, it is converted "to_json" because the local class "lcl_abap_to_json" is called.

This needs to be carefully analyzed, to avoid calling the mapping when it doesn't need to call

sbcgua commented 2 years ago

Yeah, the issue appears quite tricky.

@albertmink Just for the sake of search of quick workaround:

@jrodriguez-rc do you mean that the conversion should happen only on in/out ? Like parsing/serialization ? And the set should keep the name as is ? This makes sense to me. And also makes the whole thing more maintainable and predictable. Also aligns well with my previous (1) point about "parsing step"

albertmink commented 2 years ago

@sbcgua our boundary conditions are: JSON data is ISO en and ABAP logic is based on E . So we definitely need to transform the language field (separately from JSON) and we are fine with a converter.

The good thing, we could use an already existing class, that does the transformation. However, we would like to set the transformed value, where we encountered the reported issue.

jrodriguez-rc commented 2 years ago

About the language conversion, maybe could do it about the ABAP field type, like timestamp