SchwarzIT / sap-usi-logging-api

An easy-to-use, object-oriented encapsulation around the SAP application log (Transaction SLG1)
Apache License 2.0
24 stars 0 forks source link

Structure-Container /USI/CL_BAL_DC_STRUCTURE: Potential Loss of Data for deep structures #19

Closed NeumannJoerg closed 7 months ago

NeumannJoerg commented 8 months ago

The data of deep structures might be lost, in some cases. The following test report reproduces the issue.

TYPES: BEGIN OF ty_deep_structure,
         field_01 TYPE char10,
         field_02 TYPE string,
       END   OF  ty_deep_structure.

DATA(logger) = /usi/cl_bal_factory=>get_instance( )->create_new_logger( i_log_object = 'Z_MY_LOG_OBJECT'
                                                                        i_sub_object = 'Z_MY_SUB_OBJECT' ).
DATA(token) = logger->claim_ownership( ).

DATA(structure_container) = NEW /usi/cl_bal_dc_structure(
                                    i_structure = VALUE ty_deep_structure( field_01 = 'THIS DATA'
                                                                           field_02 = 'WILL BE LOST!' ) ).

logger->add_free_text( i_message_type = /usi/cl_bal_enum_message_type=>information
                       i_free_text    = 'Data of structure will be lost'
                       i_details      = structure_container ).

logger->save( token ).
logger->free( token ).

The debugger reveals, that the data is temporarily available inside the constructor. image

Right after leaving the constructor, the data reference inside the data container will still be bound, but the dereferrenced structure will be initial! image

The constructor of the class should be adjusted to avoid unexpected loss of data. Replacing this

structure = REF #( i_structure ).

with that

CREATE DATA structure LIKE i_structure.
ASSIGN structure->* TO FIELD-SYMBOL(<structure>).
<structure> = i_structure.

fixes the issue.

This might not be ideal in terms of performance, so this might be changed again in the future, but for now I'd rather accept an insignificant hit to performance than an unexpected loss of data.

NeumannJoerg commented 7 months ago

Adjusted the constructor. Data is now cloned into a newly created data reference, which avoids the issue.

The new test method lcl_unit_tests_serialization->test_keeps_deep_struc_values will prevent a regression in subsequent releases.