flashky / notifier-packt Goto Github PK

Use Spring Boot Configuration or something similar to keep a centralized configuration.

Now I'm starting to use much more properties but as the application is running on docker, I cannot hotswap the properties.

Some properties as the notification.*.enabled would require restart anyway unless I modify the code.

Project src should start at parent folder.

I should also test whether Eclipse is capable of generate the maven project or not.

mail.subject -> mail.daily-offer.subject

Class:
NotifierPacktApplication

Method:
getRestTemplate()

Currently there is no check of whether the proxy host and port are currently setup. Add a simple validation to check it is not null.

Add a MAIL_PASSWORD environment variable to the SimpleJavaMail password property.
The environment variable can have either a decrypted or encrypted value, but it is mandatory to be set.

It will also be possible to override via external application.yml file.

Whenever an url has whitespaces, gmail substitute it for "+" signs.

Example:

https://d255esdrn735hr.cloudfront.net/sites/default/files/3758OS_4829_Python GUI Programming Cookbook_0.jpg

Becomes:

https://d255esdrn735hr.cloudfront.net/sites/default/files/3758OS_4829_Python+GUI+Programming+Cookbook_0.jpg

This is not a problem for a href links.

However, onimg tags, the url is proxied:
https://ci5.googleusercontent.com/proxy/e2_XcRaS6a2QYUzi-O7d6CWdQiBcW0swB2mMdbH-Q-Tpt_hNqUOONQvVKxLwJs1Owd6LwNlZffRN4ZfO7JEk-AllRlxSEfP-Sjyj12JdObNABivJFcb70R0CPBYK6jc2DitdhjMtiKUN4kVbf-tNitl2Qyx9jHD9-CUV40jR=s0-d-e1-ft#https://d255esdrn735hr.cloudfront.net/sites/default/files/3758OS_4829_Python+GUI+Programming+Cookbook_0.jpg

This proxy doesn't work properly, maybe because of the "+" signs:

https://d255esdrn735hr.cloudfront.net/sites/default/files/3758OS_4829_Python%20GUI%20Programming%20Cookbook_0.jpg

This exception is triggered by an indetermined error on JSoup.

IOException has many Exception subclasses, check:
https://docs.oracle.com/javase/7/docs/api/java/io/IOException.html?is-external=true

The most important seem to be HttpStatusException, a different task for this one should be applied.

Maybe the application should have a retry system.

Now we will have two different emails. It will be nice to use a much more descriptive name the old and the new one.

Related to task #20 , add a descriptive icon before each section.

Currently there is no exception handling.
The only exception being handled is the IOException from JSoup, and it is not even doing anything but writing into a log.

There is a number of situation which could generate an exception, so I'll create separated issues for each one of them.

