Objective: The objective of the program is to create a JIRA ticket using a REST API. It's working perfectly fine without any issue. However I am not sure if it's up to good Scala standards.
I would be delighted if someone could let me know what I can do to improve it.
import org.apache.http.HttpResponse
import org.apache.http.auth.UsernamePasswordCredentials
import org.apache.http.client.methods.HttpPost
import org.apache.http.entity.StringEntity
import org.apache.http.impl.auth.BasicScheme
import org.apache.http.impl.client.HttpClients
import org.apache.http.util.EntityUtils
object JIRAUtil{
private def getJiraID(response: HttpResponse): String ={
val entity = response.getEntity
val content = EntityUtils.toString(entity)
val id = "DPPR-\\d{4}".r
val jiraID = id.findFirstIn(content)
assert(jiraID.isDefined, s"Failed to create JIRA: ${content}")
jiraID.get
}
def createJIRA(project: String, summary: String, description: String, assigneeName: String,
assigneeEmail: String): String = {
val data = s"""{
"fields": {
"project":{
"key": "${project}"
},
"summary": "[Automation] - ${summary}",
"customfield_10118": "DPPR-1109",
"assignee": {
"name": "${assigneeName}",
"emailAddress": "${assigneeEmail}"
},
"description": "${description}",
"issuetype": {
"name": "Bug"
}
}
}"""
val url = "https://acosta-it.atlassian.net/rest/api/latest/issue/"
val post = new HttpPost(url)
val httpClient = HttpClients.createDefault()
val credential = new UsernamePasswordCredentials("[email protected]", "CoddadEhptnDfU18F")
post.addHeader("Content-type", "application/json")
post.addHeader(new BasicScheme().authenticate(credential, post, null))
post.setEntity(new StringEntity(data))
val response = httpClient.execute(post)
val jiraId = getJiraID(response)
jiraId
}
}
1 Answer 1
The suggestions I'd make are all around making the code more robust and more explicit in error handling.
Representing errors
Your methods return String
s. However, I'd say that's not really a good representation of what the methods do. Here's what I mean...
getJiraID
can fail. You've checked for this in an assert
, which is great. That will give an informative message, but will also stop the program. (It's totally possible that's OK for your case, but in general, it's not). What you could do instead is have the methods return perhaps an Either[Error, String]
where Error
could be a case class that captures the relevant details of what went wrong.
Why is that important? If I'm using your code, the signature says I'll get a String
back no matter what I pass in. However, what really happens is that the programme could stop. That would be very surprising to me as a user.
If, instead, I see the signature is Either[Error, String]
I know it could fail, and I have to handle that appropriately.
To be clear: I'm making a general point. Your getJiraID
is private, and you could say it's an internal helper, and will never fail in reality, but hopefully, you see the point I'm making.
That carries through into createJIRA
which also returns a String
(no matter what), but clearly this can fail. It can fail in more ways: presumably, the http client you're using could fail due to network error or a server error. This is essentially the same point I made above.
JSON
The value data
represents JSON. What happens if project
contains a quotation mark? You'll have invalid JSON. I'd suggest using a JSON library to construct JSON safely, which will handle the cases of properly encoding String. Circe is a good one, but there are plenty of others.
Hardcoded credentials
You have included a username and password in the body of the code. When that password expires (or changes), you'll have to hunt that down. I'd suggest making those parameters. For example:
case class JiraClient(user: String, pass: String) {
def create(...) = ...
}
... and then creating a JiraClient
at the edges of your code (e.g., in a main
) and perhaps reading those credentials from the environment (command-line arguments, configuration file, environment variables). You'd then have something anyone could use, and is easy to change without a recompile.
That same suggestion might apply for the url
, too. I don't know how often that might change.
Trivial
Some of the formatting looks a little odd to my eye. E.g., String ={
vs. String = {
. Take a look at scalafmt as a way to automatically format your code for consistency.
response
andjiraId
don't appear to serve any purpose other than documentation and, in the case ofjiraId
, that's already well served bygetJiraID()
. \$\endgroup\$