Skip to content

Commit e5cb297

Browse files
rtfpessoartfpessoa
authored
Merge pull request #20 from codacy/fix-exceptions
Handle possible exception on parseJson and fix null author on json
2 parents c010b56 + ac5a9f1 commit e5cb297

File tree

3 files changed

+153
-13
lines changed

3 files changed

+153
-13
lines changed

‎src/main/scala/com/codacy/client/bitbucket/PullRequest.scala‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import play.api.libs.functional.syntax._
55
import play.api.libs.json._
66

77
case class PullRequest(id: Long, title: String, description: String,
8-
authorUsername: String, authorAvatar: Option[String],
8+
authorUsername: Option[String], authorAvatar: Option[String],
99
state: String, created_on: DateTime, updated_on: DateTime,
1010
sourceRepository: String, sourceBranch: String, sourceCommit: String,
1111
destRepository: String, destBranch: String, destCommit: Option[String],
@@ -47,8 +47,8 @@ object PullRequest {
4747
(__ \ "id").read[Long] and
4848
(__ \ "title").read[String] and
4949
(__ \ "description").read[String] and
50-
(__ \ "author" \ "username").read[String] and
51-
(__ \ "author" \ "links" \ "avatar" \ "href").read[Option[String]] and
50+
(__ \ "author" \ "username").readNullable[String] and
51+
(__ \ "author" \ "links" \ "avatar" \ "href").readNullable[String].orElse((__ \ "author" \ "links").readNullable[String]) and
5252
(__ \ "state").read[String] and
5353
(__ \ "created_on").read[DateTime] and
5454
(__ \ "updated_on").read[DateTime] and

‎src/main/scala/com/codacy/client/bitbucket/client/BitbucketClient.scala‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import java.util.concurrent.{SynchronousQueue, ThreadPoolExecutor, TimeUnit}
55
import com.codacy.client.bitbucket.util.HTTPStatusCodes
66
import com.ning.http.client.AsyncHttpClientConfig
77
import play.api.http.{ContentTypeOf, Writeable}
8-
import play.api.libs.json.{JsValue, Json, Reads}
8+
import play.api.libs.json._
99
import play.api.libs.oauth._
1010
import play.api.libs.ws.ning.{NingAsyncHttpClientConfigBuilder, NingWSClient}
1111

@@ -67,7 +67,19 @@ class BitbucketClient(key: String, secretKey: String, token: String, secretToken
6767
val jsValue = parseJson(body)
6868
jsValue match {
6969
case Right(json) =>
70-
json.validate[T].fold(_ => FailedResponse(s"Failed to parse json: $json"), a => SuccessfulResponse(a))
70+
json.validate[T] match {
71+
case s: JsSuccess[T] =>
72+
SuccessfulResponse(s.value)
73+
case e: JsError =>
74+
val msg =
75+
s"""
76+
|Failed to validate json:
77+
|$json
78+
|JsError errors:
79+
|${e.errors.mkString(System.lineSeparator)}
80+
""".stripMargin
81+
FailedResponse(msg)
82+
}
7183
case Left(message) =>
7284
FailedResponse(message.detail)
7385
}
@@ -129,14 +141,20 @@ class BitbucketClient(key: String, secretKey: String, token: String, secretToken
129141
}
130142

131143
private def parseJson(input: String): Either[ResponseError, JsValue] = {
132-
val json = Json.parse(input)
133-
134-
val errorOpt = (json \ "error").asOpt[ResponseError]
135-
136-
errorOpt.map {
137-
error =>
138-
Left(error)
139-
}.getOrElse(Right(json))
144+
Try {
145+
val json = Json.parse(input)
146+
val errorOpt = (json \ "error").asOpt[ResponseError]
147+
148+
errorOpt.map {
149+
error =>
150+
Left(error)
151+
}.getOrElse(Right(json))
152+
} match {
153+
case Success(jsValue) =>
154+
jsValue
155+
case Failure(e) =>
156+
Left(ResponseError("Failed to parse json",e.getStackTrace.mkString(System.lineSeparator),e.getMessage))
157+
}
140158
}
141159

142160
private def withClientEither[T](block: NingWSClient => Either[ResponseError, T]): Either[ResponseError, T] = {

‎src/test/scala/RemoteRepositorySpecs.scala‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,4 +265,126 @@ class RemoteRepositorySpecs extends FlatSpec with Matchers {
265265

266266
parsed shouldBe true
267267
}
268+
269+
it should "successfully parse a JSON with null author while getting info for PRs" in {
270+
271+
val input =
272+
"""
273+
|{
274+
| "description": "",
275+
| "links": {
276+
| "decline": {
277+
| "href": "https://bitbucket.org/wtv"
278+
| },
279+
| "commits": {
280+
| "href": "https://bitbucket.org/wtv"
281+
| },
282+
| "self": {
283+
| "href": "https://bitbucket.org/wtv"
284+
| },
285+
| "comments": {
286+
| "href": "https://bitbucket.org/wtv"
287+
| },
288+
| "merge": {
289+
| "href": "https://bitbucket.org/wtv"
290+
| },
291+
| "html": {
292+
| "href": "https://bitbucket.org/wtv"
293+
| },
294+
| "activity": {
295+
| "href": "https://bitbucket.org/wtv"
296+
| },
297+
| "diff": {
298+
| "href": "https://bitbucket.org/wtv"
299+
| },
300+
| "approve": {
301+
| "href": "https://bitbucket.org/wtv"
302+
| },
303+
| "statuses": {
304+
| "href": "https://bitbucket.org/wtv"
305+
| }
306+
| },
307+
| "title": "Potatos",
308+
| "close_source_branch": true,
309+
| "merge_commit": null,
310+
| "destination": {
311+
| "commit": {
312+
| "hash": "aaaaaaaaaaaa",
313+
| "links": {
314+
| "self": {
315+
| "href": "https://bitbucket.org/wtv"
316+
| }
317+
| }
318+
| },
319+
| "repository": {
320+
| "links": {
321+
| "self": {
322+
| "href": "https://bitbucket.org/wtv"
323+
| },
324+
| "html": {
325+
| "href": "https://bitbucket.org/wtv"
326+
| },
327+
| "avatar": {
328+
| "href": "https://bitbucket.org/wtv"
329+
| }
330+
| },
331+
| "type": "repository",
332+
| "name": "my-potatos",
333+
| "full_name": "codacy/my-potatos",
334+
| "uuid": "{70bb8ec5-0c14-4964-aaaa-ebb4b2235ecf}"
335+
| },
336+
| "branch": {
337+
| "name": "master"
338+
| }
339+
| },
340+
| "state": "OPEN",
341+
| "closed_by": null,
342+
| "source": {
343+
| "commit": {
344+
| "hash": "bbbbbbbbbbbb",
345+
| "links": {
346+
| "self": {
347+
| "href": "https://bitbucket.org/wtv"
348+
| }
349+
| }
350+
| },
351+
| "repository": {
352+
| "links": {
353+
| "self": {
354+
| "href": "https://bitbucket.org/wtv"
355+
| },
356+
| "html": {
357+
| "href": "https://bitbucket.org/wtv"
358+
| },
359+
| "avatar": {
360+
| "href": "https://bitbucket.org/wtv"
361+
| }
362+
| },
363+
| "type": "repository",
364+
| "name": "my-potatos",
365+
| "full_name": "codacy/my-potatos",
366+
| "uuid": "{c08af10a-b49f-4a7a-9edb-6a66b3495a6f}"
367+
| },
368+
| "branch": {
369+
| "name": "add-more-potatos"
370+
| }
371+
| },
372+
| "comment_count": 0,
373+
| "author": null,
374+
| "created_on": "2016-04-05T14:35:41.678131+00:00",
375+
| "reason": "",
376+
| "updated_on": "2016-11-01T14:25:45.932586+00:00",
377+
| "type": "pullrequest",
378+
| "id": 333,
379+
| "task_count": 0
380+
|}
381+
""".stripMargin
382+
val json = Json.parse(input)
383+
val value = json.validate[PullRequest]
384+
385+
val parsed = value.fold(e => false, r => r.destCommit.isDefined)
386+
387+
parsed shouldBe true
388+
389+
}
268390
}

0 commit comments

Comments
 (0)