From b472e5e648c3e004210ae3e88d162753d31045fb Mon Sep 17 00:00:00 2001
From: Marti Raudsepp <marti@juffo.org>
Date: Tue, 19 Jun 2018 18:41:12 +0300
Subject: [PATCH] Cache parsed certificate instead of re-parsing for each field

Use @cached_property decorator to cache the results of parse_certificate().

This significantly cuts down on the number of times certs need to be
parsed for a list view.
---
 lemur/certificates/models.py     | 39 +++++++++++++++-----------------
 lemur/tests/test_certificates.py | 17 ++++++++++++++
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/lemur/certificates/models.py b/lemur/certificates/models.py
index f88eda0a..ad6949b1 100644
--- a/lemur/certificates/models.py
+++ b/lemur/certificates/models.py
@@ -21,6 +21,7 @@ from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy import event, Integer, ForeignKey, String, PassiveDefault, func, Column, Text, Boolean
 
 from sqlalchemy_utils.types.arrow import ArrowType
+from werkzeug.utils import cached_property
 
 import lemur.common.utils
 
@@ -142,7 +143,8 @@ class Certificate(db.Model):
     sensitive_fields = ('private_key',)
 
     def __init__(self, **kwargs):
-        cert = lemur.common.utils.parse_certificate(kwargs['body'])
+        self.body = kwargs['body'].strip()
+        cert = self.parsed_cert
 
         self.issuer = defaults.issuer(cert)
         self.cn = defaults.common_name(cert)
@@ -159,7 +161,6 @@ class Certificate(db.Model):
                 defaults.certificate_name(self.cn, self.issuer, self.not_before, self.not_after, self.san), self.serial)
 
         self.owner = kwargs['owner']
-        self.body = kwargs['body'].strip()
 
         if kwargs.get('private_key'):
             self.private_key = kwargs['private_key'].strip()
@@ -184,40 +185,39 @@ class Certificate(db.Model):
         for domain in defaults.domains(cert):
             self.domains.append(Domain(name=domain))
 
+    @cached_property
+    def parsed_cert(self):
+        assert self.body, "Certificate body not set"
+        return lemur.common.utils.parse_certificate(self.body)
+
     @property
     def active(self):
         return self.notify
 
     @property
     def organization(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return defaults.organization(cert)
+        return defaults.organization(self.parsed_cert)
 
     @property
     def organizational_unit(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return defaults.organizational_unit(cert)
+        return defaults.organizational_unit(self.parsed_cert)
 
     @property
     def country(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return defaults.country(cert)
+        return defaults.country(self.parsed_cert)
 
     @property
     def state(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return defaults.state(cert)
+        return defaults.state(self.parsed_cert)
 
     @property
     def location(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return defaults.location(cert)
+        return defaults.location(self.parsed_cert)
 
     @property
     def key_type(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        if isinstance(cert.public_key(), rsa.RSAPublicKey):
-            return 'RSA{key_size}'.format(key_size=cert.public_key().key_size)
+        if isinstance(self.parsed_cert.public_key(), rsa.RSAPublicKey):
+            return 'RSA{key_size}'.format(key_size=self.parsed_cert.public_key().key_size)
 
     @property
     def validity_remaining(self):
@@ -229,13 +229,11 @@ class Certificate(db.Model):
 
     @property
     def subject(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return cert.subject
+        return self.parsed_cert.subject
 
     @property
     def public_key(self):
-        cert = lemur.common.utils.parse_certificate(self.body)
-        return cert.public_key()
+        return self.parsed_cert.public_key()
 
     @hybrid_property
     def expired(self):
@@ -300,8 +298,7 @@ class Certificate(db.Model):
         }
 
         try:
-            cert = lemur.common.utils.parse_certificate(self.body)
-            for extension in cert.extensions:
+            for extension in self.parsed_cert.extensions:
                 value = extension.value
                 if isinstance(value, x509.BasicConstraints):
                     return_extensions['basic_constraints'] = value
diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py
index 98283a9d..3368f88d 100644
--- a/lemur/tests/test_certificates.py
+++ b/lemur/tests/test_certificates.py
@@ -9,9 +9,11 @@ from cryptography import x509
 from cryptography.hazmat.backends import default_backend
 from marshmallow import ValidationError
 from freezegun import freeze_time
+from mock import patch
 
 from lemur.certificates.service import create_csr
 from lemur.certificates.views import *  # noqa
+from lemur.common import utils
 from lemur.domains.models import Domain
 
 
@@ -66,6 +68,21 @@ def test_get_certificate_primitives(certificate):
         assert len(primitives) == 24
 
 
+def test_certificate_output_schema(session, certificate, issuer_plugin):
+    from lemur.certificates.schemas import CertificateOutputSchema
+
+    # Clear the cached attribute first
+    if 'parsed_cert' in certificate.__dict__:
+        del certificate.__dict__['parsed_cert']
+
+    # Make sure serialization parses the cert only once (uses cached 'parsed_cert' attribute)
+    with patch('lemur.common.utils.parse_certificate', side_effect=utils.parse_certificate) as wrapper:
+        data, errors = CertificateOutputSchema().dump(certificate)
+        assert data['issuer'] == 'Example'
+
+    assert wrapper.call_count == 1
+
+
 def test_certificate_edit_schema(session):
     from lemur.certificates.schemas import CertificateEditInputSchema