Merge pull request #14 from TeskaLabs/Refactoring/Review-300123

Refactoring/review 300123

Author: mithunbharadwaj

README.md

@@ -60,15 +60,15 @@ https://teskalabs.github.io/asab-iris/
 
 **Templates used for emailing must be stored under**
 ```
-/Templates/Mails/
+/Templates/Email/
 ```
 
 **Templates used by Slack must be stored under**
 ```
 /Templates/Slack/
 ```
 
-**Templates used for other porpose must be stored under**
+**Templates used for other purpose must be stored under**
 ```
 /Templates/General/
 ```

asabiris/app.py

@@ -15,7 +15,7 @@
 from .output.slack import SlackOutputService
 
 # orchestrators.
-from .orchestration.sendmail import SendMailOrchestrator
+from .orchestration.sendemail import SendEmailOrchestrator
 from .orchestration.render import RenderReportOrchestrator
 
 from .handlers.kafkahandler import KafkaHandler
@@ -69,7 +69,7 @@ def __init__(self, args=None):
 		self.SlackOutputService = SlackOutputService(self)
 
 		# Orchestrators
-		self.SendMailOrchestrator = SendMailOrchestrator(self)
+		self.SendEmailOrchestrator = SendEmailOrchestrator(self)
 		self.RenderReportOrchestrator = RenderReportOrchestrator(self)
 		self.SendSlackOrchestrator = SendSlackOrchestrator(self)
 

asabiris/handlers/kafkahandler.py

@@ -89,7 +89,7 @@ async def dispatch(self, msg):
 			L.warning("Message type '{}' not implemented.".format(msg_type))
 
 	async def send_email(self, json_data):
