Commit d0b83543 authored by Georgios Dagkakis's avatar Georgios Dagkakis

Add an API to be able to construct urls

This merge request is trying to provide an API so that we can construct urls in a more organised manner than manipulating strings.

The motivation, started from this merge request: !333, where absolute_url is overriden for Web Section and Web Site. Then we found that we have to do many dirty changes like:
```
              absolute_url python: here.absolute_url();
              absolute_url python: absolute_url[:-1] if absolute_url.endswith('/') else absolute_url;
```
(more of those in f601d7a5).

So we said we should make an API to have an organised manner, like we have e.g. Base_redirect to handle redirects.

- First commit adds this as a script in erp5_core, along with a test.
- Second commit uses it. It is not on my scope to change all erp5 code, but I focused on changing on the things I already had to in
f601d7a5

@romain , @kazuhiko please make comments. Tests passed, but not on the final revision (did some amendments) and I think better not to re-run suite  before getting comments. Also, @tatuya , I think you are responsible for ERP5 interfaces, so I would like your opinion if erp5_core is the good place, or maybe some other BT or product.

My plan would be to merge this and then !333 rebased on this. But in any-way this should be separate from !333.

/reviewed-on !431
parents 72b2b986 45bb448f
...@@ -60,4 +60,6 @@ if document.getValidationState() not in validation_state: ...@@ -60,4 +60,6 @@ if document.getValidationState() not in validation_state:
web_section = context.getWebSectionValue() web_section = context.getWebSectionValue()
if web_section is None: if web_section is None:
web_section = context web_section = context
return "%s/%s" % (web_section.absolute_url(), reference) return web_section.constructUrlFor(
document_reference=reference
)
...@@ -10,8 +10,9 @@ ...@@ -10,8 +10,9 @@
came_from_valid python: not came_from or url_topmost_document.isURLAncestorOf(came_from);"> came_from_valid python: not came_from or url_topmost_document.isURLAncestorOf(came_from);">
<tal:block tal:condition="isAnon"> <tal:block tal:condition="isAnon">
<tal:block tal:define="dummy python: response.expireCookie('__ac', path='/'); <tal:block tal:define="dummy python: response.expireCookie('__ac', path='/');
url python: '%s/login_form?portal_status_message=%s' % (here.absolute_url(), here.Base_translateString('Login and/or password is incorrect.')); parameter_dict python: {'portal_status_message': here.Base_translateString('Login and/or password is incorrect.')};
url python: came_from and '%s&amp;came_from=%s' % (url, came_from) or url; parameter_dict python: parameter_dict.update({'came_from': came_from}) if came_from else parameter_dict;
url python: here.constructUrlFor(form_id='login_form', parameter_dict=parameter_dict);
dummy python: response.redirect(url);" /> dummy python: response.redirect(url);" />
</tal:block> </tal:block>
<tal:block tal:condition="not: isAnon"> <tal:block tal:condition="not: isAnon">
...@@ -19,7 +20,7 @@ ...@@ -19,7 +20,7 @@
<tal:block tal:define="dummy python: response.redirect(came_from or here.getPermanentURL(here));" /> <tal:block tal:define="dummy python: response.redirect(came_from or here.getPermanentURL(here));" />
</tal:block> </tal:block>
<tal:block tal:condition="not: came_from_valid"> <tal:block tal:condition="not: came_from_valid">
<tal:block tal:define="dummy python: response.redirect('%s?portal_status_message=%s' % (url_topmost_document.absolute_url(), here.Base_translateString('Redirection to an external site prevented.')));" /> <tal:block tal:define="dummy python: response.redirect(url_topmost_document.constructUrlFor(parameter_dict={'portal_status_message': here.Base_translateString('Redirection to an external site prevented.')}))" />
</tal:block> </tal:block>
</tal:block> </tal:block>
</tal:block> </tal:block>
......
...@@ -4,6 +4,6 @@ ...@@ -4,6 +4,6 @@
""" """
if document.hasReference(): if document.hasReference():
file_name = document.Document_getStandardFileName() file_name = document.Document_getStandardFileName()
return "%s/%s" % (context.absolute_url(), file_name) return context.constructUrlFor(document_reference=file_name)
else: else:
return "%s%s" % (document.getAbsoluteUrl(),view and '/view' or '') return context.constructUrlFor(form_id='view' if view else None)
...@@ -15,16 +15,17 @@ default_language = web_section.getLayoutProperty("default_available_language", d ...@@ -15,16 +15,17 @@ default_language = web_section.getLayoutProperty("default_available_language", d
website_url_set = {} website_url_set = {}
#simplify code of Base_doLanguage, can't ues Base_doLanguage directly #simplify code of Base_doLanguage, can't ues Base_doLanguage directly
root_website_url = web_section.getOriginalDocument().absolute_url() root_website_url = web_section.getOriginalDocument().constructUrlFor()
website_url_pattern = r'^%s(?:%s)*(/|$)' % ( website_url_pattern = r'^%s(?:%s)*(/|$)' % (
re.escape(root_website_url), re.escape(root_website_url),
'|'.join('/' + re.escape(x) for x in available_language_set)) '|'.join('/' + re.escape(x) for x in available_language_set))
for language in available_language_set: for language in available_language_set:
web_section_url = web_section.constructUrlFor()
if language == default_language: if language == default_language:
website_url_set[language] = re.sub(website_url_pattern, r'%s/\1' % root_website_url, web_section.absolute_url()) website_url_set[language] = re.sub(website_url_pattern, r'%s/\1' % root_website_url, web_section_url)
else: else:
website_url_set[language]= re.sub(website_url_pattern, r'%s/%s/\1' % (root_website_url, language), web_section.absolute_url()) website_url_set[language]= re.sub(website_url_pattern, r'%s/%s/\1' % (root_website_url, language), web_section_url)
view_as_web_method = default_web_page.getTypeBasedMethod( view_as_web_method = default_web_page.getTypeBasedMethod(
"viewAsWeb", "viewAsWeb",
......
...@@ -192,13 +192,11 @@ if dialog_method != update_method and clean_kw.get('deferred_style', 0): ...@@ -192,13 +192,11 @@ if dialog_method != update_method and clean_kw.get('deferred_style', 0):
request.set('deferred_style_dialog_method', dialog_method) request.set('deferred_style_dialog_method', dialog_method)
dialog_method = 'Base_activateSimpleView' dialog_method = 'Base_activateSimpleView'
url_params_string = make_query(clean_kw)
# XXX: We always redirect in report mode to make sure portal_skin # XXX: We always redirect in report mode to make sure portal_skin
# parameter is taken into account by SkinTool. # parameter is taken into account by SkinTool.
# If url is too long, we do not redirect to avoid crash. # If url is too long, we do not redirect to avoid crash.
# XXX: 2000 is an arbitrary value resulted from trial and error. # XXX: 2000 is an arbitrary value resulted from trial and error.
if (not(can_redirect) or len(url_params_string) > 2000): if (not(can_redirect) or len(make_query(clean_kw)) > 2000):
if dialog_method != update_method: if dialog_method != update_method:
# When we are not executing the update action, we have to change the skin # When we are not executing the update action, we have to change the skin
# manually, # manually,
...@@ -226,22 +224,11 @@ if (not(can_redirect) or len(url_params_string) > 2000): ...@@ -226,22 +224,11 @@ if (not(can_redirect) or len(url_params_string) > 2000):
request.set('URL', '%s/%s' % (context.absolute_url(), dialog_method)) request.set('URL', '%s/%s' % (context.absolute_url(), dialog_method))
return dialog_form(**kw) return dialog_form(**kw)
if error_message != '': redirect_url = context.constructUrlFor(
redirect_url = '%s/%s?%s' % ( context.absolute_url() form_id=dialog_method,
, dialog_method parameter_dict={'portal_status_message': error_message} if error_message != '' \
, 'portal_status_message=%s' % error_message else clean_kw
)
elif url_params_string != '':
redirect_url = '%s/%s?%s' % ( context.absolute_url()
, dialog_method
, url_params_string
) )
else:
redirect_url = '%s/%s' % ( context.absolute_url()
, dialog_method
)
return request.RESPONSE.redirect(redirect_url) return request.RESPONSE.redirect(redirect_url)
# vim: syntax=python # vim: syntax=python
...@@ -25,9 +25,16 @@ if dialog_id not in ('', None): ...@@ -25,9 +25,16 @@ if dialog_id not in ('', None):
# Prevent users who don't have rights to edit the object from # Prevent users who don't have rights to edit the object from
# editing it by calling the Base_edit script with correct # editing it by calling the Base_edit script with correct
# parameters directly. # parameters directly.
if not silent_mode and not request.AUTHENTICATED_USER.has_permission('Modify portal content', context) : if not silent_mode and not request.AUTHENTICATED_USER.has_permission('Modify portal content', context) :
msg = Base_translateString("You do not have the permissions to edit the object.") redirect_url = context.constructUrlFor(
redirect_url = '%s/%s?selection_index=%s&selection_name=%s&%s' % (context.absolute_url(), form_id, selection_index, selection_name, 'portal_status_message=%s' % msg) form_id=form_id,
parameter_dict={
'selection_index': selection_index,
'selection_name': selection_name,
'portal_status_message': Base_translateString("You do not have the permissions to edit the object.")
}
)
return request['RESPONSE'].redirect(redirect_url) return request['RESPONSE'].redirect(redirect_url)
# Get the form # Get the form
...@@ -248,12 +255,6 @@ if message_only: ...@@ -248,12 +255,6 @@ if message_only:
ignore_layout = int(ignore_layout) ignore_layout = int(ignore_layout)
editable_mode = int(editable_mode) editable_mode = int(editable_mode)
spp = context.getPhysicalPath()
spp =list(spp)
s_url = request["SERVER_URL"]
spp.insert(0,s_url)
#calculate direct the url instead of using absolute_url
new_url = '/'.join(spp)
# for web mode, we should use 'view' instead of passed form_id # for web mode, we should use 'view' instead of passed form_id
# after 'Save & View'. # after 'Save & View'.
...@@ -262,25 +263,25 @@ if context.REQUEST.get('is_web_mode', False) and \ ...@@ -262,25 +263,25 @@ if context.REQUEST.get('is_web_mode', False) and \
form_id = 'view' form_id = 'view'
if not selection_index: if not selection_index:
redirect_url = '%s/%s?ignore_layout:int=%s&editable_mode:int=%s&portal_status_message=%s' % ( redirect_url = context.constructUrlFor(
context.absolute_url(), form_id=form_id,
form_id, parameter_dict={
ignore_layout, 'ignore_layout': ignore_layout,
editable_mode, 'editable_mode': editable_mode,
message) 'portal_status_message': message,
}
)
else: else:
redirect_url = '%s/%s?selection_index=%s&selection_name=%s&ignore_layout:int=%s&editable_mode=%s&portal_status_message=%s' % ( redirect_url = context.constructUrlFor(
context.absolute_url(), form_id=form_id,
form_id, parameter_dict={
selection_index, 'selection_index': selection_index,
selection_name, 'selection_name': selection_name,
ignore_layout, 'ignore_layout': ignore_layout,
editable_mode, 'editable_mode': editable_mode,
message) 'portal_status_message': message,
}
)
result = request['RESPONSE'].redirect(redirect_url) result = request['RESPONSE'].redirect(redirect_url)
if silent_mode: return result, 'redirect' if silent_mode: return result, 'redirect'
......
...@@ -9,7 +9,7 @@ if website is not None and website.isStaticLanguageSelection(): ...@@ -9,7 +9,7 @@ if website is not None and website.isStaticLanguageSelection():
# Web Mode # Web Mode
root_website = website.getOriginalDocument() root_website = website.getOriginalDocument()
default_language = root_website.getDefaultAvailableLanguage() default_language = root_website.getDefaultAvailableLanguage()
root_website_url = root_website.absolute_url() root_website_url = root_website.constructUrlFor()
website_url_pattern = r'^%s(?:%s)*(/|$)' % ( website_url_pattern = r'^%s(?:%s)*(/|$)' % (
re.escape(root_website_url), re.escape(root_website_url),
'|'.join('/' + re.escape(x) for x in root_website.getAvailableLanguageList())) '|'.join('/' + re.escape(x) for x in root_website.getAvailableLanguageList()))
...@@ -24,7 +24,7 @@ if website is not None and website.isStaticLanguageSelection(): ...@@ -24,7 +24,7 @@ if website is not None and website.isStaticLanguageSelection():
if select_language == default_language: if select_language == default_language:
redirect_url = root_website_url redirect_url = root_website_url
else: else:
redirect_url = root_website_url + '/' + select_language redirect_url = root_website.constructUrlFor(document_reference=select_language)
return context.REQUEST.RESPONSE.redirect(redirect_url) return context.REQUEST.RESPONSE.redirect(redirect_url)
else: else:
# ERP5 Mode # ERP5 Mode
......
...@@ -6,22 +6,29 @@ ...@@ -6,22 +6,29 @@
TODO: improve API and extensively document. ERP5Site_redirect may TODO: improve API and extensively document. ERP5Site_redirect may
be redundant. be redundant.
""" """
# BBB: originally, form_id was the first positional argument
if not redirect_url or '/' not in redirect_url:
form_id = redirect_url or kw.pop('form_id', None)
redirect_url = context.absolute_url()
if form_id:
redirect_url += '/' + form_id
from ZTUtils import make_query from ZTUtils import make_query
request = context.getPortalObject().REQUEST request = context.getPortalObject().REQUEST
request_form = request.form request_form = request.form
request_form.update(kw) request_form.update(kw)
request_form = context.ERP5Site_filterParameterList(request_form) request_form = context.ERP5Site_filterParameterList(request_form)
request_form.update(keep_items) request_form.update(keep_items)
parameter_dict = dict([(k, v) for k, v in request_form.items() if k and v is not None])
parameters = make_query(dict([(k, v) for k, v in request_form.items() if k and v is not None])) if not redirect_url or '/' not in redirect_url:
if len(parameters): # Used when redirect_url is not given explicitely and we redirect to an ERP5 document
# BBB: originally, form_id was the first positional argument
form_id = redirect_url or kw.pop('form_id', None)
redirect_url = context.constructUrlFor(
form_id=form_id,
parameter_dict=parameter_dict,
)
else:
# Used if redirect_url is given explicitely (e.g. if we redirect to external web page)
# so constructUrlFor is not usable,
# since we have no document to redirect using is as context.
parameters = make_query(parameter_dict)
if len(parameters):
if '?' in redirect_url: if '?' in redirect_url:
separator = '&' separator = '&'
else: else:
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
local_parameter_list local_parameter_list | python: {}; local_parameter_list local_parameter_list | python: {};
action_context python: portal.restrictedTraverse(request.get('object_path', '?'), here); action_context python: portal.restrictedTraverse(request.get('object_path', '?'), here);
global actions python: here.Base_filterDuplicateActions(portal.portal_actions.listFilteredActionsFor(action_context)); global actions python: here.Base_filterDuplicateActions(portal.portal_actions.listFilteredActionsFor(action_context));
global url here/absolute_url; global url python: here.constructUrlFor();
global current_form_id python: local_parameter_list.get('dialog_id', local_parameter_list.get('form_id', 'view')); global current_form_id python: local_parameter_list.get('dialog_id', local_parameter_list.get('form_id', 'view'));
dummy python: request.set('current_form_id', current_form_id); dummy python: request.set('current_form_id', current_form_id);
global current_url python: '%s/%s' % (url, current_form_id); global current_url python: '%s/%s' % (url, current_form_id);
......
portal = context.getPortalObject() portal = context.getPortalObject()
if portal.portal_skins.updateSkinCookie(): if portal.portal_skins.updateSkinCookie():
portal.setupCurrentSkin() portal.setupCurrentSkin()
url = REQUEST.get("came_from") came_from = REQUEST.get("came_from")
if portal.portal_membership.isAnonymousUser(): if portal.portal_membership.isAnonymousUser():
RESPONSE.expireCookie("__ac", path="/") RESPONSE.expireCookie("__ac", path="/")
url = "%s/login_form?portal_status_message=%s" % ( parameter_dict = {'portal_status_message': context.Base_translateString("Login and/or password is incorrect.")}
context.absolute_url(), if came_from:
context.Base_translateString("Login and/or password is incorrect.") parameter_dict['came_from'] = came_from
+ ("&amp;came_from=" + url if url else "")) url = context.constructUrlFor(
elif not url: form_id='login_form',
url = context.absolute_url() parameter_dict=parameter_dict
)
elif came_from:
url = came_from
else:
url = context.constructUrlFor()
topmost_url_document = context.Base_getURLTopmostDocumentValue() topmost_url_document = context.Base_getURLTopmostDocumentValue()
if not topmost_url_document.isURLAncestorOf(url): if not topmost_url_document.isURLAncestorOf(url):
return context.ERP5Site_redirect(topmost_url_document.absolute_url(), return context.ERP5Site_redirect(topmost_url_document.constructUrlFor(),
keep_items={'portal_status_message': 'Redirection to an external site prevented.'}) keep_items={'portal_status_message': 'Redirection to an external site prevented.'})
return RESPONSE.redirect(url) return RESPONSE.redirect(url)
...@@ -74,6 +74,7 @@ import warnings ...@@ -74,6 +74,7 @@ import warnings
from urlparse import urlparse from urlparse import urlparse
REINDEX_SPLIT_COUNT = 100 # if folder containes more than this, reindexing should be splitted. REINDEX_SPLIT_COUNT = 100 # if folder containes more than this, reindexing should be splitted.
from Products.ERP5Type.Message import translateString from Products.ERP5Type.Message import translateString
from ZTUtils import make_query
# Dummy Functions for update / upgrade # Dummy Functions for update / upgrade
def dummyFilter(object,REQUEST=None): def dummyFilter(object,REQUEST=None):
...@@ -496,6 +497,48 @@ class FolderMixIn(ExtensionClass.Base): ...@@ -496,6 +497,48 @@ class FolderMixIn(ExtensionClass.Base):
raise AccessControl_Unauthorized(method_id) raise AccessControl_Unauthorized(method_id)
return self._recurseCallMethod(method_id, restricted=True, *args, **kw) return self._recurseCallMethod(method_id, restricted=True, *args, **kw)
security.declarePublic('constructUrlFor')
def constructUrlFor(
self,
form_id=None,
document_reference=None,
parameter_dict=None,
**kw
):
'''
Utility method to help in the construction of urls.
It should be run in the context of a document that we want as base for the url.
Arguments:
- form_id: the form that is invoked
- document_reference: the reference of a document that is rendered,
- parameter_dict: dictionary containing all get parameters
Other parameters can be used for type based methods.
'''
# Try to get type based method, but not for ERP5 Site, since
# _getTypeBasedMethod is defined in Base so not available for ERP5 Site
if self.getPortalType() != 'ERP5 Site':
method = self._getTypeBasedMethod('constructUrlFor')
if method is not None:
return method(
form_id=form_id,
document_reference=document_reference,
parameter_dict=parameter_dict,
**kw
)
url = self.absolute_url()
# form_id and document_reference are mutually exclusive,
# we should not expect both in a url
assert not (form_id and document_reference), 'Not allowed to have both form and document in the same url'
# Note that form_id and document_reference are handled the same way,
# it is different variable for semantic reasons
if form_id:
url = '%s/%s' % (url, form_id)
elif document_reference:
url = '%s/%s' % (url, document_reference)
if parameter_dict:
url = '%s?%s' % (url, make_query(parameter_dict))
return url
security.declarePublic('isURLAncestorOf') security.declarePublic('isURLAncestorOf')
def isURLAncestorOf(self, given_url): def isURLAncestorOf(self, given_url):
""" """
...@@ -506,7 +549,7 @@ class FolderMixIn(ExtensionClass.Base): ...@@ -506,7 +549,7 @@ class FolderMixIn(ExtensionClass.Base):
not access any document in given path, not access any document in given path,
hence it does not compute the inner acquisition path. hence it does not compute the inner acquisition path.
""" """
document_url = self.absolute_url() document_url = self.constructUrlFor()
parsed_given_url = urlparse(given_url) parsed_given_url = urlparse(given_url)
parsed_document_url = urlparse(document_url) parsed_document_url = urlparse(document_url)
# XXX note that the following check: # XXX note that the following check:
......
...@@ -3244,6 +3244,54 @@ return [ ...@@ -3244,6 +3244,54 @@ return [
# but this did not affect the other role # but this did not affect the other role
self.assertTrue(hasRole(role2)) self.assertTrue(hasRole(role2))
def test_constructUrlFor(self):
portal_url = self.portal.absolute_url()
# check url built for portal
portal_url = self.portal.absolute_url()
self.assertEqual(self.portal.constructUrlFor(), '%s' % portal_url)
# check urls built for a module
module_id = 'test_module'
module = self.portal.newContent(portal_type='Folder', id=module_id)
module_url = module.absolute_url()
self.assertEqual(
module.constructUrlFor(),
'%s/%s' % (portal_url, module_id)
)
# check url built for a form
form_id = 'view'
self.assertEqual(
module.constructUrlFor(form_id=form_id),
'%s/%s' % (module_url, form_id)
)
# check url built for a document reference
file_reference = 'test_file_reference'
self.portal.test_module.newContent(
portal_type='File',
reference=file_reference
)
self.assertEqual(
module.constructUrlFor(document_reference=file_reference),
'%s/%s' % (module_url, file_reference)
)
# check that form_id and document_reference are exclusive
# check url built with GET parameters
self.assertRaises(
AssertionError,
module.constructUrlFor,
form_id=form_id,
document_reference=file_reference
)
parameter_dict = {'ignore_layout': 1, 'came_from': 'foo/bar'}
self.assertIn(
module.constructUrlFor(
parameter_dict=parameter_dict,
),
(
'%s/%s?came_from=foo/bar&ignore_layout:int=1' % (portal_url, module_id),
'%s/%s?ignore_layout:int=1&came_from=foo/bar' % (portal_url, module_id),
)
)
class TestAccessControl(ERP5TypeTestCase): class TestAccessControl(ERP5TypeTestCase):
# Isolate test in a dedicaced class in order not to break other tests # Isolate test in a dedicaced class in order not to break other tests
# when this one fails. # when this one fails.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment