Decentralise Connection

[maestro-1]

This PR consists of a number of changes

• reformatting project using black

• Refactor Connection, removing its coupling to the fastapi-mail initialisation. Now custom connections can be written and passed into the send_message method. The connection argument can now serve as a proxy for custom connection, so long as they align with the BaseConnection abstract class. If no custom connection class is implemented, the fastapi-mail class tries to guess the most appropriate connection class to use based on configuration.

  async def send_message( self, message: MessageSchema, template_name: str = None, connection: BaseConnection = None, ):

     if connection is None:
         connection = self._guess_connection(
             self.config.SUPPRESS_SEND, self.config.MAIL_DEBUG
          )

• Depreciation of record_message method in favour of the more async friendly async_record_messages(tested and works)

• New Feature: The Dev Server Connection. This allows for local development and debugging of mail on a local machine. Simply set MAIL_DEBUG=1 and begin the server from a terminal with python fastapi_mail

[sabuhish]

Hi, @maestro-1 thank you for your PR. First of all sorry for being late, I could take up my head from projects. I finally got some time and wanted to take a look. Please accept my apologize.

I have reviewed the code, I have some comments that I would like to discuss. As I see you split the connection into three parts. After carefully reading, there are some issues that I realized while reading it, wanted to list them:

• InvalidConnectionObject was written but it was not imported in the fastapimail.py, have you had a chance to test it. This would cause problems while running.
• On line 85 of the connection.py login established without self keyword
• In the init.py inside none of the new connection classes are imported which module relies on this file classes or functions to be imported
• Plus setting MAIL_DEBUG automatically starts connecting to debug server, which might not be required at all, maybe user wants to debug messages to see in their prod environment
• In the TestConnection class settings not used at all, why do we keep this argument then?
• I see connection classes check is made more than one time, once the class initialized second inside send_message method. If we have already defined it once, I think worth to use it through the class attribute
• _guess_connection is a bit seems to be tricky one, again I am repeating we cannot relay on the given arguments and decide by ourself.
• Lastly, does it really worth having many connection classes and having checks, because they almost doing the same work with only a few changes it has.

Please next time let's open a discussion if we would like to bring new changes, in order to be on the same page and add new features. By this way, we can follow up with each other and know what changes are coming, and what we are realizing. Thanks again.

[maestro-1]

Hi, @maestro-1 thank you for your PR. First of all sorry for being late, I could take up my head from projects. I finally got some time and wanted to take a look. Please accept my apologize.

I have reviewed the code, I have some comments that I would like to discuss. As I see you split the connection into three parts. After carefully reading, there are some issues that I realized while reading it, wanted to list them:

* InvalidConnectionObject was written but it was not imported in the fastapimail.py, have you had a chance to test it. This would cause problems while running.

* On line 85 of the connection.py login established without self keyword

* In the **init**.py inside none of the new connection classes are imported which module relies on this file classes or functions to be imported

* Plus setting MAIL_DEBUG automatically starts connecting to debug server, which might not be required at all, maybe user wants to debug messages to see in their prod environment

* In the TestConnection class settings not used at all, why do we keep this argument then?

* I see connection classes check is made more than one time, once the class initialized second inside send_message method. If we have already defined it once, I think worth to use it through the class attribute

* _guess_connection is a bit seems to be tricky one,  again I am repeating we cannot relay on the given arguments and decide by ourself.

* Lastly, does it really worth having many connection classes and having checks, because they almost doing the same work with only a few changes it has.

Please next time let's open a discussion if we would like to bring new changes, in order to be on the same page and add new features. By this way, we can follow up with each other and know what changes are coming, and what we are realizing. Thanks again.

Thanks, @sabuhish for the review.

• good catch, don't know how come I missed the missing "self". It appears some of my updates got overridden without me noticing which would explain why some imports are missing(will make a PR to fix that). Cause I tested and ensured everything worked as is
• The reason settings are passed into the TestConnection is just for continuity, in the case at any point we decide to add any item to the test. Like testing configurations in Connections. This ideally was to be my next PR after this gets merged
• _guess_connection is just a bit used to determine the best connection to use based on configuration. I believe it helps to keep the library working as is, but makes run for possible extensions if need be. And it helps with flexibility. So changing the configuration affects what connection gets used. This means that developers don't have to make major tweaks beyond configuration details to get what they want be it testing, development or production work. However, it works under the hood so no one needs to worry about it. Also tested
• Currently, the MAIL_DEBUG option doesn't do anything. I just made it functional, didn't see the need of having a configuration value that had no purpose when it could. If they would like to the default they wouldn't have to set the MAIL_DEBUG to true. I'll add it to the docs
• Finally, there are a couple of reasons I separated the Connection into several classes. The main one was that it was too rigid so I couldn't add any extra functionalities without making it unnecessarily convoluted. In splitting it into several classes, I was able to make testing mails sent part of the connections(TestConnection) and not the FastAPI-mail class. I was able to add the functionality of a local server(DevConnection), so now you can send emails and have them appear on your machine instead of sending several dummy emails to an actual account or sending emails to non-existent accounts of which you cannot know for sure whether or not they delivered and they would be delivered instantly so you can see what your email code is sending instantly. While ProdConnection is what fastapi-mail ran exclusively on before and remains the default. Also, this allows fastapi-mail a chance to be extended such that developers can create their custom connections if they see fit. The few changes between each class are actually much larger than at first glance, making them key differences.

There seem to be some changes I made and forgot to push, which I will do shortly, including the updates I made to ensure the connection object isn't checked multiple time. I will be sure to make a discussion next time I plan on implementing such large changes. Note that these changes are very much compatible and do not break the way fastapi-mail currently works

[maestro-1]

@sabuhish Do not know if you have gotten a chance to go through my reply. I have just gotten time to send in updates for some of the concerns you raise which I agreed with: the missing self and redundancy of the multiple checks for connection in the fastapi-mail class. I also took the liberty of adding the test case for testing login in the TestConnection class(the justification for why I added settings as a parameter to that class in the first place). As for your concerns with the MAIL_DEBUG parameter in the settings, I am certain this is not a problem, this is the use of having it in the documentation. I plan to update the documentation with all the new features as soon as this PR gets approved I also addressed your concerns about importations. Now, for the most part, I don't expect anyone to want to use those classes outside of the fastapi-mail class as it is meant to work under the hood. However, I decided that nothing will be lost in by adding it to the init file.