Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cricket Players Unique Name #27464

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions sport/app/cricket/feed/PlayerNames.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package cricket.feed

import cricketModel.Player

object PlayerNames {

def uniqueNames(players: List[Player]) = (for {
playersGroupedByName <- players.groupBy(_.lastName).values
player <- playersGroupedByName
} yield {
player.id -> { if (playersGroupedByName.size > 1) s"${player.firstName} ${player.lastName}" else player.lastName }
}).toMap
}
11 changes: 10 additions & 1 deletion sport/app/cricket/feed/cricketModel.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
package cricketModel

import cricket.feed.PlayerNames
import play.api.libs.json._

import java.time.LocalDateTime

case class Team(name: String, id: String, home: Boolean, lineup: List[String])
case class Player(id: String, name: String, firstName: String, lastName: String, initials: String)

object Player {
implicit val writes: OWrites[Player] = Json.writes[Player]
}
case class Team(name: String, id: String, home: Boolean, lineup: List[String], players: List[Player]) {
lazy val uniquePlayerNames = PlayerNames.uniqueNames(players)
}

object Team {
implicit val writes: OWrites[Team] = Json.writes[Team]
Expand Down
75 changes: 65 additions & 10 deletions sport/app/cricket/feed/cricketPaDeserialisation.scala
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package conf.cricketPa

import com.madgag.scala.collection.decorators.MapDecorator
import common.Chronos
import cricket.feed.PlayerNames

import xml.{NodeSeq, XML}
import xml.{Node, NodeSeq, XML}
import scala.language.postfixOps
import cricketModel._

Expand All @@ -17,10 +19,11 @@ object Parser {
val matchDetail = parseMatchDetail(matchData)
val lineupData = XML.loadString(lineups)
val scorecardData = XML.loadString(scorecard)
val teams = parseTeams(lineupData \ "match" \ "team")

Match(
parseTeams(lineupData \ "match" \ "team"),
parseInnings(scorecardData \ "match" \ "innings"),
teams,
parseInnings(scorecardData \ "match" \ "innings", teams),
matchDetail.competitionName,
matchDetail.venueName,
matchDetail.result,
Expand Down Expand Up @@ -64,21 +67,33 @@ object Parser {

private def parseTeams(teams: NodeSeq): List[Team] =
teams.map { team =>
Team((team \ "name").text, (team \ "@id").text, (team \ "home").text == "true", parseTeamLineup(team \ "player"))
val players = parseTeamLineup(team \ "player")
Team((team \ "name").text, (team \ "@id").text, (team \ "home").text == "true", players.map(_.name), players)

}.toList

private def parseTeamLineup(lineup: NodeSeq): List[String] =
lineup.map { player => (player \ "name").text }.toList
private def parsePlayer(player: Node): Player = Player(
id = (player \ "id").text,
name = (player \ "name").text,
firstName = (player \ "firstName").text,
lastName = (player \ "lastName").text,
initials = (player \ "initials").text,
)

private def parseTeamLineup(lineup: NodeSeq): List[Player] = lineup.map(parsePlayer).toList

private def getStatistic(statistics: NodeSeq, statistic: String): String =
(statistics \ "statistic").find(node => (node \ "@type").text == statistic).map(_.text).getOrElse("")

private def parseInnings(innings: NodeSeq): List[Innings] =
private def parseInnings(innings: NodeSeq, teams: List[Team]): List[Innings] =
innings
.map { singleInnings =>
val inningsOrder = (singleInnings \ "@order").text.toInt
val battingTeam = (singleInnings \ "batting" \ "team" \ "name").text

val bowlingTeamName = (singleInnings \ "bowling" \ "team" \ "name").text
val bowlingTeam = teams.find(_.name == bowlingTeamName)

Innings(
inningsOrder,
battingTeam,
Expand All @@ -87,7 +102,7 @@ object Parser {
getStatistic(singleInnings, "declared") == "true",
getStatistic(singleInnings, "forefeited") == "true",
inningsDescription(inningsOrder, battingTeam),
parseInningsBatters(singleInnings \ "batting" \ "batter"),
parseInningsBatters(singleInnings \ "batting" \ "batter", bowlingTeam),
parseInningsBowlers(singleInnings \ "bowling" \ "bowler"),
parseInningsWickets(singleInnings \ "fallenWicket"),
getStatistic(singleInnings, "extra-byes").toInt,
Expand All @@ -101,7 +116,46 @@ object Parser {
.toList
.sortBy(_.order)

private def parseInningsBatters(batters: NodeSeq): List[InningsBatter] =
case class Dismissal(items: Seq[(String, String)]) {

def description(dismissal: NodeSeq, f: Player => String): String = {
for {
(nodeName, prefix) <- items
} yield {
s"$prefix ${f(parsePlayer((dismissal \ nodeName \ "player").head))}"
}
}.mkString(" ")

}

val dismissalTypes: Map[String, Dismissal] = Map(
"caught" -> Seq("caughtBy" -> "st", "bowledBy" -> "b"), // c Rathnayake b de Silva
"caught-sub" -> Seq("bowledBy" -> "c Sub b"), // c Sub b Kumara
"caught-and-bowled" -> Seq("caughtBy" -> "c & b"), // c &amp; b Woakes
"stumped" -> Seq("caughtBy" -> "st", "bowledBy" -> "b"), // st Ambrose b Patel
"run-out" -> Seq("caughtBy" -> "Run Out"), // Run Out Stone
"lbw" -> Seq("bowledBy" -> "lbw b"), // lbw b Stone
"bowled" -> Seq("bowledBy" -> "b"), // b Kumara
).mapV(Dismissal)

def parseDismissal(dismissal: NodeSeq, bowlingTeamOpt: Option[Team]): String = {
val description = (dismissal \ "description").text
(
for {
bowlingTeam <- bowlingTeamOpt
dismissalDescriber <- dismissalTypes.get(dismissal \@ "type")
if description == dismissalDescriber.description(dismissal, _.lastName)
} yield {
dismissalDescriber.description(
dismissal,
player => bowlingTeam.uniquePlayerNames.getOrElse(player.id, player.name),
)
}
).getOrElse(description)
}

private def parseInningsBatters(batters: NodeSeq, bowlingTeam: Option[Team]): List[InningsBatter] = {

batters
.map { batter =>
InningsBatter(
Expand All @@ -112,13 +166,14 @@ object Parser {
getStatistic(batter, "fours") toInt,
getStatistic(batter, "sixes") toInt,
(batter \ "status").text == "batted",
(batter \ "dismissal" \ "description").text,
parseDismissal(batter \ "dismissal", bowlingTeam),
getStatistic(batter, "on-strike").toInt > 0,
getStatistic(batter, "runs-scored").toInt > 0,
)
}
.toList
.sortBy(_.order)
}

private def parseInningsBowlers(bowlers: NodeSeq): List[InningsBowler] =
bowlers
Expand Down
37 changes: 37 additions & 0 deletions sport/test/cricket/feed/CricketPaDeserialisationTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package cricket.feed

import org.apache.pekko.actor.{ActorSystem => PekkoActorSystem}
import conf.cricketPa.PaFeed
import org.scalatest.{BeforeAndAfterAll, DoNotDiscover}
import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
import test.{
ConfiguredTestSuite,
WithMaterializer,
WithTestApplicationContext,
WithTestExecutionContext,
WithTestWsClient,
}

@DoNotDiscover class CricketPaDeserialisationTest
Copy link
Contributor Author

@ioannakok ioannakok Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is WIP so I added @DoNotDiscover to keep the build green. At the moment it's failing with error:

cricket.feed.CricketPaDeserialisationTest *** ABORTED ***
[info]   java.lang.NullPointerException:
[info]   at test.WithMaterializer.materializer(package.scala:121)
[info]   at test.WithMaterializer.materializer$(package.scala:121)
[info]   at cricket.feed.CricketPaDeserialisationTest.materializer$lzycompute(CricketPaDeserialisationTest.scala:11)
[info]   at cricket.feed.CricketPaDeserialisationTest.materializer(CricketPaDeserialisationTest.scala:11)
[info]   at test.WithTestWsClient.$anonfun$lazyWsClient$1(package.scala:127)
[info]   at common.Lazy.get(Lazy.scala:16)
[info]   at common.Lazy$.conversionLazy(Lazy.scala:7)
[info]   at test.WithTestWsClient.wsClient(package.scala:128)
[info]   at test.WithTestWsClient.wsClient$(package.scala:128)
[info]   at cricket.feed.CricketPaDeserialisationTest.wsClient$lzycompute(CricketPaDeserialisationTest.scala:11)

We will probably want to change the test anyway and use test data instead of hitting the PA API.
Examples of xml sports test data in the codebase:
https://github.com/guardian/frontend/tree/main/admin/test/football/testdata
https://github.com/guardian/frontend/blob/main/sport/test/resources/rugby/feed/live-scores.xml

extends AnyFlatSpec
with Matchers
with ConfiguredTestSuite
with BeforeAndAfterAll
with WithMaterializer
with WithTestWsClient
with WithTestApplicationContext
with WithTestExecutionContext
with ScalaFutures
with IntegrationPatience {
val actorSystem = PekkoActorSystem()
val paFeed = new PaFeed(wsClient, actorSystem, materializer)

whenReady(paFeed.getMatch("39145392-3f2e-8022-35f3-eac0b0654610")) { cricketMatch =>
{
cricketMatch.innings.head.batters.head.howOut shouldBe ""

}
}
}
64 changes: 64 additions & 0 deletions sport/test/cricket/feed/PlayerNamesTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package cricket.feed

import cricketModel.Player
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

class PlayerNamesTest extends AnyFlatSpec with Matchers {

"uniqueNames" should "be determined by lastName if players have different last names" in {
val player1 = Player(
id = "ae5e0dbf-d6af-70ec-76ef-1f8e83230405",
name = "Asitha Fernando",
firstName = "Asitha",
lastName = "Fernando",
initials = "A M",
)
val player2 = Player(
id = "c32cd9c7-d38a-93e7-e874-f5f4a5197812",
name = "Dimuth Karunaratne",
firstName = "Frank",
lastName = "Karunaratne",
initials = "F D M",
)
val player3 = Player(
id = "b96e5130-0348-9659-e3c6-ba887f306eeb",
name = "Kusal Mendis",
firstName = "Balapuwaduge",
lastName = "Mendis",
initials = "B K G",
)

PlayerNames.uniqueNames(List(player1, player2, player3)).values.toList should contain("Fernando")
PlayerNames.uniqueNames(List(player1, player2, player3)).values.toList should contain("Mendis")
PlayerNames.uniqueNames(List(player1, player2, player3)).values.toList should contain("Karunaratne")
}

it should "be determined by full name and lastName if players have the same last name" in {
val player1 = Player(
id = "ae5e0dbf-d6af-70ec-76ef-1f8e83230405",
name = "Asitha Fernando",
firstName = "Asitha",
lastName = "Fernando",
initials = "A M",
)
val player2 = Player(
id = "c32cd9c7-d38a-93e7-e874-f5f4a5197812",
name = "Dimuth Karunaratne",
firstName = "Frank",
lastName = "Karunaratne",
initials = "F D M",
)
val player3 = Player(
id = "d29c8d1c-29b4-517e-5b62-1277065801b2",
name = "Nishan Madushka",
firstName = "Kottasinghakkarage",
lastName = "Fernando",
initials = "K N M",
)

PlayerNames.uniqueNames(List(player1, player2, player3)).values.toList should contain("Asitha Fernando")
PlayerNames.uniqueNames(List(player1, player2, player3)).values.toList should contain("Karunaratne")
PlayerNames.uniqueNames(List(player1, player2, player3)).values.toList should contain("Kottasinghakkarage Fernando")
}
}