-		await self.App.SendMailOrchestrator.send_mail(
+		await self.App.SendEmailOrchestrator.send_email(
 			email_to=json_data["to"],
 			body_template=json_data["body"]["template"],
 			email_cc=json_data.get("cc", []),  # Optional

asabiris/handlers/webhandler.py

@@ -26,7 +26,7 @@ def __init__(self, app):
 
 		web_app = app.WebContainer.WebApp
 		web_app.router.add_put(r"/send_email", self.send_email)
-		web_app.router.add_put(r"/send_mail", self.send_email)  # This one is for backward compatiblity
+		web_app.router.add_put(r"/send_mail", self.send_email)  # This one is for backward compatibility
 		web_app.router.add_put(r"/render", self.render)
 		web_app.router.add_put(r"/send_slack", self.send_alert)
 
@@ -55,24 +55,24 @@ async def send_email(self, request, *, json_data):
 			"subject": "Lufthansa Hiest",
 			"from": "Jimmy.Conway@Goodfellas.com",
 			"body": {
-				"template": "test.md",
+				"template": "/Templates/Emails/test.md",
 				"params": {
 					"Name": "Toddy Siciro"
 			}
 		},
 		"attachments": [
 			{
-			"template": "test.md",
+			"template": "/Templates/Emails/hello.html",
 			"params": {
 				"Name": "Michael Corleone"
 				},
 			"format": "pdf",
-			"filename": "Made.pdf"
+			"filename": "Alert.pdf"
 			}]
 		}
 
 		```
-		Attached will be retrieved from request.conent when rendering the email is not required.
+		Attached will be retrieved from request.content when rendering the email is not required.
 
 		Example of the email body template:
 		```
@@ -93,7 +93,7 @@ async def send_email(self, request, *, json_data):
 		"""
 
 		try:
-			await self.App.SendMailOrchestrator.send_mail(
+			await self.App.SendEmailOrchestrator.send_email(
 				email_to=json_data["to"],
 				body_template=json_data["body"]["template"],
 				email_cc=json_data.get("cc", []),  # Optional
@@ -103,7 +103,6 @@ async def send_email(self, request, *, json_data):
 				body_params=json_data["body"].get("params", {}),  # Optional
 				attachments=json_data.get("attachments", []),  # Optional
 			)
-
 		except KeyError as e:
 			raise aiohttp.web.HTTPNotFound(text="{}".format(e))
 
@@ -128,7 +127,7 @@ async def send_alert(self, request, *, json_data):
 		{
 			"type": "slack",
 			"body": {
-				"template": "test.md",
+				"template": "/Templates/Slack/alert.md",
 				"params": {
 					"Name": "Toddy Siciro"
 			}
@@ -154,7 +153,7 @@ async def render(self, request):
 		This endpoint renders request body into template based on the format specified.
 		Example:
 		```
-		localhost:8080/render?format=pdf&template=test.md
+		localhost:8080/render?format=pdf&template=/Templates/General/test.md
 
 		format: pdf/html
 
@@ -176,11 +175,12 @@ async def render(self, request):
 		template_data = await request.json()
 
 		# Render a body
-		html = await self.render(template, template_data)
+		html = await self.App.RenderReportOrchestrator.render(template, template_data)
+
 		# get pdf from html if present.
 		if fmt == 'pdf':
 			content_type = "application/pdf"
-			pdf = self.App.HtmlToPdfService.format(html)
+			pdf = self.App.PdfFormatterService.format(html)
 		elif fmt == 'html':
 			content_type = "text/html"
 		else:

asabiris/orchestration/render.py

@@ -15,7 +15,6 @@ class RenderReportOrchestrator(object):
 	def __init__(self, app):
 		# formatters
 		self.JinjaService = app.get_service("JinjaService")
-		self.HtmlToPdfService = app.get_service("HtmlToPdfService")
 		self.MarkdownToHTMLService = app.get_service("MarkdownToHTMLService")
 
 	async def render(self, template, params):
@@ -26,7 +25,9 @@ async def render(self, template, params):
 		# - primarily use absolute path - starts with "/"
 		# - if absolute path is used, check it start with "/Templates"
 		# - if it is not absolute path, it is file name - assume it's a file in Templates folder
-		assert template.startswith("/Templates"), "Template must be stored in /Templates directory"
+		# templates must be stores in /Templates/General
+		if not template.startswith("/Templates/General"):
+			raise ValueError("Template must be stored in /Templates/General directory")
 
 		html = await self.JinjaService.format(template, params)
 		_, extension = os.path.splitext(template)

asabiris/orchestration/sendmail.py asabiris/orchestration/sendemail.py

@@ -13,7 +13,7 @@
 #
 
 
-class SendMailOrchestrator(object):
+class SendEmailOrchestrator(object):
 
 	def __init__(self, app):
 
@@ -25,7 +25,7 @@ def __init__(self, app):
 		# output
 		self.SmtpService = app.get_service("SmtpService")
 
-	async def send_mail(
+	async def send_email(
 		self, *,
 		email_to,
 		email_from=None,
@@ -67,14 +67,11 @@ async def send_mail(
 				# If content-type is application/octet-stream we assume there is additional attachments in request else we raise bad-request error.
 				template = a.get('template', None)
 
-				# - primarily use absolute path - starts with "/"
-				# - if absolute path is used, check it start with "/Templates"
-				# - if it is not absolute path, it is file name - assume it's a file in Templates folder
-
 				if template is not None:
 					params = a.get('params', {})
-
-					assert template.startswith("/Templates"), "Template must be stored in /Templates directory"
+					# templates must be stores in /Templates/Emails
+					if not template.startswith("/Templates/Email"):
+						raise ValueError("Template must be stored in /Templates/Email directory")
 
 					# get file-name of the attachment
 					file_name = self.get_file_name(a)
@@ -83,10 +80,10 @@ async def send_mail(
 					# get pdf from html if present.
 					fmt = a.get('format', 'html')
 					if fmt == 'pdf':
-						result = self.HtmlToPdfService.format(jinja_output)
+						result = self.HtmlToPdfService.format(jinja_output).read()
 						content_type = "application/pdf"
 					elif fmt == 'html':
-						result = jinja_output
+						result = jinja_output.encode("utf-8")
 						content_type = "text/html"
 					else:
 						raise ValueError("Invalid/unknown format '{}'".format(fmt))
@@ -124,7 +121,9 @@ async def render(self, template, params):
 
 		jinja_output will be used for extracting subject.
 		"""
-		assert template.startswith("/Templates"), "Template must be stored in /Templates directory"
+		# templates must be stores in /Templates/Emails
+		if not template.startswith("/Templates/Email"):
+			raise ValueError("Template must be stored in /Templates/Email directory")
 
 		try:
 			jinja_output = await self.JinjaService.format(template, params)

asabiris/orchestration/sendslack.py

@@ -33,7 +33,10 @@ async def send_to_slack(self, msg):
 		# - primarily use absolute path - starts with "/"
 		# - if absolute path is used, check it start with "/Templates"
 		# - if it is not absolute path, it is file name - assume it's a file in Templates folder
-		assert body['template'].startswith("/Templates"), "Template must be stored in /Templates directory"
+
+		# templates must be stores in /Templates/Slack
+		if not body['template'].startswith("/Templates/Slack"):
+			raise ValueError("Template must be stored in /Templates/Slack directory")
 
 		body["params"] = body.get("params", {})
 		output = await self.JinjaService.format(body["template"], body["params"])

library/Templates/Email/hello.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html lang="EN">
+<title>Sample subject</title>
+<body>
+	<p>Hello,</p>
+	<p style="color: green">I'm very simple message by {{name}}!</p>
+	<p>Your,<br/>ASAB Iris</p>
+</body>
+</html>

library/Templates/hello.md library/Templates/Email/hello.md

@@ -1,7 +1,7 @@
 SUBJECT: Hello from ASAB Iris
 Hello,
 
-I'm very simple message!
+I'm very simple message by {{name}}!
 
 Your,  
 ASAB Iris

library/Templates/hello.html library/Templates/General/hello.html

@@ -2,7 +2,7 @@
 <html lang="EN">
 <body>
 	<p>Hello,</p>
-	<p style="color: green">I'm very simple message!</p>
+	<p style="color: green">I'm very simple message by {{name}}!</p>
 	<p>Your,<br/>ASAB Iris</p>
 </body>
 </html>

library/Templates/General/hello.md

@@ -0,0 +1,7 @@
+SUBJECT: Hello from ASAB Iris
+Hello,
+
+I'm very simple message by {{name}}!
+
+Your,  
+ASAB Iris

library/alert.md library/Templates/Slack/alert.md

@@ -1,5 +1,4 @@
-Ooops. Something wrong happened.
-
+This is a sample message.
 
 {{ message }}