Promote aggregation key to case class
Use a case class rather than a string as an aggregation key.
This avoids string parsing strings when extracting tokens
from the aggregation key.
It should simplify the addition of new aggregation strategies
since the handling of the tokens of the aggregation key will
be handled by the AggregationKey case class.
Change-Id: Ibd91665ea3d126f64aa14f270c3e8a09552b8d23
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
index f634725..98e4e06 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/Contributors.scala
@@ -24,8 +24,6 @@
import com.googlesource.gerrit.plugins.analytics.common._
import org.kohsuke.args4j.{Option => ArgOption}
-import scala.util.Try
-
@CommandMetaData(name = "contributors", description = "Extracts the list of contributors to a project")
class ContributorsCommand @Inject()(val executor: ContributorsService,
val projects: ProjectsCollection,
@@ -130,10 +128,11 @@
}
class ContributorsService @Inject()(repoManager: GitRepositoryManager,
- projectCache:ProjectCache,
+ projectCache: ProjectCache,
histogram: UserActivityHistogram,
gsonFmt: GsonFormatter,
commitsStatisticsCache: CommitsStatisticsCache) {
+
import RichBoolean._
def get(projectRes: ProjectResource, startDate: Option[Long], stopDate: Option[Long],
@@ -141,7 +140,7 @@
: TraversableOnce[UserActivitySummary] = {
ManagedResource.use(repoManager.openRepository(projectRes.getNameKey)) { repo =>
- val stats = new Statistics(projectRes.getNameKey, commitsStatisticsCache)
+ val stats = new Statistics(projectRes.getNameKey, commitsStatisticsCache)
val branchesExtractor = extractBranches.option(new BranchesExtractor(repo))
histogram.get(repo, new AggregatedHistogramFilterByDates(startDate, stopDate, branchesExtractor, aggregationStrategy))
@@ -155,10 +154,10 @@
case class IssueInfo(code: String, link: String)
-case class UserActivitySummary(year: Integer,
- month: Integer,
- day: Integer,
- hour: Integer,
+case class UserActivitySummary(year: Option[Int],
+ month: Option[Int],
+ day: Option[Int],
+ hour: Option[Int],
name: String,
email: String,
numCommits: Integer,
@@ -179,24 +178,30 @@
def apply(statisticsHandler: Statistics)(uca: AggregatedUserCommitActivity)
: Iterable[UserActivitySummary] = {
- def stringToIntOrNull(x: String): Integer = Try(new Integer(x)).getOrElse(null)
+ statisticsHandler.forCommits(uca.getIds: _*).map { stat =>
+ val maybeBranches =
+ uca.key.branch.fold(Array.empty[String])(Array(_))
- uca.key.split("/", AggregationStrategy.MAX_MAPPING_TOKENS) match {
- case Array(email, year, month, day, hour, branch) =>
- statisticsHandler.forCommits(uca.getIds: _*).map { stat =>
- val maybeBranches =
- Option(branch).filter(_.nonEmpty).map(b => Array(b)).getOrElse(Array.empty)
-
- UserActivitySummary(
- stringToIntOrNull(year), stringToIntOrNull(month), stringToIntOrNull(day), stringToIntOrNull(hour),
- uca.getName, email, stat.commits.size,
- stat.numFiles, stat.numDistinctFiles, stat.addedLines, stat.deletedLines,
- stat.commits.toArray, maybeBranches, stat.issues.map(_.code)
- .toArray, stat.issues.map(_.link).toArray, uca.getLatest, stat
- .isForMergeCommits,stat.isForBotLike
- )
- }
- case _ => throw new Exception(s"invalid key format found ${uca.key}")
+ UserActivitySummary(
+ uca.key.year,
+ uca.key.month,
+ uca.key.day,
+ uca.key.hour,
+ uca.getName,
+ uca.key.email,
+ stat.commits.size,
+ stat.numFiles,
+ stat.numDistinctFiles,
+ stat.addedLines,
+ stat.deletedLines,
+ stat.commits.toArray,
+ maybeBranches,
+ stat.issues.map(_.code).toArray,
+ stat.issues.map(_.link).toArray,
+ uca.getLatest,
+ stat.isForMergeCommits,
+ stat.isForBotLike
+ )
}
}
}
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala
index 74abf08..f39fc73 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregatedCommitHistogram.scala
@@ -16,19 +16,19 @@
import java.util.Date
-import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy.BY_BRANCH
+import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy.{AggregationKey, BY_BRANCH}
import org.eclipse.jgit.lib.PersonIdent
import org.eclipse.jgit.revwalk.RevCommit
import org.gitective.core.stat.{CommitHistogram, CommitHistogramFilter, UserCommitActivity}
-class AggregatedUserCommitActivity(val key: String, val name: String, val email: String)
+class AggregatedUserCommitActivity(val key: AggregationKey, val name: String, val email: String)
extends UserCommitActivity(name, email)
class AggregatedCommitHistogram(var aggregationStrategy: AggregationStrategy)
extends CommitHistogram {
def includeWithBranches(commit: RevCommit, user: PersonIdent, branches: Set[String]): Unit = {
- for ( branch <- branches ) {
+ for (branch <- branches) {
val originalStrategy = aggregationStrategy
this.aggregationStrategy = BY_BRANCH(branch, aggregationStrategy)
this.include(commit, user)
@@ -38,11 +38,13 @@
override def include(commit: RevCommit, user: PersonIdent): AggregatedCommitHistogram = {
val key = aggregationStrategy.mapping(user, commit.getAuthorIdent.getWhen)
- val activity = Option(users.get(key)) match {
+ val keyString = key.toString
+
+ val activity = Option(users.get(keyString)) match {
case None =>
val newActivity = new AggregatedUserCommitActivity(key,
user.getName, user.getEmailAddress)
- users.put(key, newActivity)
+ users.put(keyString, newActivity)
newActivity
case Some(foundActivity) => foundActivity
}
@@ -56,7 +58,7 @@
}
object AggregatedCommitHistogram {
- type AggregationStrategyMapping = (PersonIdent, Date) => String
+ type AggregationStrategyMapping = (PersonIdent, Date) => AggregationKey
}
abstract class AbstractCommitHistogramFilter(aggregationStrategy: AggregationStrategy)
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala
index 079c307..3292fc9 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/AggregationStrategy.scala
@@ -41,33 +41,57 @@
def utc: LocalDateTime = d.toInstant.atZone(ZoneOffset.UTC).toLocalDateTime
}
+ case class AggregationKey(email: String,
+ year: Option[Int] = None,
+ month: Option[Int] = None,
+ day: Option[Int] = None,
+ hour: Option[Int] = None,
+ branch: Option[String] = None)
+
object EMAIL extends AggregationStrategy {
val name: String = "EMAIL"
- val mapping: (PersonIdent, Date) => String = (p, _) => s"${p.getEmailAddress}/////"
+ val mapping: (PersonIdent, Date) => AggregationKey = (p, _) =>
+ AggregationKey(email = p.getEmailAddress)
}
object EMAIL_YEAR extends AggregationStrategy {
val name: String = "EMAIL_YEAR"
- val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}////"
+ val mapping: (PersonIdent, Date) => AggregationKey = (p, d) =>
+ AggregationKey(email = p.getEmailAddress, year = Some(d.utc.getYear))
}
object EMAIL_MONTH extends AggregationStrategy {
val name: String = "EMAIL_MONTH"
- val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}///"
+ val mapping: (PersonIdent, Date) => AggregationKey = (p, d) =>
+ AggregationKey(email = p.getEmailAddress,
+ year = Some(d.utc.getYear),
+ month = Some(d.utc.getMonthValue))
}
object EMAIL_DAY extends AggregationStrategy {
val name: String = "EMAIL_DAY"
- val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}/${d.utc.getDayOfMonth}//"
+ val mapping: (PersonIdent, Date) => AggregationKey = (p, d) =>
+ AggregationKey(email = p.getEmailAddress,
+ year = Some(d.utc.getYear),
+ month = Some(d.utc.getMonthValue),
+ day = Some(d.utc.getDayOfMonth))
}
object EMAIL_HOUR extends AggregationStrategy {
val name: String = "EMAIL_HOUR"
- val mapping: (PersonIdent, Date) => String = (p, d) => s"${p.getEmailAddress}/${d.utc.getYear}/${d.utc.getMonthValue}/${d.utc.getDayOfMonth}/${d.utc.getHour}/"
+ val mapping: (PersonIdent, Date) => AggregationKey = (p, d) =>
+ AggregationKey(email = p.getEmailAddress,
+ year = Some(d.utc.getYear),
+ month = Some(d.utc.getMonthValue),
+ day = Some(d.utc.getDayOfMonth),
+ hour = Some(d.utc.getHour))
}
- case class BY_BRANCH(branch: String, baseAggregationStrategy: AggregationStrategy) extends AggregationStrategy {
+ case class BY_BRANCH(branch: String,
+ baseAggregationStrategy: AggregationStrategy)
+ extends AggregationStrategy {
val name: String = s"BY_BRANCH($branch)"
- val mapping: (PersonIdent, Date) => String = (p, d) => s"${baseAggregationStrategy.mapping(p, d)}$branch"
+ val mapping: (PersonIdent, Date) => AggregationKey = (p, d) =>
+ baseAggregationStrategy.mapping(p, d).copy(branch = Some(branch))
}
}
diff --git a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala
index cca743d..4417f3f 100644
--- a/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala
+++ b/src/main/scala/com/googlesource/gerrit/plugins/analytics/common/GsonFormatter.scala
@@ -15,19 +15,18 @@
package com.googlesource.gerrit.plugins.analytics.common
import java.io.PrintWriter
-
-import com.google.gerrit.server.OutputFormat
-import com.google.gson.{Gson, GsonBuilder, JsonSerializer}
-import com.google.inject.Singleton
import java.lang.reflect.Type
-import com.google.gson._
+import com.google.gerrit.server.OutputFormat
+import com.google.gson.{Gson, GsonBuilder, JsonSerializer, _}
+import com.google.inject.Singleton
@Singleton
class GsonFormatter {
val gsonBuilder: GsonBuilder =
OutputFormat.JSON_COMPACT.newGsonBuilder
.registerTypeHierarchyAdapter(classOf[Iterable[Any]], new IterableSerializer)
+ .registerTypeHierarchyAdapter(classOf[Option[Any]], new OptionSerializer())
def format[T](values: TraversableOnce[T], out: PrintWriter) {
val gson: Gson = gsonBuilder.create
@@ -45,4 +44,13 @@
}
}
+ class OptionSerializer extends JsonSerializer[Option[Any]] {
+ def serialize(src: Option[Any], typeOfSrc: Type, context: JsonSerializationContext): JsonElement = {
+ src match {
+ case None => JsonNull.INSTANCE
+ case Some(v) => context.serialize(v)
+ }
+ }
+ }
+
}
\ No newline at end of file
diff --git a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/ContributorsServiceSpec.scala b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/ContributorsServiceSpec.scala
index 39aefb4..acefed1 100644
--- a/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/ContributorsServiceSpec.scala
+++ b/src/test/scala/com/googlesource/gerrit/plugins/analytics/test/ContributorsServiceSpec.scala
@@ -17,7 +17,7 @@
import java.lang.reflect.Type
import com.google.gerrit.acceptance.UseLocalDisk
-import com.google.gson.{Gson, JsonDeserializationContext, JsonDeserializer, JsonElement}
+import com.google.gson._
import com.googlesource.gerrit.plugins.analytics.UserActivitySummary
import com.googlesource.gerrit.plugins.analytics.common.AggregationStrategy.EMAIL_HOUR
import com.googlesource.gerrit.plugins.analytics.common.GsonFormatter
@@ -47,7 +47,7 @@
val stats = TestGson().fromJson(statsJson.getEntityContent, classOf[UserActivitySummary])
inside(stats) {
- case UserActivitySummary(_,_,_,_,theAuthorName,theAuthorEmail,numCommits,numFiles, numDistinctFiles, addedLines, deletedLines, commits, _, _, _, _, _, _) =>
+ case UserActivitySummary(_, _, _, _, theAuthorName, theAuthorEmail, numCommits, numFiles, numDistinctFiles, addedLines, deletedLines, commits, _, _, _, _, _, _) =>
theAuthorName shouldBe aContributorName
theAuthorEmail shouldBe aContributorEmail
numCommits shouldBe 1
@@ -68,9 +68,16 @@
json.getAsJsonArray.asScala.map(_.getAsString).toSet
}
+ class OptionDeserializer extends JsonDeserializer[Option[Any]] {
+ override def deserialize(jsonElement: JsonElement, `type`: Type, jsonDeserializationContext: JsonDeserializationContext): Option[Any] = {
+ Some(jsonElement)
+ }
+ }
+
def apply(): Gson =
new GsonFormatter()
- .gsonBuilder
- .registerTypeHierarchyAdapter(classOf[Iterable[String]], new SetStringDeserializer)
- .create()
+ .gsonBuilder
+ .registerTypeHierarchyAdapter(classOf[Iterable[String]], new SetStringDeserializer)
+ .registerTypeHierarchyAdapter(classOf[Option[Any]], new OptionDeserializer())
+ .create()
}
\ No newline at end of file
