openapi: incorrect rollbacks handling

[trojikman]

Message from the user:
We are using your OpenAPI module for Odoo V13 and found some really
interesting behaviour when error is raises.

In short:

• Defining a public method to a model class and enabling access to it.
• Editing any field on the model through API
• Raising any error in the defined function
• All changes are saved to record of the model even when error is raised

So based on our investigation it seems that OpenAPI doesn't handle odoo
rollbacks correctly and this leads to possible saving incomplete data to DB.
Is this intended behaviour or a bug in the OpenAPI implementation?

In full (POC):

1. Setting up a public method to any class. (res.partner this case):

@api.model
def test_call(self):
random_partner = self.env['res.partner'].browse(85)

# add a bit of information to some field
random_partner.function += "C"

# raise error right after
raise ValidationError("error")

2. calling the function from the API

3. Response is returned from the API call that has the validation error
   with the text error

4. Changes are still implemented in the DB

(When stepping through the OpenAPI code these key points were found)
** When exception is hit in the called method, pinguin.py
controller_method_wrapper seems to catch it and form a response:*

def decorator(controller_method):
@api_route(*args, **kwargs)
@functools.wraps(controller_method)
def controller_method_wrapper(*iargs, **ikwargs):

auth_header = get_auth_header(
request.httprequest.headers, raise_exception=True
)
db_name, user_token = get_data_from_auth_header(auth_header)
setup_db(request.httprequest, db_name)
authenticated_user = authenticate_token_for_user(user_token)
namespace = get_namespace_by_name_from_users_namespaces(
authenticated_user, ikwargs["namespace"],
raise_exception=True
)
data_for_log = {
"namespace_id": namespace.id,
"namespace_log_request": namespace.log_request,
"namespace_log_response": namespace.log_response,
"user_id": authenticated_user.id,
"user_request": None,
"user_response": None,
}

try:
response = controller_method(*iargs, **ikwargs)
except werkzeug.exceptions.HTTPException as e:
response = e.response
except Exception as e:
traceback.print_exc()
if hasattr(e, "error") and isinstance(e.error, Exception):
e = e.error
response = error_response(
status=500,
error=type(e).__name__,
error_descrip=e.name if hasattr(e, "name") else str(e),
)

data_for_log.update(
{"user_request": request.httprequest, "user_response":
response}
)
create_log_record(**data_for_log)

return response

** apijsonrequest.py dispatch method that handles the request that has
the actual _handle_exception method is never reached*

def dispatch(self):
if self.jsonp_handler:
return self.jsonp_handler()
try:
rpc_request_flag = rpc_request.isEnabledFor(logging.DEBUG)
rpc_response_flag = rpc_response.isEnabledFor(logging.DEBUG)
if rpc_request_flag or rpc_response_flag:
endpoint = self.endpoint.method.__name__
model = self.params.get("model")
method = self.params.get("method")
args = self.params.get("args", [])

start_time = time.time()
start_memory = 0
if psutil:
start_memory = memory_info(psutil.Process(os.getpid()))
if rpc_request and rpc_response_flag:
rpc_request.debug(
"%s: %s %s, %s", endpoint, model, method,
pprint.pformat(args)
)
# ----- result is returned here but exception handler is never
reached!
result = self._call_function(**self.params)

if rpc_request_flag or rpc_response_flag:
end_time = time.time()
end_memory = 0
if psutil:
end_memory = memory_info(psutil.Process(os.getpid()))
logline = "{}: {} {}: time:{:.3f}s mem: {}k -> {}k (diff:
{}k)".format(
endpoint,
model,
method,
end_time - start_time,
start_memory / 1024,
end_memory / 1024,
(end_memory - start_memory) / 1024,
)
if rpc_response_flag:
rpc_response.debug("%s, %s", logline,
pprint.pformat(result))
else:
rpc_request.debug(logline)
return self._json_response(result)
except Exception as e:
return self._handle_exception(e)

odoo\http.py __exit__ method is reached that does the commit() in DB

def __exit__(self, exc_type, exc_value, traceback):
_request_stack.pop()

if self._cr:
try:
if exc_type is None and not self._failed:
# --------- COMMIT is made here as no exception is
raised and self has not failed
self._cr.commit()
if self.registry:
self.registry.signal_changes()
elif self.registry:
self.registry.reset_changes()
finally:
self._cr.close()
# just to be sure no one tries to re-use the request
self.disable_db = True
self.uid = None