yahoo-eng-team team mailing list archive

[Venkat Sundaram <tsv@xxxxxx>]

Public bug reported:

The XmlBodyMiddleware converts request body from XML string to JSON
string and passes it on to the JsonBodyMiddleware next in chain.

Observe the following line in the XmlBodyMiddleware.process_request method:
         request.body = jsonutils.dumps(
                         serializer.from_xml(request.body))

serializer.from_xml already converts the body to a python dict, but it
dumps it again as a JSON string to the request body.

Now, observe the following lines in JsonBodyMiddleware .process_request method:
        # Abort early if we don't have any work to do
        params_json = request.body
        if not params_json:
            return
 ...
        params = {}
        for k, v in params_parsed.iteritems():
            if k in ('self', 'context'):
                continue
            if k.startswith('_'):
                continue
            params[k] = v
...
        request.environ[PARAMS_ENV] = params

The JsonBodyMiddleware converts JSON string to dict and then stores the
dict in request.environ (It does some additional filtering of the dict
before storing, but that doesn't look like it is JSON specific)

So, if the input request body has XML and the xml_body/json_body
middlware are used in the paste pipeline, the effective conversion is:
XML string => dict => JSON string => dict

This may induce a performance penalty (however minor it is) due to the
unnecessary JSON to dict conversion. The following is the required
conversion: XML string => dict

To achieve this, the following could be done:

Refactor the following lines from JsonBodyMiddlware.process_request method and move it to a common module as a method:
        params = {}
        for k, v in params_parsed.iteritems():
            if k in ('self', 'context'):
                continue
            if k.startswith('_'):
                continue
            params[k] = v

Then, call that method from XmlBodyMiddleware.process_request method
passing the serialized xml dict. Store the filtered dict as
request.environ[PARAMS_ENV]. Then, set the request.body to None.

With this change, the JsonBodyMiddlware next in chain would not process the request due to the following initial check:
        # Abort early if we don't have any work to do
        params_json = request.body
        if not params_json:
            return

** Affects: keystone
     Importance: Undecided
         Status: New

** Summary changed:

- XmlBodyMiddleware logic causes an unnecessary JSON string to dict conversion
+ XmlBodyMiddleware logic causes an unnecessary JSON to dict conversion

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to Keystone.
https://bugs.launchpad.net/bugs/1270516

Title:
  XmlBodyMiddleware logic causes an unnecessary JSON to dict conversion

Status in OpenStack Identity (Keystone):
  New

Bug description:
  The XmlBodyMiddleware converts request body from XML string to JSON
  string and passes it on to the JsonBodyMiddleware next in chain.

  Observe the following line in the XmlBodyMiddleware.process_request method:
           request.body = jsonutils.dumps(
                           serializer.from_xml(request.body))

  serializer.from_xml already converts the body to a python dict, but it
  dumps it again as a JSON string to the request body.

  Now, observe the following lines in JsonBodyMiddleware .process_request method:
          # Abort early if we don't have any work to do
          params_json = request.body
          if not params_json:
              return
   ...
          params = {}
          for k, v in params_parsed.iteritems():
              if k in ('self', 'context'):
                  continue
              if k.startswith('_'):
                  continue
              params[k] = v
  ...
          request.environ[PARAMS_ENV] = params

  The JsonBodyMiddleware converts JSON string to dict and then stores
  the dict in request.environ (It does some additional filtering of the
  dict before storing, but that doesn't look like it is JSON specific)

  So, if the input request body has XML and the xml_body/json_body
  middlware are used in the paste pipeline, the effective conversion is:
  XML string => dict => JSON string => dict

  This may induce a performance penalty (however minor it is) due to the
  unnecessary JSON to dict conversion. The following is the required
  conversion: XML string => dict

  To achieve this, the following could be done:

  Refactor the following lines from JsonBodyMiddlware.process_request method and move it to a common module as a method:
          params = {}
          for k, v in params_parsed.iteritems():
              if k in ('self', 'context'):
                  continue
              if k.startswith('_'):
                  continue
              params[k] = v

  Then, call that method from XmlBodyMiddleware.process_request method
  passing the serialized xml dict. Store the filtered dict as
  request.environ[PARAMS_ENV]. Then, set the request.body to None.

  With this change, the JsonBodyMiddlware next in chain would not process the request due to the following initial check:
          # Abort early if we don't have any work to do
          params_json = request.body
          if not params_json:
              return

To manage notifications about this bug go to:
https://bugs.launchpad.net/keystone/+bug/1270516/+subscriptions