Skip to content

Commit

Permalink
Merge pull request #25166 from guardian/jc/remane-odds-ands-ends
Browse files Browse the repository at this point in the history
Renames `TopMentionsTopicType` to `TopicType`.
  • Loading branch information
joecowton1 authored Jun 30, 2022
2 parents a4498ba + a22f4be commit 6286ee6
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 73 deletions.
24 changes: 12 additions & 12 deletions article/app/topics/TopicS3Client.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import com.amazonaws.services.s3.model.{GetObjectRequest, S3Object}
import com.amazonaws.util.IOUtils
import common.GuLogging
import conf.Configuration
import model.{TopMentionJsonParseException, TopicsApiResponse}
import model.{TopicsJsonParseException, TopicsApiResponse}
import play.api.libs.json.{JsError, JsSuccess, Json}
import services.S3
import topics.S3ObjectImplicits.RichS3Object
Expand All @@ -30,10 +30,10 @@ final class TopicS3ClientImpl extends TopicS3Client with S3 with GuLogging {
s3ObjectList.map(_.getKey)
} match {
case Success(value) =>
log.info(s"got list of ${value.length} top mentions from S3")
log.info(s"got list of ${value.length} topics from S3")
Future.successful(value)
case Failure(exception) =>
log.error(s"failed in getting the list of top mentions from S3", exception)
log.error(s"failed in getting the list of topics from S3", exception)
Future.failed(exception)
}
}
Expand All @@ -46,7 +46,7 @@ final class TopicS3ClientImpl extends TopicS3Client with S3 with GuLogging {
client.getObject(request).parseToTopicsApiResponse
}.flatten match {
case Success(value) =>
log.info(s"got topMentionResponse from S3 for key ${key}")
log.info(s"got TopicsApiResponse from S3 for key ${key}")
Future.successful(value)
case Failure(exception) =>
log.error(s"S3 retrieval failed for key ${key}", exception)
Expand All @@ -57,14 +57,14 @@ final class TopicS3ClientImpl extends TopicS3Client with S3 with GuLogging {

private def getBucket() = {
optionalBucket.getOrElse(
throw new RuntimeException("top mention bucket config is empty, make sure config parameter has value"),
throw new RuntimeException("topics bucket config is empty, make sure config parameter has value"),
)
}

private def getClient[T](callS3: AmazonS3 => Future[T]) = {
client
.map { callS3(_) }
.getOrElse(Future.failed(new RuntimeException("No client exist for TopicS3Client")))
.getOrElse(Future.failed(new RuntimeException("No client exists for TopicS3Client")))
}
}

Expand All @@ -74,15 +74,15 @@ object S3ObjectImplicits {
val json = Json.parse(asString(s3Object))

Json.fromJson[TopicsApiResponse](json) match {
case JsSuccess(topMentionResponse, __) =>
log.debug(s"Parsed TopMentionsDetails from S3 for key ${s3Object.getKey}")
Success(topMentionResponse)
case JsSuccess(topicsApiResponse, __) =>
log.debug(s"Parsed TopicsApiResponse from S3 for key ${s3Object.getKey}")
Success(topicsApiResponse)
case JsError(errors) =>
val errorPaths = errors.map { error => error._1.toString() }.mkString(",")
log.error(s"Error parsing topMentionResponse from S3 for key ${s3Object.getKey} paths: ${errorPaths}")
log.error(s"Error parsing TopicsApiResponse from S3 for key ${s3Object.getKey} paths: ${errorPaths}")
Failure(
TopMentionJsonParseException(
s"could not parse S3 TopMentionsDetails json. Errors paths(s): $errors",
TopicsJsonParseException(
s"could not parse S3 TopicsApiResponse json. Errors paths(s): $errors",
),
)
}
Expand Down
16 changes: 8 additions & 8 deletions article/app/topics/TopicService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ class TopicService(topicS3Client: TopicS3Client) extends GuLogging {
private val topicsDetails = Box[Option[Map[String, TopicsApiResponse]]](None)

def refreshTopics()(implicit executionContext: ExecutionContext): Future[Unit] = {
val retrievedTopMentions = topicS3Client.getListOfKeys().map { key => key.map { retrieveTopicsDetails(_) } }
val retrievedTopics = topicS3Client.getListOfKeys().map { key => key.map { retrieveTopicsDetails(_) } }

retrievedTopMentions
retrievedTopics
.flatMap(Future.sequence(_))
.map(response => {
topicsDetails send Some(response.toMap)
log.info("successfully refreshed top mentions")
log.info("successfully refreshed topics")
})
.recover {
case e =>
log.error("Could not refresh top mentions", e)
log.error("Could not refresh topics", e)
}
}

Expand All @@ -29,8 +29,8 @@ class TopicService(topicS3Client: TopicS3Client) extends GuLogging {
}

def getAvailableTopics(blogId: String): Option[Seq[AvailableTopic]] = {
getBlogTopicsApiResponse(blogId).map(mentions =>
mentions.results.map(mention => AvailableTopic(mention.`type`, mention.name, mention.count)),
getBlogTopicsApiResponse(blogId).map(topicsApiResponse =>
topicsApiResponse.results.map(topic => AvailableTopic(topic.`type`, topic.name, topic.count)),
)
}

Expand All @@ -40,10 +40,10 @@ class TopicService(topicS3Client: TopicS3Client) extends GuLogging {

def getSelectedTopic(
blogId: String,
topMentionEntity: SelectedTopic,
topicEntity: SelectedTopic,
): Option[TopicResult] = {
getBlogTopicsApiResponse(blogId).flatMap(_.results.find(result => {
result.`type` == topMentionEntity.`type` && result.name == topMentionEntity.value
result.`type` == topicEntity.`type` && result.name == topicEntity.value
}))
}

Expand Down
28 changes: 14 additions & 14 deletions article/test/LiveBlogControllerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import play.api.test._
import play.api.test.Helpers._
import org.scalatest.{BeforeAndAfterAll, DoNotDiscover}
import org.scalatestplus.mockito.MockitoSugar
import model.{LiveBlogPage, TopicResult, SelectedTopic, TopMentionsTopicType, AvailableTopic, TopicsLiveBlog}
import model.{LiveBlogPage, TopicResult, SelectedTopic, TopicType, AvailableTopic, TopicsLiveBlog}
import topics.{TopicS3Client, TopicService}

import scala.concurrent.Future
Expand All @@ -33,27 +33,27 @@ import scala.concurrent.Future
var fakeDcr = new DCRFake()
val topicResult = TopicResult(
name = "Fifa",
`type` = TopMentionsTopicType.Org,
`type` = TopicType.Org,
blocks = Seq("56d08042e4b0d38537b1f70b"),
count = 1,
percentage_blocks = 1.2f,
)

val fakeAvailableTopics = Vector(
AvailableTopic(TopMentionsTopicType.Gpe, "United Kingdom", 6),
AvailableTopic(TopMentionsTopicType.Gpe, "Russia", 4),
AvailableTopic(TopMentionsTopicType.Org, "KPMG", 4),
AvailableTopic(TopMentionsTopicType.Gpe, "Ukraine", 3),
AvailableTopic(TopMentionsTopicType.Gpe, "China", 2),
AvailableTopic(TopMentionsTopicType.Gpe, "United States", 2),
AvailableTopic(TopMentionsTopicType.Loc, "Europe", 2),
AvailableTopic(TopMentionsTopicType.Gpe, "Moscow", 2),
AvailableTopic(TopMentionsTopicType.Org, "PZ Cussons", 2),
AvailableTopic(TopMentionsTopicType.Person, "Emmanuel Macron", 1),
AvailableTopic(TopicType.Gpe, "United Kingdom", 6),
AvailableTopic(TopicType.Gpe, "Russia", 4),
AvailableTopic(TopicType.Org, "KPMG", 4),
AvailableTopic(TopicType.Gpe, "Ukraine", 3),
AvailableTopic(TopicType.Gpe, "China", 2),
AvailableTopic(TopicType.Gpe, "United States", 2),
AvailableTopic(TopicType.Loc, "Europe", 2),
AvailableTopic(TopicType.Gpe, "Moscow", 2),
AvailableTopic(TopicType.Org, "PZ Cussons", 2),
AvailableTopic(TopicType.Person, "Emmanuel Macron", 1),
);

when(
fakeTopicService.getSelectedTopic(path, SelectedTopic(TopMentionsTopicType.Org, "Fifa")),
fakeTopicService.getSelectedTopic(path, SelectedTopic(TopicType.Org, "Fifa")),
) thenReturn Some(
topicResult,
)
Expand Down Expand Up @@ -314,7 +314,7 @@ import scala.concurrent.Future
assertDcrCalledForLiveBlogWithBlocks(fakeDcr, expectedBlocks = Seq("56d08042e4b0d38537b1f70b"))
}

"renderArticle" should "doesn't call getTopMentionsByTopic given filterKeyEvents and topics query params are provided" in new Setup {
"renderArticle" should "doesn't call getSelectedTopic given filterKeyEvents and topics query params are provided" in new Setup {
val fakeRequest = FakeRequest(GET, s"${path}").withHeaders("host" -> "localhost:9000")

val result = liveBlogController.renderArticle(
Expand Down
18 changes: 9 additions & 9 deletions article/test/model/LiveBlogCurrentPageTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package model

import model.liveblog.BodyBlock.{KeyEvent, SummaryEvent}
import model.liveblog._
import model.TopMentionsTopicType.TopMentionsTopicType
import model.TopicType.TopicType
import org.joda.time.DateTime
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
Expand Down Expand Up @@ -65,7 +65,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
filterKeyEvents = false,
topicResult = Some(
TopicResult(
`type` = TopMentionsTopicType.Org,
`type` = TopicType.Org,
name = "someName",
blocks = Seq(),
count = 0,
Expand Down Expand Up @@ -375,8 +375,8 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
should(result, currentPage = expectedCurrentPage, pagination = expectedPagination)
}

it should "only filters blocks by key events given both key events and top mentions are provided" in {
val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("1", "3"))
it should "only filters blocks by key events given both key events and topicResult are provided" in {
val topicResult = getFakeTopicResult(TopicType.Org, "tfl", Seq("1", "3"))
val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 5, numberOfKeyEventsBlocks = 2, None)
val result =
LiveBlogCurrentPage.firstPage(
Expand All @@ -400,7 +400,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
}

it should "returns the 1st page of the topic filtered blocks" in {
val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("1", "2", "3", "4"))
val topicResult = getFakeTopicResult(TopicType.Org, "tfl", Seq("1", "2", "3", "4"))
val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 5, numberOfKeyEventsBlocks = 2, None)
val result =
LiveBlogCurrentPage.firstPage(
Expand Down Expand Up @@ -670,7 +670,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
}

it should "returns the correct 2nd page of the topic filtered blocks" in {
val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "4", "5", "6"))
val topicResult = getFakeTopicResult(TopicType.Org, "tfl", Seq("2", "3", "4", "5", "6"))
val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2, None)

val expectedFirstPage = FirstPage(
Expand Down Expand Up @@ -707,7 +707,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
}

"updates" should "return only the topic filtered blocks after the lastUpdated block given lastUpdated is also a topic block" in {
val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "4", "5", "6"))
val topicResult = getFakeTopicResult(TopicType.Org, "tfl", Seq("2", "3", "4", "5", "6"))
val sinceBlock = SinceBlockId("5")
val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2, Some("5"))

Expand All @@ -723,7 +723,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
}

"updates" should "return only the topic filtered blocks after the lastUpdated block given lastUpdated is NOT a topic block" in {
val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "5", "6"))
val topicResult = getFakeTopicResult(TopicType.Org, "tfl", Seq("2", "3", "5", "6"))
val sinceBlockId = "4"
val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2, Some(sinceBlockId))

Expand All @@ -740,7 +740,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers {
}

private def getFakeTopicResult(
topicType: TopMentionsTopicType,
topicType: TopicType,
topicName: String,
blocks: Seq[String],
) = {
Expand Down
26 changes: 13 additions & 13 deletions article/test/topics/TopicServiceTest.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package topics

import model.{TopicsApiResponse, TopicResult, SelectedTopic, TopMentionsTopicType, AvailableTopic}
import model.{TopicsApiResponse, TopicResult, SelectedTopic, TopicType, AvailableTopic}
import org.scalatest.{BeforeAndAfterAll}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
Expand All @@ -22,7 +22,7 @@ class TopicServiceTest
val topicResult =
TopicResult(
name = "name1",
`type` = TopMentionsTopicType.Org,
`type` = TopicType.Org,
blocks = Seq("blockId1"),
count = 1,
percentage_blocks = 1.2f,
Expand All @@ -31,24 +31,24 @@ class TopicServiceTest
val topicResults = Seq(
TopicResult(
name = "name1",
`type` = TopMentionsTopicType.Org,
`type` = TopicType.Org,
blocks = Seq("blockId1"),
count = 1,
percentage_blocks = 1.2f,
),
TopicResult(
name = "name2",
`type` = TopMentionsTopicType.Person,
`type` = TopicType.Person,
blocks = Seq("blockId1"),
count = 10,
percentage_blocks = 1.2f,
),
)
val successResponse =
TopicsApiResponse(entity_types = Seq(TopMentionsTopicType.Org), results = Seq(topicResult), model = "model")
TopicsApiResponse(entity_types = Seq(TopicType.Org), results = Seq(topicResult), model = "model")

val successMultiResponse =
TopicsApiResponse(entity_types = Seq(TopMentionsTopicType.Org), results = topicResults, model = "model")
TopicsApiResponse(entity_types = Seq(TopicType.Org), results = topicResults, model = "model")

"refreshTopics" should "return successful future given getListOfKeys s3 call fails" in {
when(fakeClient.getListOfKeys()) thenReturn Future.failed(new Throwable(""))
Expand Down Expand Up @@ -95,7 +95,7 @@ class TopicServiceTest
val topicService = new TopicService(fakeClient)
val refreshJob = Await.result(topicService.refreshTopics(), 1.second)

val result = topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "name1"))
val result = topicService.getSelectedTopic("key1", SelectedTopic(TopicType.Org, "name1"))

result.get should equal(topicResult)
}
Expand All @@ -107,7 +107,7 @@ class TopicServiceTest
val topicService = new TopicService(fakeClient)
val refreshJob = Await.result(topicService.refreshTopics(), 1.second)

val result = topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "NAME1"))
val result = topicService.getSelectedTopic("key1", SelectedTopic(TopicType.Org, "NAME1"))

