[fix] Avoid redundant SQL write query during CA/Cert creation generation

[SitaGanesh]

Checklist

• I have read the OpenWISP Contributing Guidelines.
• I have manually tested the changes proposed in this pull request.
• I have written new test cases for new code and/or updated existing tests for changes to existing code.
• I have updated the documentation.

Reference to Existing Issue

Closes #120.

Description of Changes

Modified a test to the test_new function in test_cert.py to measure the number of database queries before and after the optimization. Ensuring that there should be at least 1 less query.

Screenshot

[SitaGanesh]

Sir @nemesifier, I would like to contribute to this organization, I believe I have made changes correctly as you have mentioned. I want to move forward in the next challenges that you assign me, I will do my best to tackle them by facing them. Could you guide me on what to do further to prove myself?

[nemesifier]

Thank you for your patience @SitaGanesh!

We've been busy completing some projects before Christmas and the End of the year and couldn't look into this yet.

Please see my comments below.

Please also ensure tests and QA checks pass locally, read the contributing guidelines for more info on QA checks and tests:
https://openwisp.io/docs/dev/developer/contributing.html

[SitaGanesh]

Thank you @nemesifier for the feedback, and I was awaiting your response

I will review and try to modify the test to utilize assertNumQueries. I apologize for using my own method to implement the code, and I will ensure that it minimizes unnecessary alterations.

Let me repeat what you said for clarification:

• Update the test case to add the assertNumQueries assertion and manually verify that the number of queries has decreased.

• The generate variable and the related logic in my code may no longer be necessary, so I need to remove them completely.

Thank you for pointing me to the contributing guidelines and the references for assertNumQueries. I’ll review them carefully, and I need some time, sir, to accomplish this task.
Please confirm if I understood the points correctly.

[nemesifier]

I believe your understanding is correct, take your time. Ping us in the dev chat when ready. Thanks.

[SitaGanesh]

Hi sir @nemesifier, this is the PR #170 which solves the both cases. I hope I have created a successful PR with all the requirements ment for consideration. If there will be any modification in the code please let me know. I will alter that them.

[SitaGanesh]

I have made the suggested changes by removing the generate variable and simplifying the save method as follows:

def save(self, *args, **kwargs): if not self.pk and not self.certificate and not self.private_key: if not self.serial_number: self.serial_number = self._generate_serial_number() self._generate() super().save(*args, **kwargs)
the test cases goes here:-

I have made the suggested changes by using assertNumQueries as follows:

def test_new(self): with self.assertNumQueries(3): # 3 queries to be made cert = self._create_cert() self.assertNotEqual(cert.certificate, '') self.assertNotEqual(cert.private_key, '') x509 = cert.x509 self.assertEqual(x509.get_serial_number(), cert.serial_number) subject = x509.get_subject()

The test cases goes here:-

[coveralls]

coverage: 99.567% (-0.005%) from 99.572%
when pulling ca31ca0 on SitaGanesh:add-an-assertion-in-the-tests
into da564dc on openwisp:master.

[nemesifier]

I made some simplifications in ca31ca0 and now is ready, thanks @SitaGanesh.

The merge-base changed after approval.

[SitaGanesh]

Thank you, @nemesifier

I'm really happy to see the PR merged!
Also, thank you for making the final simplifications—I’ve realized that I usually get stuck at those parts, which sometimes makes my initial attempts less successful. But I’m working on improving that, and I can already feel the difference as I try not to repeat the same mistakes.

Right now, I'm exploring the issues on the next release board and feeling enthusiastic to tackle more! Looking forward to learning more and contributing further.