diff --git a/admin/app/com/lucidchart/piezo/admin/controllers/JobDataMapPatch.scala b/admin/app/com/lucidchart/piezo/admin/controllers/JobDataMapPatch.scala new file mode 100644 index 00000000..56f6aa01 --- /dev/null +++ b/admin/app/com/lucidchart/piezo/admin/controllers/JobDataMapPatch.scala @@ -0,0 +1,112 @@ +package com.lucidchart.piezo.admin.controllers + +import org.quartz.JobKey +import play.api.libs.json.* + +/** + * A request to set some keys in a single job's data map, leaving the rest of the job definition alone. + * + * Only the supplied keys are written. Keys already on the job that the request does not mention keep their values, and + * there is no way to ask this endpoint to remove a key or clear a map. + * + * @param jobKey + * the job to update + * @param entries + * the keys to set, in order + */ +case class JobDataMapPatch(jobKey: JobKey, entries: List[DataMap]) + +object JobDataMapPatch { + + /** + * Parses the body of a bulk job data map update. + * + * The body looks like + * {{{ + * { + * "jobs": [ + * { + * "group": "sqs", + * "name": "enqueue-documents", + * "job-data-map": { "lastRunTime": "2026-08-30T17:00:00Z" } + * } + * ] + * } + * }}} + * + * `job-data-map` may also be given in the `[{"key": ..., "value": ...}]` shape that the job read endpoints return. It + * is required, so that a misspelled field name is a visible error rather than a request that reports success while + * changing nothing. + * + * Errors are collected across the whole `jobs` list, so a caller patching hundreds of jobs sees every bad entry at + * once rather than one per round trip. + */ + def parse(json: JsValue): JsResult[List[JobDataMapPatch]] = { + for { + jobs <- pathed((json \ "jobs").validate[List[JsValue]], JsPath \ "jobs") + patches <- traverse(jobs.zipWithIndex) { case (job, index) => + pathed(parseJob(job), JsPath \ "jobs" \ index) + } + } yield patches + } + + private def parseJob(json: JsValue): JsResult[JobDataMapPatch] = { + for { + group <- pathed((json \ "group").validate[String], JsPath \ "group") + name <- pathed((json \ "name").validate[String], JsPath \ "name") + entries <- pathed(parseEntries(json \ "job-data-map"), JsPath \ "job-data-map") + } yield JobDataMapPatch(new JobKey(name, group), entries) + } + + private def parseEntries(dataMap: JsLookupResult): JsResult[List[DataMap]] = { + dataMap.toOption match { + case Some(obj: JsObject) => + traverse(obj.fields.toList) { case (key, value) => + pathed(parseEntry(key, value), JsPath \ key) + } + case Some(JsArray(pairs)) => + traverse(pairs.toList.zipWithIndex) { case (pair, index) => + pathed(parsePairEntry(pair), JsPath \ index) + } + case _ => + JsError(JsPath, "Expected an object of key/value pairs or an array of {key, value} objects") + } + } + + private def parsePairEntry(pair: JsValue): JsResult[DataMap] = { + for { + key <- pathed((pair \ "key").validate[String], JsPath \ "key") + value <- (pair \ "value").toOption + .map(JsSuccess(_)) + .getOrElse(JsError(JsPath \ "value", "error.path.missing")) + entry <- pathed(parseEntry(key, value), JsPath \ "value") + } yield entry + } + + private def parseEntry(key: String, value: JsValue): JsResult[DataMap] = { + value match { + case JsString(string) => JsSuccess(DataMap(key, string)) + case JsNumber(number) => JsSuccess(DataMap(key, number.bigDecimal.toPlainString)) + case JsBoolean(boolean) => JsSuccess(DataMap(key, boolean.toString)) + case _ => JsError(JsPath, "Job data values must be a string, number, or boolean") + } + } + + private def pathed[A](result: JsResult[A], path: JsPath): JsResult[A] = { + result match { + case JsSuccess(value, _) => JsSuccess(value) + case error: JsError => error.repath(path) + } + } + + private def traverse[A, B](inputs: List[A])(f: A => JsResult[B]): JsResult[List[B]] = { + inputs.map(f).foldRight[JsResult[List[B]]](JsSuccess(Nil)) { (next, sofar) => + (next, sofar) match { + case (JsSuccess(value, _), JsSuccess(rest, _)) => JsSuccess(value +: rest) + case (JsError(errors), JsError(restErrors)) => JsError(errors ++ restErrors) + case (JsError(errors), _) => JsError(errors) + case (_, error: JsError) => error + } + } + } +} diff --git a/admin/app/com/lucidchart/piezo/admin/controllers/Jobs.scala b/admin/app/com/lucidchart/piezo/admin/controllers/Jobs.scala index deb75b31..0700b19d 100644 --- a/admin/app/com/lucidchart/piezo/admin/controllers/Jobs.scala +++ b/admin/app/com/lucidchart/piezo/admin/controllers/Jobs.scala @@ -485,6 +485,56 @@ class Jobs( ) } + def patchJobDataMaps: Action[JsValue] = Action(parse.json(maxFormSize)) { request => + JobDataMapPatch.parse(request.body) match { + case error: JsError => + BadRequest( + Json.obj( + "message" -> "Could not parse job data map updates", + "errors" -> JsError.toJson(error), + ), + ) + case JsSuccess(patches, _) => + val (failures, updated) = patches.map(applyJobDataMapPatch).partitionMap(identity) + val body = Json.obj( + "count" -> patches.size, + "updated" -> updated.map(jobKeyJson), + "failures" -> failures, + ) + if (failures.isEmpty) Ok(body) else MultiStatus(body) + } + } + + private def applyJobDataMapPatch(patch: JobDataMapPatch): Either[JsObject, JobKey] = { + val jobKey = patch.jobKey + try { + Option(scheduler.getJobDetail(jobKey)) match { + case None => + Left(patchFailure(jobKey, "Job %s %s not found".format(jobKey.getGroup, jobKey.getName))) + case Some(jobDetail) => + patch.entries.foreach(entry => jobDetail.getJobDataMap.put(entry.key, entry.value)) + scheduler.addJob(jobDetail, true, true) + logger.info("Updated job data map for job %s %s".format(jobKey.getGroup, jobKey.getName)) + Right(jobKey) + } + } catch { + case e: Exception => + val errorMsg = "Exception caught updating the job data map of job %s %s. -- %s".format( + jobKey.getGroup, + jobKey.getName, + e.getLocalizedMessage(), + ) + logger.error(errorMsg, e) + Left(patchFailure(jobKey, errorMsg)) + } + } + + private def jobKeyJson(jobKey: JobKey): JsObject = + Json.obj("jobGroup" -> jobKey.getGroup, "jobName" -> jobKey.getName) + + private def patchFailure(jobKey: JobKey, errorMessage: String): JsObject = + jobKeyJson(jobKey) ++ Json.obj("errorMessage" -> errorMessage) + def jobNameTypeAhead(group: String): Action[AnyContent] = Action { request => val jobs = diff --git a/admin/conf/routes b/admin/conf/routes index 6af7c4b1..c020f747 100644 --- a/admin/conf/routes +++ b/admin/conf/routes @@ -16,6 +16,8 @@ POST /data/jobs com.lucidchart.piezo.admin.controller GET /data/jobs com.lucidchart.piezo.admin.controllers.Jobs.getJobsDetail GET /data/jobs/:group/:name com.lucidchart.piezo.admin.controllers.Jobs.getJobDetail(group: String, name: String) +POST /data/jobs/job-data-map com.lucidchart.piezo.admin.controllers.Jobs.patchJobDataMaps + GET /typeahead/jobs/:group com.lucidchart.piezo.admin.controllers.Jobs.jobNameTypeAhead(group: String) GET /triggers com.lucidchart.piezo.admin.controllers.Triggers.getIndex diff --git a/admin/test/com/lucidchart/piezo/admin/controllers/JobDataMapPatchTest.scala b/admin/test/com/lucidchart/piezo/admin/controllers/JobDataMapPatchTest.scala new file mode 100644 index 00000000..d26888f0 --- /dev/null +++ b/admin/test/com/lucidchart/piezo/admin/controllers/JobDataMapPatchTest.scala @@ -0,0 +1,89 @@ +package com.lucidchart.piezo.admin.controllers + +import org.specs2.mutable.* +import play.api.libs.json.* + +class JobDataMapPatchTest extends Specification { + + private def parseOne(json: JsValue): JobDataMapPatch = { + JobDataMapPatch.parse(json) match { + case JsSuccess(List(patch), _) => patch + case other => throw new AssertionError(s"Expected exactly one patch, got $other") + } + } + + private def body(dataMap: JsValue): JsValue = { + Json.obj("jobs" -> Json.arr(Json.obj("group" -> "g", "name" -> "n", "job-data-map" -> dataMap))) + } + + "JobDataMapPatch.parse" should { + + "read the job key and the keys to set" in { + val patch = parseOne(body(Json.obj("a" -> "1"))) + patch.jobKey.getGroup must equalTo("g") + patch.jobKey.getName must equalTo("n") + patch.entries must equalTo(List(DataMap("a", "1"))) + } + + "stringify numbers and booleans" in { + val entries = parseOne(body(Json.obj("i" -> 1756400000000L, "d" -> 1.5, "b" -> true))).entries + entries must containTheSameElementsAs( + List(DataMap("i", "1756400000000"), DataMap("d", "1.5"), DataMap("b", "true")), + ) + } + + "not use scientific notation for large numbers" in { + parseOne(body(Json.obj("i" -> BigDecimal("1e20")))).entries must equalTo( + List(DataMap("i", "100000000000000000000")), + ) + } + + "accept the key/value array shape returned by the read endpoints" in { + val dataMap = Json.arr(Json.obj("key" -> "a", "value" -> "1"), Json.obj("key" -> "b", "value" -> "2")) + parseOne(body(dataMap)).entries must equalTo(List(DataMap("a", "1"), DataMap("b", "2"))) + } + + "accept an empty data map as a job with nothing to change" in { + parseOne(body(Json.obj())).entries must beEmpty + } + + "reject a null value rather than read it as a removal" in { + JobDataMapPatch.parse(body(Json.obj("a" -> JsNull))) must beAnInstanceOf[JsError] + } + + "reject a null value in the key/value array shape" in { + val dataMap = Json.arr(Json.obj("key" -> "a", "value" -> JsNull)) + JobDataMapPatch.parse(body(dataMap)) must beAnInstanceOf[JsError] + } + + "reject a value that is not a scalar" in { + JobDataMapPatch.parse(body(Json.obj("a" -> Json.obj("b" -> "1")))) must beAnInstanceOf[JsError] + } + + "reject a job with no data map, so a misspelled field is not a silent no-op" in { + val json = Json.obj("jobs" -> Json.arr(Json.obj("group" -> "g", "name" -> "n", "jobDataMap" -> Json.obj()))) + JobDataMapPatch.parse(json) must beAnInstanceOf[JsError] + } + + "reject a body with no jobs field" in { + JobDataMapPatch.parse(Json.obj()) must beAnInstanceOf[JsError] + } + + "report every bad job in one response" in { + val json = Json.obj( + "jobs" -> Json.arr( + Json.obj("name" -> "missing-group", "job-data-map" -> Json.obj()), + Json.obj("group" -> "g", "name" -> "n", "job-data-map" -> Json.obj()), + Json.obj("group" -> "g", "name" -> "n", "job-data-map" -> Json.obj("a" -> Json.arr())), + ), + ) + JobDataMapPatch.parse(json) match { + case JsError(errors) => + errors.map(_._1.toJsonString).toList must containTheSameElementsAs( + List("obj.jobs[0].group", "obj.jobs[2].job-data-map.a"), + ) + case other => throw new AssertionError(s"Expected errors, got $other") + } + } + } +} diff --git a/admin/test/com/lucidchart/piezo/admin/controllers/JobDataMapService.scala b/admin/test/com/lucidchart/piezo/admin/controllers/JobDataMapService.scala new file mode 100644 index 00000000..c7b1f801 --- /dev/null +++ b/admin/test/com/lucidchart/piezo/admin/controllers/JobDataMapService.scala @@ -0,0 +1,228 @@ +package com.lucidchart.piezo.admin.controllers + +import ch.qos.logback.classic.{Level, Logger} +import com.lucidchart.piezo.WorkerSchedulerFactory +import com.lucidchart.piezo.admin.models.MonitoringTeams +import com.lucidchart.piezo.jobs.monitoring.HeartBeat +import java.util.Properties +import org.quartz.{JobBuilder, JobKey, Scheduler, SimpleScheduleBuilder, TriggerBuilder, TriggerKey} +import org.slf4j.LoggerFactory +import org.specs2.mutable.* +import play.api.Configuration +import play.api.libs.json.* +import play.api.mvc.Result +import play.api.test.* +import play.api.test.Helpers.* +import scala.concurrent.Future +import scala.jdk.CollectionConverters.* + +class JobDataMapService extends Specification { + val rootLogger: Logger = LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME).asInstanceOf[Logger] + rootLogger.setLevel(Level.DEBUG) + + private val jobView = new com.lucidchart.piezo.admin.views.html.job(Configuration.empty) + private val group = "dataMapTestGroup" + + private def newScheduler(): Scheduler = { + val schedulerFactory = new WorkerSchedulerFactory() + val properties = new Properties + properties.load(getClass().getResourceAsStream("/quartz_test.properties")) + schedulerFactory.initialize(properties) + schedulerFactory.getScheduler() + } + + private def controller(scheduler: Scheduler) = + new Jobs( + scheduler, + TestUtil.mockModelComponents, + jobView, + Helpers.stubControllerComponents(), + MonitoringTeams.empty, + ) + + /** + * Creates a durable job with the given data map, plus a trigger, so tests can check that a patch leaves the rest of + * the job alone. + */ + private def createJob( + scheduler: Scheduler, + name: String, + data: Map[String, String], + durable: Boolean = true, + ): JobKey = { + val jobDetail = JobBuilder + .newJob(classOf[HeartBeat]) + .withIdentity(name, group) + .withDescription("a job with a data map") + .storeDurably(durable) + .requestRecovery(true) + .usingJobData(new org.quartz.JobDataMap(data.asJava)) + .build() + val trigger = TriggerBuilder.newTrigger + .withIdentity(name, group) + .forJob(jobDetail) + .withSchedule(SimpleScheduleBuilder.simpleSchedule.withIntervalInSeconds(5).withRepeatCount(1)) + .build() + scheduler.deleteJob(jobDetail.getKey) + scheduler.scheduleJob(jobDetail, trigger) + jobDetail.getKey + } + + private def patch(scheduler: Scheduler, body: JsValue): Future[Result] = { + val request = FakeRequest(PATCH, "/data/jobs/job-data-map").withBody(body) + controller(scheduler).patchJobDataMaps(request) + } + + private def dataMapOf(scheduler: Scheduler, jobKey: JobKey): Map[String, AnyRef] = { + scheduler.getJobDetail(jobKey).getJobDataMap.asScala.toMap + } + + "patchJobDataMaps" should { + + "merge the given keys into the existing data map" in { + val scheduler = newScheduler() + val jobKey = createJob(scheduler, "merge", Map("lastRunTime" -> "100", "queue" -> "documents")) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj( + "group" -> group, + "name" -> "merge", + "job-data-map" -> Json.obj("lastRunTime" -> "50"), + ), + ), + ), + ) + + status(result) must equalTo(OK) + (contentAsJson(result) \ "count").as[Int] must equalTo(1) + (contentAsJson(result) \ "failures").as[JsArray].value must beEmpty + dataMapOf(scheduler, jobKey) must equalTo(Map("lastRunTime" -> "50", "queue" -> "documents")) + } + + "leave the rest of the job and its triggers alone" in { + val scheduler = newScheduler() + val jobKey = createJob(scheduler, "untouched", Map("lastRunTime" -> "100")) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj("group" -> group, "name" -> "untouched", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + ), + ), + ) + + status(result) must equalTo(OK) + val jobDetail = scheduler.getJobDetail(jobKey) + jobDetail.getDescription must equalTo("a job with a data map") + jobDetail.getJobClass must equalTo(classOf[HeartBeat]) + jobDetail.isDurable must beTrue + jobDetail.requestsRecovery must beTrue + scheduler.getTriggersOfJob(jobKey).asScala.map(_.getKey) must equalTo( + Seq(new TriggerKey("untouched", group)), + ) + } + + "refuse to remove a key, leaving the job untouched" in { + val scheduler = newScheduler() + val jobKey = createJob(scheduler, "remove", Map("lastRunTime" -> "100", "stale" -> "yes")) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj("group" -> group, "name" -> "remove", "job-data-map" -> Json.obj("stale" -> JsNull)), + ), + ), + ) + + status(result) must equalTo(BAD_REQUEST) + dataMapOf(scheduler, jobKey) must equalTo(Map("lastRunTime" -> "100", "stale" -> "yes")) + } + + "patch every job in one request" in { + val scheduler = newScheduler() + val first = createJob(scheduler, "bulk1", Map("lastRunTime" -> "100")) + val second = createJob(scheduler, "bulk2", Map("lastRunTime" -> "200")) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj("group" -> group, "name" -> "bulk1", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + Json.obj("group" -> group, "name" -> "bulk2", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + ), + ), + ) + + status(result) must equalTo(OK) + (contentAsJson(result) \ "updated").as[JsArray].value must haveSize(2) + dataMapOf(scheduler, first) must equalTo(Map("lastRunTime" -> "50")) + dataMapOf(scheduler, second) must equalTo(Map("lastRunTime" -> "50")) + } + + "report a missing job without skipping the rest" in { + val scheduler = newScheduler() + val jobKey = createJob(scheduler, "present", Map("lastRunTime" -> "100")) + scheduler.deleteJob(new JobKey("absent", group)) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj("group" -> group, "name" -> "absent", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + Json.obj("group" -> group, "name" -> "present", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + ), + ), + ) + + status(result) must equalTo(MULTI_STATUS) + val json = contentAsJson(result) + (json \ "count").as[Int] must equalTo(2) + (json \ "updated").as[JsArray].value must haveSize(1) + (json \ "failures" \ 0 \ "jobName").as[String] must equalTo("absent") + (json \ "failures" \ 0 \ "errorMessage").as[String] must contain("not found") + dataMapOf(scheduler, jobKey) must equalTo(Map("lastRunTime" -> "50")) + } + + "patch a non durable job that is only kept around by its trigger" in { + val scheduler = newScheduler() + val jobKey = createJob(scheduler, "nonDurable", Map("lastRunTime" -> "100"), durable = false) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj("group" -> group, "name" -> "nonDurable", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + ), + ), + ) + + status(result) must equalTo(OK) + dataMapOf(scheduler, jobKey) must equalTo(Map("lastRunTime" -> "50")) + scheduler.getJobDetail(jobKey).isDurable must beFalse + scheduler.getTriggersOfJob(jobKey).asScala must haveSize(1) + } + + "apply nothing when the body is malformed" in { + val scheduler = newScheduler() + val jobKey = createJob(scheduler, "malformed", Map("lastRunTime" -> "100")) + + val result = patch( + scheduler, + Json.obj( + "jobs" -> Json.arr( + Json.obj("group" -> group, "name" -> "malformed", "job-data-map" -> Json.obj("lastRunTime" -> "50")), + Json.obj("group" -> group, "name" -> "no-data-map-here"), + ), + ), + ) + + status(result) must equalTo(BAD_REQUEST) + dataMapOf(scheduler, jobKey) must equalTo(Map("lastRunTime" -> "100")) + } + } +}