result should equal(None)
}
Expand All @@ -119,7 +119,7 @@ class TopicServiceTest
val topicService = new TopicService(fakeClient)
val refreshJob = Await.result(topicService.refreshTopics(), 1.second)

val result = topicService.getSelectedTopic("key2", SelectedTopic(TopMentionsTopicType.Org, "name1"))
val result = topicService.getSelectedTopic("key2", SelectedTopic(TopicType.Org, "name1"))

result should equal(None)
}
Expand All @@ -132,7 +132,7 @@ class TopicServiceTest
val refreshJob = Await.result(topicService.refreshTopics(), 1.second)

val result =
topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Person, "Boris"))
topicService.getSelectedTopic("key1", SelectedTopic(TopicType.Person, "Boris"))

result should equal(None)
}
Expand All @@ -145,7 +145,7 @@ class TopicServiceTest
val refreshJob = Await.result(topicService.refreshTopics(), 1.second)

val result =
topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "someRandomOrg"))
topicService.getSelectedTopic("key1", SelectedTopic(TopicType.Org, "someRandomOrg"))

result should equal(None)
}
Expand All @@ -156,8 +156,8 @@ class TopicServiceTest
val expectedTopics =
Some(
List(
AvailableTopic(TopMentionsTopicType.Org, "name1", 1),
AvailableTopic(TopMentionsTopicType.Person, "name2", 10),
AvailableTopic(TopicType.Org, "name1", 1),
AvailableTopic(TopicType.Person, "name2", 10),
),
)

Expand Down
Loading

0 comments on commit 6286ee6

Please sign in to comment.