Clean render()/check() NGSIv1 method from indent argument passing #2760

fgalan · 2016-12-09T18:16:39Z

The removal work has been already done at branch haderding/remove_ngsiv1_indent (note the typo 'haderding' in the branch name).

However, in order to have a "safe measure" in the case that some API client in IoTP were (wrongly) processing the result in text-positional way (instead of the right way in JSON way) we should implement a formatting step just before response NGSIv1 payloads in order to keep 100% backward compatibility (the existing .test and unit test will guarantee that). This formatting step should be activable by configuration (e.g. -legacyNgsiv1Indent) and it is thought as a provisional measure to be removed (including the CLI) once we check that no IoTP case breaks.

The builtin PrettyWriter from rapidjson could do the trick (see

fiware-orion/src/lib/rest/restReply.cpp

Line 70 in 43e96f9

#if 0

). However, it seems that we are using an extra whitespace before the : (different to the usual way in any JSON pretty printer out there) so probably we would need to re-implement a special pretty printer ourselves :(

The text was updated successfully, but these errors were encountered:

fgalan · 2017-01-10T15:29:25Z

We are now in an "almost" finished status with regards to this change, and the situation is as follows:

Branch haderding/remove_ngsiv1_indent implements a refactor in render()/check() code to remove all the whitespace and all the newlines in all the NGSIv1 renders.

In addition, the -paranoidV1Indent CLI parameter can be used to get the exact NGSIv1 renderings that we had before haderding/remove_ngsiv1_indent (at least as far we have checked with existing .test suite) except in the following cases:

Case A: GET /v1/contextEntities/<type> response is correctly indented (before haderding/remove_ngsiv1_indent some (see name and attributes) elements were incorrently over-indent by 2 spaces). Example on how it was before haderding/remove_ngsiv1_indent:

{
    "name" : "Thing",
    "attributes" : [
      "A1"
    ],
  "statusCode" : {
    "code" : "200",
    "reasonPhrase" : "OK"
  }

Case B: correct compound metadata values rendered. Previously it was without indent, printing the "value" plus the value in one line, with trailing } in the same line. Example on how it was before haderding/remove_ngsiv1_indent:

{
  "contextResponses" : [
    {
      "contextElement" : {
        ...
        "attributes" : [
          {
            "name" : "A1",
            "type" : "Number",
            "value" : 1,
            "metadatas" : [
              {
                "name" : "M1",
                "type" : "StructuredValue",
"value": [1,2,3]              },
              {
                "name" : "M2",
                "type" : "StructuredValue",
"value":{"x":1,"y":2}              },
              {
                "name" : "M3",
                "type" : "Text",
                "value" : "simple"
              }
            ]
          }
        ]
      },
      ...
      }
    }
  ]
}

Case C: correct forward updates with compound values for attributes. Previously (at least for the object case) were printed without indent, in a single line, with trailing } in the same line. Note this is not the same as case B, due to "value" is not printed in the same line. Example on how it was before haderding/remove_ngsiv1_indent:


{
  "contextElements" : [
    {
      "type" : "T1",
      "isPattern" : "false",
      "id" : "E1",
      "attributes" : [
        {
          "name" : "A1",
          "type" : "",
          "value" : {
"x":2,"y":true,"z":null,"x2":1.5,"str":"This is a string"          }
        }
      ]
    }
  ],
  "updateAction" : "UPDATE"
}

Case D: correct no-string no-compound metadata value. Previously it was correctly indented, but with trailing } in the same line. Example on how it was before haderding/remove_ngsiv1_indent:

{
  "subscriptionId" : "586cc5cdae60ba468cbf1309",
  "originator" : "localhost",
  "contextResponses" : [
    {
      "contextElement" : {
        "type" : "T",
        "isPattern" : "false",
        "id" : "E1",
        "attributes" : [
          {
            "name" : "A",
            "type" : "ComplexVector",
            "value" : [
              1,
              {
                "b" : "foo"
              }
            ],
            "metadatas" : [
              {
                "name" : "previousValue",
                "type" : "Number",
                "value": 1              }
            ]
          }
        ]
      },
      "statusCode" : {
        "code" : "200",
        "reasonPhrase" : "OK"
      }
    }
  ]
}

