Skip to content

Commit

Permalink
Clean up serializer/deserializer naming and prevent OOI issues
Browse files Browse the repository at this point in the history
For some reason, after 10 or so years, some test code started
throwing NPEs after an unrelated changeset due to order-of-initial-
isation issues in the JSON format objects. Where these are not
macro-generated they're now `lazy vals`. Also rename a ton of the
other reads/writes/format objects for consistency, with a leading
'_' to make it clear they're auxilliary, often macro-generated
companion object implicits.
  • Loading branch information
mikesname committed Oct 15, 2024
1 parent 5431309 commit e5e2cce
Show file tree
Hide file tree
Showing 55 changed files with 428 additions and 445 deletions.
2 changes: 1 addition & 1 deletion modules/admin/app/client/json/ClientWriteable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ package client.json
import play.api.libs.json.Writes

trait ClientWriteable[T] {
def clientFormat: Writes[T]
def _clientFormat: Writes[T]
}
304 changes: 152 additions & 152 deletions modules/admin/app/client/json/package.scala

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion modules/admin/app/controllers/admin/AdminSearch.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ case class AdminSearch @Inject()(
render {
case Accepts.Json() => Ok(Json.obj(
"numPages" -> result.page.numPages,
"page" -> Json.toJson(result.page.items.map(_._1))(Writes.seq(client.json.anyModelJson.clientFormat)),
"page" -> Json.toJson(result.page.items.map(_._1))(Writes.seq(client.json.anyModelJson._clientFormat)),
"facets" -> result.facetClasses
))
case _ => Ok(views.html.admin.search.search(result, controllers.admin.routes.AdminSearch.search()))
Expand Down
2 changes: 1 addition & 1 deletion modules/admin/app/controllers/admin/Metrics.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ case class Metrics @Inject()(
private def jsonResponse[T](result: SearchResult[(T, SearchHit)])(implicit request: Request[AnyContent], w: ClientWriteable[T]): Result = {
render {
case Accepts.Json() | Accepts.JavaScript() => Ok(Json.obj(
"page" -> Json.toJson(result.mapItems(_._1).page)(pageWrites(w.clientFormat)),
"page" -> Json.toJson(result.mapItems(_._1).page)(pageWrites(w._clientFormat)),
"params" -> result.params,
"appliedFacets" -> result.facets,
"facetClasses" -> result.facetClasses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ case class VocabularyEditor @Inject()(
) extends AdminController with Read[Vocabulary] with Search {

private val maxSize = config.underlying.getBytes("ehri.admin.vocabEditor.maxPayloadSize")
private val clientFormFormat = client.json.conceptJson.fFormat
private val clientFormat = client.json.conceptJson.clientFormat
private val clientFormFormat = client.json.conceptJson._formFormat
private val _clientFormat = client.json.conceptJson._clientFormat

private case class AccountRequest[A](account: Account, request: Request[A]) extends WrappedRequest[A](request)

Expand Down Expand Up @@ -137,7 +137,7 @@ case class VocabularyEditor @Inject()(

def get(id: String, cid: String): Action[AnyContent] = UserAction(parse.anyContent).async { implicit request =>
userDataApi.get[Concept](cid).map { item =>
Ok(Json.toJson(item)(clientFormat))
Ok(Json.toJson(item)(_clientFormat))
}
}

Expand All @@ -159,17 +159,17 @@ case class VocabularyEditor @Inject()(

def broader(id: String, cid: String): Action[Seq[String]] = UserAction(parse.json[Seq[String]]).async { implicit request =>
userDataApi.parent[Concept, Concept](cid, request.body).map { item =>
Ok(Json.toJson(item)(clientFormat))
Ok(Json.toJson(item)(_clientFormat))
}
}

def updateItem(id: String, cid: String): Action[JsValue] = UserAction(parse.json(maxSize)).async { implicit request =>
request.body.validate[Concept](clientFormat).fold(
request.body.validate[Concept](_clientFormat).fold(
invalid => Future.successful(BadRequest(JsError.toJson(invalid))),
data => {
userDataApi.update[Concept, ConceptF](cid, data.data).map { item =>
implicit val conceptFormat: Writes[Concept] = client.json.conceptJson.clientFormat
Ok(Json.toJson(item)(clientFormat))
implicit val conceptFormat: Writes[Concept] = client.json.conceptJson._clientFormat
Ok(Json.toJson(item)(_clientFormat))
}.recover {
case e: ValidationError => BadRequest(Json.toJson(e.errorSet.errors))
}
Expand All @@ -182,7 +182,7 @@ case class VocabularyEditor @Inject()(
invalid => Future.successful(BadRequest(JsError.toJson(invalid))),
data => {
userDataApi.createInContext[Vocabulary, ConceptF, Concept](id, data).map { item =>
Created(Json.toJson(item)(clientFormat))
Created(Json.toJson(item)(_clientFormat))
}.recover {
case e: ValidationError => BadRequest(Json.toJson(e.errorSet.errors))
}
Expand Down
18 changes: 9 additions & 9 deletions modules/admin/app/models/IngestResult.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ sealed trait IngestResult

object IngestResult {

implicit val reads: Reads[IngestResult] = Reads { json =>
implicit val _reads: Reads[IngestResult] = Reads { json =>
json.validate[ImportValidationError]
.map(e => ErrorLog(s"Validation error at ${e.context}", e.toString))
.orElse(json.validate[ErrorLog])
.orElse(json.validate[ImportLog])
.orElse(json.validate[SyncLog])
}
implicit val writes: Writes[IngestResult] = Writes {
case i: ImportLog => Json.toJson(i)(ImportLog.format)
case i: SyncLog => Json.toJson(i)(SyncLog.format)
case i: ErrorLog => Json.toJson(i)(ErrorLog.format)
implicit val _writes: Writes[IngestResult] = Writes {
case i: ImportLog => Json.toJson(i)(ImportLog._format)
case i: SyncLog => Json.toJson(i)(SyncLog._format)
case i: ErrorLog => Json.toJson(i)(ErrorLog._format)
}
implicit val format: Format[IngestResult] = Format(reads, writes)
implicit val _format: Format[IngestResult] = Format(_reads, _writes)
}

case class ImportValidationError(context: String, errorSet: ErrorSet) extends RuntimeException(errorSet.toString) {
Expand Down Expand Up @@ -56,7 +56,7 @@ case class ImportLog(

object ImportLog {
implicit val config: Aux[Json.MacroOptions] = JsonConfiguration(SnakeCase)
implicit val format: Format[ImportLog] = Json.format[ImportLog]
implicit val _format: Format[ImportLog] = Json.format[ImportLog]
}

// The result of an EAD synchronisation, which incorporates an import
Expand All @@ -68,14 +68,14 @@ case class SyncLog(
) extends IngestResult

object SyncLog {
implicit val format: Format[SyncLog] = Json.format[SyncLog]
implicit val _format: Format[SyncLog] = Json.format[SyncLog]
}

// An import error we can understand, e.g. not a crash!
case class ErrorLog(error: String, details: String) extends IngestResult

object ErrorLog {
implicit val format: Format[ErrorLog] = Json.format[ErrorLog]
implicit val _format: Format[ErrorLog] = Json.format[ErrorLog]
}


38 changes: 19 additions & 19 deletions modules/api/app/models/api/v1/JsonApiV1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ object JsonApiV1 {
)

object DocumentaryUnitDescriptionAttrs {
implicit val writes: Writes[DocumentaryUnitDescriptionAttrs] = Json.writes[DocumentaryUnitDescriptionAttrs]
implicit val _writes: Writes[DocumentaryUnitDescriptionAttrs] = Json.writes[DocumentaryUnitDescriptionAttrs]

def apply(d: DocumentaryUnitDescriptionF)(implicit messages: Messages): DocumentaryUnitDescriptionAttrs =
new DocumentaryUnitDescriptionAttrs(
Expand Down Expand Up @@ -66,7 +66,7 @@ object JsonApiV1 {
)

object DocumentaryUnitAttrs {
implicit val writes: Writes[DocumentaryUnitAttrs] = Json.writes[DocumentaryUnitAttrs]
implicit val _writes: Writes[DocumentaryUnitAttrs] = Json.writes[DocumentaryUnitAttrs]

def apply(d: DocumentaryUnit)(implicit messages: Messages): DocumentaryUnitAttrs =
new DocumentaryUnitAttrs(
Expand All @@ -92,7 +92,7 @@ object JsonApiV1 {
)

object AddressAttrs {
implicit val writes: Writes[AddressAttrs] = Json.writes[AddressAttrs]
implicit val _writes: Writes[AddressAttrs] = Json.writes[AddressAttrs]

def apply(a: AddressF)(implicit messages: Messages): AddressAttrs = new AddressAttrs(
a.name,
Expand Down Expand Up @@ -133,7 +133,7 @@ object JsonApiV1 {
)

object RepositoryAttrs {
implicit val writes: Writes[RepositoryAttrs] = Json.writes[RepositoryAttrs]
implicit val _writes: Writes[RepositoryAttrs] = Json.writes[RepositoryAttrs]

def apply(r: Repository)(implicit messages: Messages): RepositoryAttrs = {
r.data.primaryDescription.map { d =>
Expand Down Expand Up @@ -172,7 +172,7 @@ object JsonApiV1 {
)

object DocumentaryUnitLinks {
implicit val writes: Writes[DocumentaryUnitLinks] = Json.writes[DocumentaryUnitLinks]
implicit val _writes: Writes[DocumentaryUnitLinks] = Json.writes[DocumentaryUnitLinks]
}

case class DocumentaryUnitRelations(
Expand All @@ -184,7 +184,7 @@ object JsonApiV1 {
// Not using default writes here because missing (Optional)
// relations should be expressed using null
// http://jsonapi.org/format/#document-resource-object-related-resource-links
implicit val writes: Writes[DocumentaryUnitRelations] = Writes[DocumentaryUnitRelations] { r =>
implicit val _writes: Writes[DocumentaryUnitRelations] = Writes[DocumentaryUnitRelations] { r =>
Json.obj(
"holder" -> r.holder,
"parent" -> r.parent
Expand All @@ -206,7 +206,7 @@ object JsonApiV1 {
)

object HistoricalAgentAttrs {
implicit val writes: Writes[HistoricalAgentAttrs] = Json.writes[HistoricalAgentAttrs]
implicit val _writes: Writes[HistoricalAgentAttrs] = Json.writes[HistoricalAgentAttrs]

def apply(a: HistoricalAgent)(implicit messages: Messages): HistoricalAgentAttrs = {
a.data.primaryDescription.map { d =>
Expand Down Expand Up @@ -234,7 +234,7 @@ object JsonApiV1 {
)

object HistoricalAgentLinks {
implicit val writes: Writes[HistoricalAgentLinks] = Json.writes[HistoricalAgentLinks]
implicit val _writes: Writes[HistoricalAgentLinks] = Json.writes[HistoricalAgentLinks]
}

case class RepositoryLinks(
Expand All @@ -244,15 +244,15 @@ object JsonApiV1 {
)

object RepositoryLinks {
implicit val writes: Writes[RepositoryLinks] = Json.writes[RepositoryLinks]
implicit val _writes: Writes[RepositoryLinks] = Json.writes[RepositoryLinks]
}

case class RepositoryRelations(
country: Option[JsValue]
)

object RepositoryRelations {
implicit val writes: Writes[RepositoryRelations] = Json.writes[RepositoryRelations]
implicit val _writes: Writes[RepositoryRelations] = Json.writes[RepositoryRelations]
}

case class CountryAttrs(
Expand All @@ -266,7 +266,7 @@ object JsonApiV1 {
)

object CountryAttrs {
implicit val writes: Writes[CountryAttrs] = Json.writes[CountryAttrs]
implicit val _writes: Writes[CountryAttrs] = Json.writes[CountryAttrs]

def apply(c: Country)(implicit requestHeader: RequestHeader, messages: Messages): CountryAttrs =
new CountryAttrs(
Expand All @@ -286,7 +286,7 @@ object JsonApiV1 {
)

object CountryLinks {
implicit val writes: Writes[CountryLinks] = Json.writes[CountryLinks]
implicit val _writes: Writes[CountryLinks] = Json.writes[CountryLinks]
}

case class ConceptDescriptionAttrs(
Expand All @@ -302,7 +302,7 @@ object JsonApiV1 {
)

object ConceptDescriptionAttrs {
implicit val writes: Writes[ConceptDescriptionAttrs] = Json.writes[ConceptDescriptionAttrs]
implicit val _writes: Writes[ConceptDescriptionAttrs] = Json.writes[ConceptDescriptionAttrs]

def apply(d: ConceptDescriptionF)(implicit messages: Messages): ConceptDescriptionAttrs = new ConceptDescriptionAttrs(
languageCode = d.languageCode,
Expand All @@ -326,7 +326,7 @@ object JsonApiV1 {
)

object ConceptAttrs {
implicit val writes: Writes[ConceptAttrs] = Json.writes[ConceptAttrs]
implicit val _writes: Writes[ConceptAttrs] = Json.writes[ConceptAttrs]

def apply(c: Concept)(implicit requestHeader: RequestHeader, messages: Messages): ConceptAttrs =
new ConceptAttrs(
Expand All @@ -347,7 +347,7 @@ object JsonApiV1 {
)

object ConceptLinks {
implicit val writes: Writes[ConceptLinks] = Json.writes[ConceptLinks]
implicit val _writes: Writes[ConceptLinks] = Json.writes[ConceptLinks]
}

case class JsonApiResponse(
Expand Down Expand Up @@ -380,7 +380,7 @@ object JsonApiV1 {
)

object ResourceIdentifier {
implicit val writes: Writes[ResourceIdentifier] = Json.writes[ResourceIdentifier]
implicit val _writes: Writes[ResourceIdentifier] = Json.writes[ResourceIdentifier]

def apply(m: Model) = new ResourceIdentifier(m.id, m.isA.toString)
}
Expand All @@ -393,7 +393,7 @@ object JsonApiV1 {
)

object PaginationLinks {
implicit val writes: Writes[PaginationLinks] = Json.writes[PaginationLinks]
implicit val _writes: Writes[PaginationLinks] = Json.writes[PaginationLinks]
}

case class JsonApiError(
Expand All @@ -406,7 +406,7 @@ object JsonApiV1 {
)

object JsonApiError {
implicit val writes: Writes[JsonApiError] = Json.writes[JsonApiError]
implicit val _writes: Writes[JsonApiError] = Json.writes[JsonApiError]
}

case class GeoPoint(
Expand All @@ -415,7 +415,7 @@ object JsonApiV1 {
)

object GeoPoint {
implicit val writes: Writes[GeoPoint] = Writes { p =>
implicit val _writes: Writes[GeoPoint] = Writes { p =>
Json.obj(
"type" -> "Point",
"coordinates" -> Json.arr(p.latitude, p.longitude)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ object GlobalPermissionSet {
* value in each, converting them to a tuple for internal use.
*
*/
implicit val restReads: Reads[GlobalPermissionSet] = Reads.seq(Reads.map(Reads.map(Reads.seq[String]))).map { pd =>
implicit val _reads: Reads[GlobalPermissionSet] = Reads.seq(Reads.map(Reads.map(Reads.seq[String]))).map { pd =>
pd.flatMap { pmap =>
pmap.headOption.map { case (user, perms) =>
(user, perms.flatMap {
Expand All @@ -45,6 +45,5 @@ case class GlobalPermissionSet(data: GlobalPermissionSet.PermData) extends Permi
* Check if this permission set has the given permission.
*/
def has(sub: ContentTypes.Value, permission: PermissionType.Value): Boolean =
data.flatMap(_._2.get(sub)).filter(plist => plist.exists(p =>
PermissionType.in(p, permission))).nonEmpty
data.flatMap(_._2.get(sub)).exists(plist => plist.exists(p => PermissionType.in(p, permission)))
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ object ItemPermissionSet {
* a less stringly typed version: all the entity types and permissions
* should all correspond to Enum values in ContentType and PermissionType.
*/
implicit def restReads(contentType: ContentTypes.Value): Reads[ItemPermissionSet] = Reads.seq(Reads.map(Reads.seq[String])).map { pd =>
implicit def _reads(contentType: ContentTypes.Value): Reads[ItemPermissionSet] = Reads.seq(Reads.map(Reads.seq[String])).map { pd =>
// Raw data is a Seq[Map[String, Seq[String]]]
import scala.util.control.Exception._
pd.flatMap { userPermMap =>
Expand Down
2 changes: 1 addition & 1 deletion modules/backend/src/main/scala/models/Readable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import scala.annotation.implicitNotFound
*/
@implicitNotFound("No member of type class Readable found for type ${T}")
trait Readable[T] {
val restReads: Reads[T]
def _reads: Reads[T]
}

object Readable {
Expand Down
2 changes: 1 addition & 1 deletion modules/backend/src/main/scala/models/Writable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import scala.annotation.implicitNotFound
*/
@implicitNotFound("No member of type class Writable found for type ${T}")
trait Writable[T] {
val restFormat: Format[T]
def _format: Format[T]
}

object Writable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import scala.concurrent.{ExecutionContext, Future}
private case class WsCypherResultData(row: List[JsValue], meta: Seq[JsValue])

private object WsCypherResultData {
implicit val reads: Reads[WsCypherResultData] = (
implicit val _reads: Reads[WsCypherResultData] = (
(__ \ "row").read[List[JsValue]] and
(__ \ "meta").read[Seq[JsValue]]
)(WsCypherResultData.apply _)
Expand All @@ -31,7 +31,7 @@ private case class WsCypherResult(columns: Seq[String], data: Seq[WsCypherResult
}

private object WsCypherResult {
implicit val reads: Reads[WsCypherResult] = (
implicit val _reads: Reads[WsCypherResult] = (
(__ \ "columns").read[Seq[String]] and
(__ \ "data").read[Seq[WsCypherResultData]]
)(WsCypherResult.apply _)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ import play.api.libs.json.{Format, Json}
case class BatchResult(created: Int, updated: Int, unchanged: Int, errors: Map[String, String])

object BatchResult {
implicit val format: Format[BatchResult] = Json.format[BatchResult]
implicit val _format: Format[BatchResult] = Json.format[BatchResult]
}
Loading

0 comments on commit e5e2cce

Please sign in to comment.