Currently I have a Jenkinsfile (#17), but it only builds a docker image.

I need additional steps on the pipeline to restart the container:

• Stop old version container.
• Start new version container.

• ProxyProperty must become ProxyProperties and will be a standalone properties class.
• PacktProperties will be removed.
• Update any classes using PacktProperties to use the specific properties classes.

There not to many messages, but they a bit of a mess.
Besides that, messages shouldn not be categorized by logger level (info, warn, error). A info message can later become a warn message, for example, so it shouldn't be coupled to the naming convention.

Currently EmailService receives a PacktFreeOffer, so it is tightly coupled to that domain object.

The following is proposed to loose the coupling:

• Create a specific email model class EmailData.
• Create a specific EmailService interface with method:
  • void send(EmailData)
• Modify EmailService:
  • Rename it to SimpleJavaMailService.
  • Remove NotificationListener interface and add EmailService interface instead.
  • Adapt the current notify(PacktFreeOffer) method to send(EmailData) method.
• Create an adapter class EmailNotifier which will implement NotificationListener interface. This class will send the object to the wrapped inside the EmailData class to the SimpleJavaMailService .
• Modify NotifierPacktApplication class to initialize the new EmailListener there instead of the old one.

This task should be priotirized first, as the old task should continue working as before, and then we can do the following tasks.

Currently, the json object is being mapped to the dto using a private method on ApiCheckoutService.

Maybe it should be moved to a Mapper class.

The contract has been there for a long time, but now we need to implement it for the weekly offers.

Should be true, but it is 'false'.

https://icons8.com/license

Currently I have a Jenkinsfile (#17), but it only builds a docker image.

I need additional steps on the pipeline:

• Push to local registry, or Docker Hub.
• Restart the container using the new version (#28).

API endpoints are being calculated by some ApiCheckoutService private methods.

Endpoint: offers

	private String getOfferListEndpoint() {

		LocalDate startDate = LocalDate.now();
		LocalDate endDate = LocalDate.now().plusDays(1);
		
		return Url.SERVICES.getUriComponentsBuilder()
				.path(ApiPath.OFFERS.getPath())
				.queryParam("dateFrom", startDate.toString())
				.queryParam("dateTo", endDate.toString())
				.build().toString();
	}

This code only allows to retrieve offers between today and tomorrow, leaving some uncovered cases:

• Any books between today and a specified number of days.
• Any books between a specified date and a specified number of days.
• Any books between today and a specified date.
• Any books between two specified dates.

Analyze to find a much more suitable and reusable solution, but apply YAGNI principle.
Maybe no private method would be necessary, as the build is simple enough to repeat it multiple times. Or maybe add two method signatures:

private String getOfferListEndpoint(LocalDate startDate, int numberOfDays) {
...
}

private String getOfferListEndpoint(LocalDate startDate, LocalDate endDate) {
...
}

I find the first method much more useful.

Endpoint: book summary

	private String getSummaryEndpoint(Long productId) {
		
		return Url.STATIC.getUriComponentsBuilder()
				.path(ApiPath.SUMMARY.getPath())
				.buildAndExpand(productId).toString();
		
	}

The second endpoint is so much simple that seems redundant to use a private method to calculate it. Also, there aren't as much combinations for the input parameters, there is just one fixed parameter that acts as the book id.

The only reason we could need to refactor this code is that we may need to calculate this API url for additional checkouts (IE: weekly offers, all of them with its summary).

Packt has been updated so the CSS selectors not longer match.

Update application.yml to match the new values.

There is at least one known case of throwingIllegalArgumentException:

java.lang.IllegalArgumentException: Malformed URL: htttps://www.packtpub.com/packt/offers/free-learning
	at org.jsoup.helper.HttpConnection.url(HttpConnection.java:136)
	at org.jsoup.helper.HttpConnection.connect(HttpConnection.java:76)
	at org.jsoup.Jsoup.connect(Jsoup.java:73) 
       [...]
Caused by: java.net.MalformedURLException: unknown protocol: htttps
	at java.net.URL.<init>(URL.java:600)
	at java.net.URL.<init>(URL.java:490)
	at java.net.URL.<init>(URL.java:439)
	at org.jsoup.helper.HttpConnection.url(HttpConnection.java:134)

In this case, the IllegalArgumentException is caused by MalformedURLException.

The handler should:

1. Log the exception message as error.
2. Log the cause message as error.
3. Log the exception trace on a different error file.
4. Terminate the program.

The current version is old. It could be improved.

Bad credentials

When using an incorrect password for ```MAIL_PASSWORD``:

org.simplejavamail.mailer.internal.mailsender.MailSenderException: Third party error
[...]
Caused by: javax.mail.AuthenticationFailedException: 535-5.7.8 Username and Password not accepted. Learn more at

When using bad credentials:

1. Log as error.
2. Log cause as error.
3. Terminate program.

Unknown SMTP host

When using an invalid SMTP hostname:

simplejavamail:
  smtp:
    host: smtp.gmail.commmmmm

`
The following exception will ocurr:

org.simplejavamail.mailer.internal.mailsender.MailSenderException: Third party error
[...]
Caused by: com.sun.mail.util.MailConnectException: Couldn't connect to host, port: smtp.gmaill.com, 587; timeout -1;
[...]
Caused by: java.net.UnknownHostException: smtp.gmaill.com
[...]

Either the host server is down at that moment, or the hostname is incorrect.

First, we should do a test connection when the Mailer object is created. If that test worked and we get this exception later, we can confirm at 99,9% that the server is just down.

Therefore, we should handle as it follows:
When test connecting

See Spring Boot Admin.
Maybe use eureka + admin for keeping centralized logs and addicional info.

Currently, proxies are define like this:

service:
  proxy:
    # Do not add with 'http://' at the beginning!
    host: myproxy.host.com
    port: 8080

It must be:

proxy:
  # Do not add with 'http://' at the beginning!
  host: myproxy.host.com
  port: 8080

Modify also ProxyProperty class.

Currently, it is only logging the result of obtaining the offer:

• When no offer has been found.
• When the offer is the same as the previous execution.
• When a new offer has been detected.

Maybe I should add additional logging messages:

• When the scheduled task has started. That way, if the task gets stuck at some point, I can know that it did start running.
• When the scheduled task has finished. That way, I can control the execution time of the task.

Maybe I should use aspects for this.

Now that the model classes hashCode(), equals(Object) and toString() methods have been refactored (#22) there's some issue that has emerged.

For equals() method, the EqualsBuilder cannot do the following checks:

• Input object is null.
• Input object reference is the same as calling object reference.
• Input object class is not the same as calling object class.

This is done as it follows and it is the same for all classes:

if (this == obj)
    return true;
if (obj == null)
    return false;
if (getClass() != obj.getClass())
    return false;

Therefore, I believe that it would be great to have an additional utility class that receives two objects and performs does checkings. Somewhat like:

public static boolean checkReference(Object leftSide, Object rightSide);

The change is so small that I barel think it worths the time, but it could be done if I don't have anything else to do.

JSoup will throw HttpStatusException when get response is not 200 (HTTP Status OK):
https://jsoup.org/apidocs/org/jsoup/HttpStatusException.html

In this case, the Exception Handler should:

1. Log exception message as warning
2. Log the HTTP status code as warning.
3. Log the stack trace on a warning log file.
4. Continue execution.
5. (Optional): Retry later.

Currently I'm deploying new versions manually. I must create a jenkins pipeline using Jenkinsfile to automate version deploying.

There's a chance to not have any free offers.

For example, during the winter big sale, Packt didn't refresh the upcomming offers, so at 28th December, there were no offers left:

GET https://services.packtpub.com/free-learning-v1/offers?dateFrom=2018-12-28&dateTo=2018-12-29

This would result into the process not sending any email, which is okay, but the process didn't log anything to let the user know this.

Currently it is tightly coupled to PacktFreeOffer.
Instead of creating another different NotificationListener for the new weekly data, we could make the listener to be templated, and instantiate subclasses as a specific subtemplate type.

This is closely related to #53 (Refactor EmailService).

There are three cases of throwing this exception.

1. When MAIL_PASSWORD environment variable contains a valid encrypted password (surrounded by ENC(password)) but the ENCRYPT_MASTER_PASSWORD environment variable has not been setup or its value is wrong (user fault).
2. When MAIL_PASSWORD environment variable contains an invalid encrypted password, even if the ENCRYPT_MASTER_PASSWORD is right (user fault).
3. When any other bean dependency couldn't be injected while creating Spring context (programmer fault).

The ErrorHandler should be able to distinguish between the three cases, giving proper feedback to the user.

In mobile version, h3 headers seems to be slightly smaller than the text.

They should be little bit bigger, but not as much as desktop.

Before starting any additional tasks, merge the branches.

Version 2.2.0 will have two different tasks:

• Daily task for free offer of the day.
• Weekly task for the week upcoming free offers.

As of Single Responsability Principle, we should have two different tasks for this, so each of them can track its execution and notify its specific listeners.

Also, both of them have listeners, so both of them share the same listener related attribute and methods. This should be refactor into an abstract template class, and then, rename the old task.

Packt API allows to see the upcoming offers, not just the current day offer.

It could be interesting to generate a weekly email to summarize the upcomming offers.

SimpleJavaMail provides a feature for checking the current configuration works:
http://www.simplejavamail.org/#/features
http://www.simplejavamail.org/#section-connection-test

A connection test should be added to EmailService in the constructor to check the details are correct instead of waiting for a later error.

This would allow to simplify a little bit how the email service works, allowing to build the template engine outside and

The buttons don't fit well in mobile device. Improve this.

Currently, the model doesn't match the json data fair enough:
PacktFreeOffer is in fact, BookSummary data,

An offer contains a BookSummary + additional details as:

• Offer start date
• Offer end date

There are more details, but they are not as important as this.

Maybe PacktFreeOffer should contain this additional details, loaded from different jsons, or remodel this a bit.

Related to #47, there are two message property files for mails (spanish and english) but there is only one common message property file.

Add the missing english one.

Currently, both email buttons have hardcoded links on them.
They should be removed and use the right links.

Apache Commons lang builders are much more human readable, and it allows you to add additional lines easily, without needing to remove all the method and generate it again.

References

• https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/builder/HashCodeBuilder.html
• https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/builder/EqualsBuilder.html
• https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/builder/ToStringBuilder.html

License is described here:
http://www.simplejavamail.org/#/download
https://tldrlegal.com/license/apache-license-2.0-(apache-2.0)

Probably I will need to copy the license to this repository.

It will be pretty similar to the existing one, but will use a different bunch of notifiers (the NotificationListener currently is related to PacktFreeOffer and we need a different one).

Either by solving the warnings or finding out how to reduce the log level to just ERROR.

The new API provides many additional data.

Sample:

{
"title": "Mastering Python Networking",
"type": "books",
"coverImage": "https://d255esdrn735hr.cloudfront.net/sites/default/files/B03660.png",
"productId": "9781784397005",
"isbn13": "9781784397005",
"oneLiner": "Become an expert in implementing advanced, network-related tasks with Python.",
"pages": 446,
"publicationDate": "2017-06-28T05:48:00.000Z",
"length": "13 hours 22 minutes",
"about": "<p>This book begins with a review of the TCP/ IP protocol suite and a refresher of the core elements of the Python language. Next, you will start using Python and supported libraries to automate network tasks from the current major network vendors. We will look at automating traditional network devices based on the command-line interface, as well as newer devices with API support, with hands-on labs. We will then learn the concepts and practical use cases of the Ansible framework in order to achieve your network goals.</p>\r\n<p>We will then move on to using Python for DevOps, starting with using open source tools to test, secure, and analyze your network. Then, we will focus on network monitoring and visualization. We will learn how to retrieve network information using a polling mechanism, flow-based monitoring, and visualizing the data programmatically. Next, we will learn how to use the Python framework to build your own customized network web services.</p>\r\n<p>In the last module, you will use Python for SDN, where you will use a Python-based controller with OpenFlow in a hands-on lab to learn its concepts and applications. We will compare and contrast OpenFlow, OpenStack, OpenDaylight, and NFV. Finally, you will use everything you’ve learned in the book to construct a migration plan to go from a legacy to a scalable SDN-based network.</p>\r\n<p>&nbsp;</p>",
"learn": "<ul>\r\n<li>Review all the fundamentals of Python and the TCP/IP suite</li>\r\n<li>Use Python to execute commands when the device does not support the API or programmatic interaction with the device</li>\r\n<li>Implement automation techniques by integrating Python with Cisco, Juniper, and Arista eAPI</li>\r\n<li>Integrate Ansible using Python to control Cisco, Juniper, and Arista networks</li>\r\n<li>Achieve network security with Python</li>\r\n<li>Build Flask-based web-service APIs with Python</li>\r\n<li>Construct a Python-based migration plan from a legacy to scalable SDN-based network.</li>\r\n</ul>",
"features": "<ul>\r\n<li>Build the skills to perform all networking tasks using Python with ease</li>\r\n<li>Use Python for network device automation, DevOps, and software-defined networking</li>\r\n<li>Get practical guidance to networking with Python</li>\r\n</ul>",
"authors":[
"28853"
],
"shopUrl": "/networking-and-servers/mastering-python-networking",
"readUrl": "/book/networking-and-servers/9781784397005",
"category": "networking-and-servers",
"earlyAccess": false,
"available": true
}

I could add additional useful information to the email as:

• onLiner
• about
• learn
• features

Fix all things on README.md to improve code quality in codacy.