The harnessFunctions.sh localBrokerStart() function has been adjusted in haderding/remove_ngsiv1_indent in order to use -paranoidV1Indent. Thus, these are the only changed in .test files, due to the case A to D above:

0000_log_operation/log_rest.test: In this case, changes are not meaninfull, as this .test covers an administrative operation, neither "popular" nor used by external clients.
0117_convop_using_standard_ops/getAttributesForEntityType.test: fails in steps 1, 3, 5, 6 and 7, all them due to case A.
0519_ngsi10_entity_types_listing/contextEntityTypeAttributes.test: fails in step 1 and 2, both due to case A.
0676_servicepath_context_types/servicepath_specific_sametype.test: fails in steps 2, 3, 4, 5, 6 and 7 all them due to case A.
1068_metadata_value_as_compound/vector_compound_in_metadata_for_api_v1.test: fail in step 2, due to case B.
1156_mq_as_uri_param_for_metadata/mq_as_scope_for_compound_values_in_v1_queries.test: fail in step 5, due to case B.
1156_mq_as_uri_param_for_metadata/mq_as_scope_in_v1_queries.test: fail in step 5, due to case D.
2248_forwarding_updates_overwrite_the_types_of_attributes/attribute_type_converted_to_string_on_forward.test: fail in step 3 due to case C
2339_json_native_types_in_v1/json_native_types_in_v1.test: fails in step 2 and 4, due to case D.
2507_csub_metadata_field/previous_value_with_compounds_ngsiv1.test: fail in step 5 due to case D; fails in step 7 and 9 due to case B.
2507_csub_metadata_field/special_metadata_in_notifications_ngsiv1.test: fails in steps 5, 7, 9 and 11 due to case D.

In all cases above, note that the only changes are in the Content-Length lines. This means that the JSON content has not actually changed in any case, only the "whitespace sugar" in the responses.

EDIT: the compare branch also shows changes in the 2846_empty_maps_in_metrics/empty_maps_in_metrics.test test. This is probably due to changes in the content length of the above test are causing slight changes in the byte count statistics.

fgalan · 2017-01-11T08:51:42Z

Ready to merge this work into master but, due to planning reasons, this will be deferred some sprints away in the future. Milestone changed.

fgalan · 2017-07-25T15:01:54Z

Quick link to the branch holding the changes related with this issue (useful while the corresponding PR is not yet cast):

https://github.com/telefonicaid/fiware-orion/compare/haderding/remove_ngsiv1_indent

fgalan · 2017-10-17T15:41:36Z

The following 119 unit test are failing. They will be DISABLED.

