Skip to content

Commit

Permalink
some micro optimizations
Browse files Browse the repository at this point in the history 
reduce calls where possible
  • Loading branch information
@jensens
jensens committed Dec 14, 2020
1 parent 12efd8b commit ed4d6b0
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 51 deletions.
19 changes: 8 additions & 11 deletions plone/app/caching/lookup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,9 @@ def __call__(self):
return None

# 3. Look up the published name in the page template mapping
if ploneCacheSettings.templateRulesetMapping is not None:
ruleset = ploneCacheSettings.templateRulesetMapping.get(name, None)
if ruleset is not None:
return ruleset
ruleset = ploneCacheSettings.templateRulesetMapping.get(name, None)
if ruleset is not None:
return ruleset

# 4. Find the parent of the published object
parent = getattr(self.published, "__parent__", None)
Expand All @@ -97,13 +96,11 @@ def __call__(self):

# 4.1.2.1. Look up the parent type in the content type
# mapping
if ploneCacheSettings.contentTypeRulesetMapping is not None:
ruleset = ploneCacheSettings.contentTypeRulesetMapping.get(
parentPortalType,
None,
)
if ruleset is not None:
return ruleset
ruleset = ploneCacheSettings.contentTypeRulesetMapping.get(
parentPortalType, None
)
if ruleset is not None:
return ruleset

# 4.1.2.2. Look up a ruleset on the parent object and
# return
Expand Down
52 changes: 24 additions & 28 deletions plone/app/caching/operations/etags.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from plone.app.caching.operations.utils import getLastModifiedAnnotation
from Products.CMFCore.utils import getToolByName
from zope.component import adapter
from zope.component.hooks import getSite
from zope.component import queryMultiAdapter
from zope.interface import implementer
from zope.interface import Interface
Expand All @@ -21,14 +22,11 @@ def __init__(self, published, request):
self.request = request

def __call__(self):
context = getContext(self.published)
portal_state = queryMultiAdapter(
(context, self.request), name="plone_portal_state"
)
if portal_state is None:
tool = queryMultiAdapter((getSite(), self.request), name="portal_membership")
if tool is None:
return None

member = portal_state.member()
member = tool.getAuthenticatedMember()
if member is None:
return None

Expand All @@ -47,21 +45,18 @@ def __init__(self, published, request):
self.request = request

def __call__(self):
context = getContext(self.published)
portal_state = queryMultiAdapter(
(context, self.request), name="plone_portal_state"
)
if portal_state is None:
tool = queryMultiAdapter((getSite(), self.request), name="portal_membership")
if tool is None:
return None

if portal_state.anonymous():
if bool(tool.isAnonymousUser()):
return "Anonymous"

member = portal_state.member()
member = tool.getAuthenticatedMember()
if member is None:
return None

return ";".join(sorted(member.getRolesInContext(context)))
return ";".join(sorted(member.getRolesInContext(getContext(self.published))))


@implementer(IETagValue)
Expand Down Expand Up @@ -91,14 +86,19 @@ def __init__(self, published, request):
self.request = request

def __call__(self):
language = self.request.get("LANGUAGE", None)
if language:
return language
context = getContext(self.published)
language = aq_inner(context).Language()
if language:
return language
portal_state = queryMultiAdapter(
(context, self.request), name="plone_portal_state"
)
if portal_state is None:
return None

return portal_state.language()
return portal_state.default_language()


@implementer(IETagValue)
Expand Down Expand Up @@ -132,11 +132,10 @@ def __init__(self, published, request):

def __call__(self):
context = getContext(self.published)
tools = queryMultiAdapter((context, self.request), name="plone_tools")
if tools is None:
catalog = getToolByName(context, "portal_catalog", None)
if catalog is None:
return None

return str(tools.catalog().getCounter())
return str(catalog.getCounter())


@implementer(IETagValue)
Expand All @@ -157,7 +156,7 @@ def __call__(self):
)
if context_state is None:
return None
return str(int(context_state.is_locked()))
return "1" if context_state.is_locked() else "0"


