Michael Pasternak has uploaded a new change for review.
Change subject: sdk: refactor invocation flow
......................................................................
sdk: refactor invocation flow
This change introduces workaround for the race conditions
on [1] and [2] which are caused by the non ability of the
former to supply tread safe environment as seen in #949187
and #949189.
[1] cookielib.DefaultCookiePolicy
[2] httplib.HTTPConnection
Change-Id: I4c50e04ef763d4f572cfb4a5e4df0bb045401453
Signed-off-by: Michael Pasternak <mpast...@redhat.com>
---
M src/ovirtsdk/infrastructure/connectionspool.py
M src/ovirtsdk/infrastructure/proxy.py
A src/ovirtsdk/utils/synchronizationhelper.py
M src/ovirtsdk/web/connection.py
A src/ovirtsdk/web/cookiejaradapter.py
5 files changed, 285 insertions(+), 172 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine-sdk refs/changes/94/13894/1
diff --git a/src/ovirtsdk/infrastructure/connectionspool.py
b/src/ovirtsdk/infrastructure/connectionspool.py
index dbaf8a7..cc179cf 100644
--- a/src/ovirtsdk/infrastructure/connectionspool.py
+++ b/src/ovirtsdk/infrastructure/connectionspool.py
@@ -16,8 +16,15 @@
from Queue import Queue
import thread
+import cookielib
+import urlparse
+
+from cookielib import DefaultCookiePolicy
+
from ovirtsdk.web.connection import Connection
from ovirtsdk.infrastructure.errors import ImmutableError
+from ovirtsdk.utils.synchronizationhelper import synchronized
+
class ConnectionsPool(object):
'''
@@ -35,6 +42,14 @@
self.__url = url
self.__context = context
+
+ # Create the cookies policy and jar:
+ self.__cookies_jar = cookielib.CookieJar(
+ policy=cookielib.DefaultCookiePolicy(
+ strict_ns_domain=DefaultCookiePolicy.DomainStrictNoDots,
+ allowed_domains=self.__getAllowedDomains(url)
+ )
+ )
for _ in range(count):
self.__free_connections.put(item=Connection(url=url, \
@@ -64,10 +79,25 @@
# TODO: add more connections if needed
# continue
- def _freeResource(self, conn):
- with self.__rlock:
- conn = self.__busy_connections.pop(conn.get_id())
- self.__free_connections.put_nowait(conn)
+ def getCookiesJar(self):
+ """
+ returns cookies container
+ """
+ return self.__cookies_jar
+
+ @synchronized
+ def addCookieHeaders(self, request_adapter):
+ """
+ Adds the stored cookie/s to the request adapter:
+ """
+ self.getCookiesJar().add_cookie_header(request_adapter)
+
+ @synchronized
+ def storeCookies(self, response_adapter, request_adapter):
+ """
+ Stores the cookie/s located in the response adapter in request adapter:
+ """
+ self.getCookiesJar().extract_cookies(response_adapter, request_adapter)
def get_url(self):
return self.__url
@@ -76,6 +106,26 @@
def context(self):
return self.__context
+ @synchronized
+ def __getAllowedDomains(self, url):
+ '''
+ fetches allowed domains for cookie
+ '''
+
+ LOCAL_HOST = 'localhost'
+ parsed_url = urlparse.urlparse(url)
+ domains = [parsed_url.hostname]
+
+ if parsed_url.hostname == LOCAL_HOST:
+ return domains.append(LOCAL_HOST + '.local')
+
+ return domains
+
+ def _freeResource(self, conn):
+ with self.__rlock:
+ conn = self.__busy_connections.pop(conn.get_id())
+ self.__free_connections.put_nowait(conn)
+
def __setattr__(self, name, value):
if name in ['__context', 'context']:
raise ImmutableError(name)
diff --git a/src/ovirtsdk/infrastructure/proxy.py
b/src/ovirtsdk/infrastructure/proxy.py
index 7f604cd..caef125 100644
--- a/src/ovirtsdk/infrastructure/proxy.py
+++ b/src/ovirtsdk/infrastructure/proxy.py
@@ -14,77 +14,9 @@
# limitations under the License.
#
-import cookielib
-import socket
-import urlparse
-
-from ovirtsdk.infrastructure.errors import RequestError, ConnectionError, \
- FormatError
+from ovirtsdk.infrastructure.errors import RequestError, ConnectionError,
FormatError
from ovirtsdk.xml import params
-from cookielib import DefaultCookiePolicy
from lxml import etree
-
-class CookieJarAdapter():
- """
- This class is an adapter that implements the methods that the CookieJar
- expects and needs in order to add the cookie headers to an HTTP request.
-
- From the point of view of the CookieJar it looks like a urllib2 request and
- also like a response, but it just saves the headers to a list that will
later
- be retrieved by the proxy.
- """
- def __init__(self, url, headers):
- # Save the URL and the headers:
- self._url = url
- self._headers = headers
-
- # Extract the scheme and the host name from the URL:
- parsed_url = urlparse.urlparse(self._url)
- self._scheme = parsed_url.scheme
- self._host = parsed_url.hostname
-
- # The following methods are needed to simulate the behaviour of an urllib2
- # request class:
- def get_full_url(self):
- return self._url
-
- def get_host(self):
- return self._host
-
- def get_type(self):
- return self._scheme
-
- def is_unverifiable(self):
- return False
-
- def get_origin_req_host(self):
- return self._host
-
- def has_header(self, header):
- return header.lower() in self._headers
-
- def get_header(self, header_name, default=None):
- return self._headers.get(header_name.lower(), default)
-
- def header_items(self):
- return self._headers.items()
-
- def add_unredirected_header(self, key, val):
- self._headers[key.lower()] = val
-
- # The following method is needed to simulate the behaviour of an urllib2
- # response class:
- def info(self):
- return self
-
- # This methods simulates the object returned by the info method of the
- # urllib2 response class:
- def getheaders(self, name):
- result = []
- for key, value in self._headers.items():
- if key.lower() == name.lower():
- result.append(value)
- return result
class Proxy():
'''
@@ -102,26 +34,6 @@
# In order to create the cookies adapter we need to extract from the
# URL the host name, so that we can accept cookies only from that host:
self._url = self.__connections_pool.get_url()
-
- # Create the cookies policy and jar:
- cookies_policy = cookielib.DefaultCookiePolicy(
- strict_ns_domain=DefaultCookiePolicy.DomainStrictNoDots,
- allowed_domains=self.__getAllowedDomains(self._url))
- self._cookies_jar = cookielib.CookieJar(policy=cookies_policy)
-
- def __getAllowedDomains(self, url):
- '''
- fetches allowed domains for cookie
- '''
-
- LOCAL_HOST = 'localhost'
- parsed_url = urlparse.urlparse(url)
- domains = [parsed_url.hostname]
-
- if parsed_url.hostname == LOCAL_HOST:
- return domains.append(LOCAL_HOST + '.local')
-
- return domains
def getConnectionsPool(self):
'''
@@ -196,9 +108,10 @@
headers=headers, \
conn=self.getConnectionsPool().getConnection(), \
last=last,
- noParse=noParse)
+ noParse=noParse,
+ persistent_auth=self._persistent_auth)
- def __doRequest(self, method, url, conn, body=None, headers={},
last=False, noParse=False):
+ def __doRequest(self, method, url, conn, body=None, headers={},
last=False, noParse=False, persistent_auth=True):
'''
Performs HTTP request
@@ -209,75 +122,21 @@
@param headers: request headers
@param last: disables persistence authentication
@param noParse: disables xml2py conversion
+ @param persistent_auth: session based auth
'''
- try:
- # Copy request headers to avoid by-ref lookup after
- # JSESSIONID has been injected
- request_headers = headers.copy()
- # Add cookie headers as needed:
- request_adapter = CookieJarAdapter(self._url + url,
request_headers)
- self._cookies_jar.add_cookie_header(request_adapter)
-
- # Every request except the last one should indicate that we prefer
- # to use persistent authentication:
- if self._persistent_auth and not last:
- request_headers["Prefer"] = "persistent-auth"
-
- # Send the request and wait for the response:
- conn.doRequest(
+ response = conn.doRequest(
method=method,
url=url,
body=body,
- headers=request_headers,
- no_auth=self._persistent_auth and \
- self.__isSetJsessionCookie(self._cookies_jar)
+ headers=headers,
+ last=last,
+ persistent_auth=persistent_auth
)
- response = conn.getResponse()
-
- # Read the response headers (there is always a response,
- # even for error responses):
- response_headers = dict(response.getheaders())
-
- # Parse the received body only if there are no errors reported by
- # the server (this needs review, as less than 400 doesn't garantee
- # a correct response, it could be a redirect, and many other
- # things):
- if response.status >= 400:
- raise RequestError, response
-
- # Copy the cookies from the response:
- response_adapter = CookieJarAdapter(self._url, response_headers)
- self._cookies_jar.extract_cookies(response_adapter,
request_adapter)
-
- # Parse the body:
- response_body = response.read()
-
- # Print response body (if in debug mode)
- self.__do_debug(conn, response_body)
-
- if not noParse:
- return self.__xml2py(response_body)
- return response_body
-
- except socket.error, e:
- raise ConnectionError, str(e)
- finally:
- conn.close()
-
- def __isSetJsessionCookie(self, cookies_jar):
- '''
- Checks if JSESSIONID cookie is set
-
- @param cookies_jar: cookies container
- '''
- if cookies_jar and len(cookies_jar._cookies) > 0:
- for key in cookies_jar._cookies.keys():
- if cookies_jar._cookies[key].has_key('/api') and \
- cookies_jar._cookies[key]['/api'].has_key('JSESSIONID'):
- return True
- return False
+ if not noParse:
+ return self.__xml2py(response)
+ return response
def __xml2py(self, obj):
'''
@@ -291,13 +150,6 @@
# the motivation for this error is #915036
raise FormatError
return obj
-
- def __do_debug(self, conn, body):
- '''
- Prints request body (when in debug) to STDIO
- '''
- if conn.getConnection().debuglevel:
- print 'body:\n' + body if body else ''
def get_url(self):
'''
diff --git a/src/ovirtsdk/utils/synchronizationhelper.py
b/src/ovirtsdk/utils/synchronizationhelper.py
new file mode 100644
index 0000000..7e6ad27
--- /dev/null
+++ b/src/ovirtsdk/utils/synchronizationhelper.py
@@ -0,0 +1,47 @@
+#
+# Copyright (c) 2013 Red Hat Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+
+########################################
+############ GENERATED CODE ############
+########################################
+
+
+import threading
+
+def synchronized(function):
+ """
+ Synchronizes the function
+ """
+ return __usinglock(threading.RLock())(function)
+
+def __usinglock(lock):
+ def decorator(function):
+ body = __tryexecute(lock.release)(function)
+ def execute(*args, **kwargs):
+ lock.acquire()
+ return body(*args, **kwargs)
+ return execute
+ return decorator
+
+def __tryexecute(finallyf):
+ def decorator(invokeable):
+ def execute(*args, **kwargs):
+ try: result = invokeable(*args, **kwargs)
+ finally: finallyf()
+ return result
+ return execute
+ return decorator
diff --git a/src/ovirtsdk/web/connection.py b/src/ovirtsdk/web/connection.py
index fa30fc4..b21cc22 100644
--- a/src/ovirtsdk/web/connection.py
+++ b/src/ovirtsdk/web/connection.py
@@ -14,14 +14,18 @@
# limitations under the License.
#
-import base64
-from httplib import HTTPConnection
+import types
import urllib
import urlparse
-from ovirtsdk.web.httpsconnection import HTTPSConnection
-from ovirtsdk.infrastructure.errors import NoCertificatesError, ImmutableError
-import types
+import base64
+import socket
+
+from httplib import HTTPConnection
+
+from ovirtsdk.web.cookiejaradapter import CookieJarAdapter
from ovirtsdk.infrastructure.context import context
+from ovirtsdk.web.httpsconnection import HTTPSConnection
+from ovirtsdk.infrastructure.errors import NoCertificatesError,
ImmutableError, RequestError, ConnectionError
class Connection(object):
'''
@@ -40,6 +44,7 @@
strict=strict,
timeout=timeout)
+ self.__url = url
self.__connection.set_debuglevel(int(debug))
self.__headers = self.__createStaticHeaders(username, password)
self.__manager = manager
@@ -72,7 +77,7 @@
return headers
- def doRequest(self, method, url, body=urllib.urlencode({}), headers={},
no_auth=False):
+ def doRequest(self, method, url, body=urllib.urlencode({}), headers={},
last=False, persistent_auth=True):
'''
Performs HTTP request
@@ -80,9 +85,87 @@
@param url: URL to invoke the request on
@param body: request body
@param headers: request headers
- @param no_auth: do not authorize request (authorization is done via
cookie)
+ @param last: disables persistence authentication
+ @param persistent_auth: session based auth
'''
- return self.__connection.request(method, url, body,
self.getHeaders(headers, no_auth))
+
+ try:
+ # Copy request headers to avoid by-ref lookup after
+ # JSESSIONID has been injected
+ request_headers = headers.copy()
+
+ # Add cookie headers as needed:
+ request_adapter = CookieJarAdapter(self.__url + url,
request_headers)
+ self.__manager.addCookieHeaders(request_adapter)
+
+ # Every request except the last one should indicate that we prefer
+ # to use persistent authentication:
+ if persistent_auth and not last:
+ request_headers["Prefer"] = "persistent-auth"
+
+ # Send the request and wait for the response:
+ response = self.__connection.request(
+ method,
+ url,
+ body,
+ self.getHeaders(request_headers,
+ no_auth=
+ persistent_auth and \
+ self.__isSetJsessionCookie(
+
self.__manager.getCookiesJar()
+ ),
+ )
+ )
+
+ response = self.getResponse()
+
+ # Read the response headers (there is always a response,
+ # even for error responses):
+ response_headers = dict(response.getheaders())
+
+ # Parse the received body only if there are no errors reported by
+ # the server (this needs review, as less than 400 doesn't garantee
+ # a correct response, it could be a redirect, and many other
+ # things):
+ if response.status >= 400:
+ raise RequestError, response
+
+ # Copy the cookies from the response:
+ response_adapter = CookieJarAdapter(self.__url, response_headers)
+ self.__manager.storeCookies(response_adapter, request_adapter)
+
+ # Parse the body:
+ response_body = response.read()
+
+ # Print response body (if in debug mode)
+ self.__do_debug(self, response_body)
+
+ return response_body
+
+ except socket.error, e:
+ raise ConnectionError, str(e)
+ finally:
+ self.close()
+
+ def __do_debug(self, conn, body):
+ '''
+ Prints request body (when in debug) to STDIO
+ '''
+ if conn.getConnection().debuglevel:
+ print 'body:\n' + body if body else ''
+
+ def __isSetJsessionCookie(self, cookies_jar):
+ '''
+ Checks if JSESSIONID cookie is set
+
+ @param cookies_jar: cookies container
+ '''
+ if cookies_jar and len(cookies_jar._cookies) > 0:
+ for key in cookies_jar._cookies.keys():
+ if cookies_jar._cookies[key].has_key('/api') and \
+ cookies_jar._cookies[key]['/api'].has_key('JSESSIONID'):
+ return True
+ return False
def getHeaders(self, headers, no_auth=False):
extended_headers = self.getDefaultHeaders(no_auth)
diff --git a/src/ovirtsdk/web/cookiejaradapter.py
b/src/ovirtsdk/web/cookiejaradapter.py
new file mode 100644
index 0000000..0d2f203
--- /dev/null
+++ b/src/ovirtsdk/web/cookiejaradapter.py
@@ -0,0 +1,81 @@
+
+#
+# Copyright (c) 2010 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import urlparse
+
+
+class CookieJarAdapter():
+ """
+ This class is an adapter that implements the methods that the CookieJar
+ expects and needs in order to add the cookie headers to an HTTP request.
+
+ From the point of view of the CookieJar it looks like a urllib2 request and
+ also like a response, but it just saves the headers to a list that will
later
+ be retrieved by the proxy.
+ """
+ def __init__(self, url, headers):
+ # Save the URL and the headers:
+ self._url = url
+ self._headers = headers
+
+ # Extract the scheme and the host name from the URL:
+ parsed_url = urlparse.urlparse(self._url)
+ self._scheme = parsed_url.scheme
+ self._host = parsed_url.hostname
+
+ # The following methods are needed to simulate the behaviour of an urllib2
+ # request class:
+ def get_full_url(self):
+ return self._url
+
+ def get_host(self):
+ return self._host
+
+ def get_type(self):
+ return self._scheme
+
+ def is_unverifiable(self):
+ return False
+
+ def get_origin_req_host(self):
+ return self._host
+
+ def has_header(self, header):
+ return header.lower() in self._headers
+
+ def get_header(self, header_name, default=None):
+ return self._headers.get(header_name.lower(), default)
+
+ def header_items(self):
+ return self._headers.items()
+
+ def add_unredirected_header(self, key, val):
+ self._headers[key.lower()] = val
+
+ # The following method is needed to simulate the behaviour of an urllib2
+ # response class:
+ def info(self):
+ return self
+
+ # This methods simulates the object returned by the info method of the
+ # urllib2 response class:
+ def getheaders(self, name):
+ result = []
+ for key, value in self._headers.items():
+ if key.lower() == name.lower():
+ result.append(value)
+ return result
--
To view, visit http://gerrit.ovirt.org/13894
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c50e04ef763d4f572cfb4a5e4df0bb045401453
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine-sdk
Gerrit-Branch: master
Gerrit-Owner: Michael Pasternak <mpast...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