[  FAILED  ] 119 tests, listed below:
[  FAILED  ] putIndividualContextEntityAttribute.json
[  FAILED  ] Convenience.shortPath
[  FAILED  ] Convenience.badPathNgsi9
[  FAILED  ] Convenience.badPathNgsi10
[  FAILED  ] AppendContextElementRequest.render_json
[  FAILED  ] AppendContextElementRequest.check_json
[  FAILED  ] AppendContextElementResponse.render_json
[  FAILED  ] AppendContextElementResponse.check_json
[  FAILED  ] ContextAttributeResponse.check_json
[  FAILED  ] ContextAttributeResponseVector.render_json
[  FAILED  ] ContextAttributeResponseVector.check_json
[  FAILED  ] UpdateContextAttributeRequest.render_json
[  FAILED  ] UpdateContextAttributeRequest.check_json
[  FAILED  ] UpdateContextElementRequest.render_json
[  FAILED  ] UpdateContextElementRequest.check_json
[  FAILED  ] UpdateContextElementResponse.render_json
[  FAILED  ] UpdateContextElementResponse.check_json
[  FAILED  ] RegisterProviderRequest.json_ok
[  FAILED  ] RegisterContextRequest.json_ok
[  FAILED  ] RegisterContextRequest.json_noContextRegistration
[  FAILED  ] RegisterContextRequest.json_noProvidingApplication
[  FAILED  ] RegisterContextRequest.json_emptyProvidingApplication
[  FAILED  ] RegisterContextRequest.json_entityIdWithIsPatternTrue
[  FAILED  ] RegisterContextRequest.json_badContextRegistrationAttributeIsDomain
[  FAILED  ] RegisterContextResponse.jsonRender
[  FAILED  ] DiscoverContextAvailabilityRequest.noEntityIdList_json
[  FAILED  ] DiscoverContextAvailabilityRequest.emptyEntityIdList_json
[  FAILED  ] DiscoverContextAvailabilityRequest.invalidIsPatternValue_json
[  FAILED  ] DiscoverContextAvailabilityRequest.unsupportedAttributeForEntityId_json
[  FAILED  ] DiscoverContextAvailabilityRequest.overrideEntityIdIsPattern_json
[  FAILED  ] DiscoverContextAvailabilityRequest.emptyEntityIdId_json
[  FAILED  ] DiscoverContextAvailabilityRequest.noEntityIdId_json
[  FAILED  ] DiscoverContextAvailabilityRequest.noScopeType_json
[  FAILED  ] DiscoverContextAvailabilityRequest.noScopeValue_json
[  FAILED  ] DiscoverContextAvailabilityRequest.emptyScopeType_json
[  FAILED  ] DiscoverContextAvailabilityRequest.emptyScopeValue_json
[  FAILED  ] DiscoverContextAvailabilityRequest.emptyAttributeName_json
[  FAILED  ] DiscoverContextAvailabilityResponse.jsonRender
[  FAILED  ] SubscribeContextAvailabilityRequest.json_noEntityId
[  FAILED  ] SubscribeContextAvailabilityRequest.json_badDuration
[  FAILED  ] SubscribeContextAvailabilityResponse.jsonRender
[  FAILED  ] NotifyContextAvailabilityRequest.ok_json
[  FAILED  ] NotifyContextAvailabilityRequest.json_render
[  FAILED  ] UnsubscribeContextAvailabilityRequest.badSubscriptionId_json
[  FAILED  ] UnsubscribeContextAvailabilityResponse.jsonRender
[  FAILED  ] UpdateContextAvailabilitySubscriptionRequest.json_ok
[  FAILED  ] UpdateContextAvailabilitySubscriptionRequest.json_invalidIsPattern
[  FAILED  ] UpdateContextAvailabilitySubscriptionResponse.jsonRender
[  FAILED  ] QueryContextRequest.ok_json
[  FAILED  ] QueryContextRequest.badIsPattern_json
[  FAILED  ] QueryContextRequest.emptyAttribute_json
[  FAILED  ] QueryContextRequest.emptyAttributeExpression_json
[  FAILED  ] QueryContextRequest.scopeGeolocationCircleInvertedBadValueJson
[  FAILED  ] QueryContextRequest.scopeGeolocationCircleZeroRadiusJson
[  FAILED  ] QueryContextRequest.scopeGeolocationPolygonInvertedBadValueJson
[  FAILED  ] QueryContextRequest.scopeGeolocationPolygonNoVerticesJson
[  FAILED  ] QueryContextRequest.scopeGeolocationPolygonOneVertexJson
[  FAILED  ] QueryContextRequest.scopeGeolocationPolygonTwoVerticesJson
[  FAILED  ] QueryContextResponse.json_render
[  FAILED  ] NotifyContextRequest.json_ok
[  FAILED  ] NotifyContextRequest.json_badIsPattern
[  FAILED  ] NotifyContextRequest.json_render
[  FAILED  ] SubscribeContextRequest.badIsPattern_json
[  FAILED  ] SubscribeContextRequest.invalidDuration_json
[  FAILED  ] SubscribeContextRequest.scopeGeolocationCircleInvertedBadValueJson
[  FAILED  ] SubscribeContextRequest.scopeGeolocationCircleZeroRadiusJson
[  FAILED  ] SubscribeContextRequest.scopeGeolocationPolygonInvertedBadValueJson
[  FAILED  ] SubscribeContextRequest.scopeGeolocationPolygonNoVerticesJson
[  FAILED  ] SubscribeContextRequest.scopeGeolocationPolygonOneVertexJson
[  FAILED  ] SubscribeContextRequest.scopeGeolocationPolygonTwoVerticesJson
[  FAILED  ] SubscribeContextResponse.json_render
[  FAILED  ] UnsubscribeContextRequest.badSubscriptionId_json
[  FAILED  ] UnsubscribeContextResponse.jsonRender
[  FAILED  ] UpdateContextRequest.badIsPattern_json
[  FAILED  ] UpdateContextResponse.jsonRender
[  FAILED  ] UpdateContextSubscriptionRequest.badLength_json
[  FAILED  ] UpdateContextSubscriptionRequest.invalidDuration_json
[  FAILED  ] UpdateContextSubscriptionRequest.scopeGeolocationCircleInvertedBadValueJson
[  FAILED  ] UpdateContextSubscriptionRequest.scopeGeolocationCircleZeroRadiusJson
[  FAILED  ] UpdateContextSubscriptionRequest.scopeGeolocationPolygonInvertedBadValueJson
[  FAILED  ] UpdateContextSubscriptionRequest.scopeGeolocationPolygonNoVerticesJson
[  FAILED  ] UpdateContextSubscriptionRequest.scopeGeolocationPolygonOneVertexJson
[  FAILED  ] UpdateContextSubscriptionRequest.scopeGeolocationPolygonTwoVerticesJson
[  FAILED  ] UpdateContextSubscriptionResponse.json_render
[  FAILED  ] ContextAttribute.render
[  FAILED  ] ContextElement.check
[  FAILED  ] NotifyCondition.render
[  FAILED  ] AttributeDomainName.ok
[  FAILED  ] AttributeList.ok
[  FAILED  ] ConditionValueList.ok
[  FAILED  ] ContextElementResponse.render
[  FAILED  ] ContextRegistrationAttribute.render
[  FAILED  ] ContextRegistrationAttributeVector.render
[  FAILED  ] ProvidingApplication.render
[  FAILED  ] AttributeExpression.ok
[  FAILED  ] ContextRegistrationResponse.render
[  FAILED  ] EntityId.render
[  FAILED  ] Metadata.render
[  FAILED  ] MetadataVector.render
[  FAILED  ] NotifyConditionVector.render
[  FAILED  ] Originator.render
[  FAILED  ] Reference.render
[  FAILED  ] RestrictionString.render
[  FAILED  ] Scope.render
[  FAILED  ] StatusCode.render
[  FAILED  ] SubscribeError.render
[  FAILED  ] SubscriptionId.render
[  FAILED  ] Throttling.render
[  FAILED  ] UpdateActionType.render
[  FAILED  ] CompoundValueNode.vectorInvalidAndOk
[  FAILED  ] CompoundValueNode.structInvalidAndOk
[  FAILED  ] compoundValue.updateUnknownPath
[  FAILED  ] compoundValue.updateTwoItemsSameNameInStructJson
[  FAILED  ] compoundValue.updateTwoStructsJson
[  FAILED  ] compoundValue.sixLevelsJson
[  FAILED  ] jsonRequest.jsonTreat
[  FAILED  ] OrionError.all
[  FAILED  ] RestService.noSuchServiceAndNotFound
[  FAILED  ] rest.servicePathSplit