@implementer(IETagValue)
Expand Down Expand Up @@ -197,13 +196,10 @@ def __init__(self, published, request):
self.request = request

def __call__(self):
context = getContext(self.published)
portal_state = queryMultiAdapter(
(context, self.request), name="plone_portal_state"
)
if portal_state is None:
tool = queryMultiAdapter((getSite(), self.request), name="portal_membership")
if tool is None:
return None
if portal_state.anonymous():
if bool(tool.isAnonymousUser()):
return None
return "{}{}".format(time.time(), random.randint(0, 1000))

Expand All @@ -220,4 +216,4 @@ def __init__(self, published, request):
self.request = request

def __call__(self):
return self.request.get("__cp") and "1" or "0"
return "1" if self.request.get("__cp") or "0"
18 changes: 6 additions & 12 deletions plone/app/caching/operations/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,7 @@ def cachedResponse(
response.setHeader(k, v)

response.setHeader("X-RAMCache", PAGE_CACHE_KEY, literal=1)

if not gzip:
response.enableHTTPCompression(request, disable=True)
else:
response.enableHTTPCompression(request)
response.enableHTTPCompression(request, disable=not gzip)

return body

Expand Down Expand Up @@ -393,13 +389,11 @@ def isModified(request, etag=None, lastModified=None):

etagMatched = True

"""
If a site turns off etags after having them on, the pages previously
served will return an If-None-Match header, but the site will not be
configured for etags. In this case, force a refresh to load the
latest headers. I interpret this as the spec rule that the
etags do NOT match, and therefor we must not return a 304.
"""
# If a site turns off etags after having them on, the pages previously
# served will return an If-None-Match header, but the site will not be
# configured for etags. In this case, force a refresh to load the
# latest headers. I interpret this as the spec rule that the
# etags do NOT match, and therefor we must not return a 304.
if ifNoneMatch and etag is None:
return True

Expand Down

2 comments on commit ed4d6b0

@mauritsvanrees
Copy link
Member

@mauritsvanrees mauritsvanrees commented on ed4d6b0 Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jensens
This causes various test failures, also after the next two commits. Jenkins is still green because p.a.caching is not checked out. Can you look into it?
For example:

Failure in test test_anon_only (plone.app.caching.tests.test_operation_parameters.TestOperationParameters)
Traceback (most recent call last):
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 60, in testPartExecutor
    yield
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 676, in run
    self._callTestMethod(testMethod)
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 633, in _callTestMethod
    method()
  File "/Users/maurits/community/plone-coredev/6.0/src/plone.app.caching/plone/app/caching/tests/test_operation_parameters.py", line 72, in test_anon_only
    self.assertEqual("plone.content.itemView", browser.headers["X-Cache-Rule"])
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 912, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: 'plone.content.itemView' != None

and

Failure in test test_CatalogCounter (plone.app.caching.tests.test_etags.TestETags)
Traceback (most recent call last):
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 60, in testPartExecutor
    yield
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 676, in run
    self._callTestMethod(testMethod)
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 633, in _callTestMethod
    method()
  File "/Users/maurits/community/plone-coredev/6.0/src/plone.app.caching/plone/app/caching/tests/test_etags.py", line 289, in test_CatalogCounter
    self.assertEqual("10", etag())
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 912, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/Users/maurits/.pyenv/versions/3.8.5/lib/python3.8/unittest/case.py", line 905, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: '10' != None

End result:

Total: 186 tests, 10 failures, 2 errors, 0 skipped in 17.097 seconds.

It could be that this is only a problem in tests. It might be that some stuff is mocked and it interferes with your work. Certainly -m test_etags finishes very quickly so it likely is a unittest without any layer setup. Maybe this then fails:

queryMultiAdapter((getSite(), self.request), name="portal_membership")

Just a guess.

@jensens
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it into my queue, I have anyway some open tasks in here.

Please sign in to comment.