1
\$\begingroup\$

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("gaurang.shah@abc.com", "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
  }

}
| improve this question | | | | |
\$\endgroup\$
  • \$\begingroup\$ The variables response and jiraId don't appear to serve any purpose other than documentation and, in the case of jiraId, that's already well served by getJiraID(). \$\endgroup\$ – jwvh Aug 30 '19 at 4:29
2
\$\begingroup\$

The suggestions I'd make are all around making the code more robust and more explicit in error handling.

Representing errors

Your methods return Strings. 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.

| improve this answer | | | | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.