There isn't any "core" unit test in that list, but anyway they should be fixed eventually (this issue will remain opened until we find a solution for this).

fgalan · 2017-10-17T18:35:30Z

Above 119 disabled tests in commit c689a24. Note the commit has +/- 119 lines changed, as expected.

I'm writting done in this issue as it can be useful to reverse it in the future to re-enable again.

fgalan · 2017-10-17T18:59:01Z

Implemented in PR #3020

However, this is not the end of the story for this issue :). Pending after the above PR merge:

After a quarentine period, remove the PARANOID_JSON_INDENT stuff. This will cause unalignment of all NGSIv1 .test files; fix them using fixContentLengths.py
Fix the 119 disabled test, either reovering it or removing it if no longer needed.

fgalan · 2017-10-19T08:20:29Z

Most of the work consolidated in 1.9.0. Post-implementation tasks moved forward a couple of versions, to milestone 1.11.0.

fgalan · 2018-01-22T10:08:29Z

PR #3083 does:

After a quarentine period, remove the PARANOID_JSON_INDENT stuff. This will cause unalignment of all NGSIv1 .test files; fix them using fixContentLengths.py

Fix the disabled unit tests is pending.

fgalan · 2018-01-25T09:21:10Z

PR #3086 fix the pending disabled unit tests.

This was the last item of work in this issue, so it can be closed.

fgalan added should techdebt labels Dec 9, 2016

fgalan added this to the Refactors milestone Dec 9, 2016

fgalan modified the milestones: Refactors, 1.7.0 Dec 21, 2016

fgalan self-assigned this Dec 22, 2016

This was referenced Dec 23, 2016

FIX Accept headers errors in NGSIv2 request were incorrectly rendered using "OrionError" element #2789

Merged

Erroneous API v2 request receives API v1 response #2765

Closed

This was referenced Jan 4, 2017

FIX CB_START_CMD_PREFIX at harnessFunctions.sh #2797

Merged

FIX use --prety-print in accumulstorStart in all .test in all cases where it makes sense #2798

Merged

fgalan modified the milestones: 1.9.0, 1.7.0 Jan 11, 2017

fgalan modified the milestones: 1.8.0, 1.9.0 Jan 29, 2017

fgalan mentioned this issue Jul 26, 2017

Unify json serialization by using rapidjson #2960

Open

fgalan modified the milestones: 1.9.0, 1.8.0 Sep 11, 2017

fgalan mentioned this issue Oct 17, 2017

FIX NGSIv1 responses don't use pretty-print JSON format any longer, as NGSIv2 responses work (potentially saving around 50% size) #3020

Merged

fgalan modified the milestones: 1.9.0, 1.11.0 Oct 19, 2017

fgalan mentioned this issue Jan 22, 2018

REMOVE all code related with PARANOID_JSON_INDENT and -paranoidV1Indent #3083

Merged

fgalan mentioned this issue Jan 25, 2018

FIX recover 119 unit test previously disabled #3086

Merged

fgalan closed this as completed Jan 25, 2018

fgalan mentioned this issue Apr 10, 2018

Csub cache improvements #2691

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean render()/check() NGSIv1 method from indent argument passing #2760

Clean render()/check() NGSIv1 method from indent argument passing #2760

fgalan commented Dec 9, 2016 •

edited

Loading

fgalan commented Jan 10, 2017 •

edited

Loading

fgalan commented Jan 11, 2017

fgalan commented Jul 25, 2017

fgalan commented Oct 17, 2017

fgalan commented Oct 17, 2017

fgalan commented Oct 17, 2017 •

edited

Loading

fgalan commented Oct 19, 2017

fgalan commented Jan 22, 2018

fgalan commented Jan 25, 2018

Clean render()/check() NGSIv1 method from indent argument passing #2760

Clean render()/check() NGSIv1 method from indent argument passing #2760

Comments

fgalan commented Dec 9, 2016 • edited Loading

fgalan commented Jan 10, 2017 • edited Loading

fgalan commented Jan 11, 2017

fgalan commented Jul 25, 2017

fgalan commented Oct 17, 2017

fgalan commented Oct 17, 2017

fgalan commented Oct 17, 2017 • edited Loading

fgalan commented Oct 19, 2017

fgalan commented Jan 22, 2018

fgalan commented Jan 25, 2018

fgalan commented Dec 9, 2016 •

edited

Loading

fgalan commented Jan 10, 2017 •

edited

Loading

fgalan commented Oct 17, 2017 •

edited

Loading