Dynamic Entity tests. response type

obp-api/src/test/scala/code/api/util/http4s/Http4sJsonContentTypeTest.scala

@@ -0,0 +1,88 @@
+package code.api.util.http4s
+
+import cats.effect.IO
+import cats.effect.unsafe.implicits.global
+import code.api.util.CallContext
+import code.api.util.http4s.Http4sRequestAttributes.{EndpointHelpers, callContextKey}
+import net.liftweb.json.{DefaultFormats, Formats}
+import org.http4s.{Request, Response}
+import org.scalatest.{FeatureSpec, GivenWhenThen, Matchers}
+import org.typelevel.ci.CIString
+
+import scala.concurrent.Future
+
+/**
+ * Regression tests for the native http4s endpoint response helpers in
+ * `Http4sRequestAttributes.EndpointHelpers`.
+ *
+ * These helpers render a Lift-JSON String and hand it to http4s' `Ok`/`Created`. Passing a
+ * raw String uses the default `EntityEncoder[String]`, which labels the response
+ * `Content-Type: text/plain; charset=UTF-8`. OBP only ever returns JSON from these helpers,
+ * so the content type must be `application/json` — otherwise strict clients (e.g. the
+ * API Manager frontend) reject a perfectly good JSON body.
+ *
+ * This was a real bug on the http4s-migrated dynamic-entity CRUD endpoints. The older Lift
+ * endpoints and the Lift -> http4s bridge set `application/json` correctly (covered by
+ * Http4sResponseConversionTest); this test pins the *native* http4s builders.
+ */
+class Http4sJsonContentTypeTest extends FeatureSpec with Matchers with GivenWhenThen {
+
+  private implicit val formats: Formats = DefaultFormats
+
+  /** A request carrying a (default) CallContext, as the helpers expect. */
+  private def reqWithCallContext: Request[IO] =
+    Request[IO]().withAttribute(callContextKey, CallContext())
+
+  private def contentTypeOf(resp: Response[IO]): String =
+    resp.headers.get(CIString("Content-Type")).map(_.head.value).getOrElse("")
+
+  feature("Native http4s endpoint helpers label JSON responses as application/json") {
+
+    scenario("executeAndRespond (200 OK) sets application/json") {
+      Given("a 200 helper returning a JSON object")
+      When("the response is built")
+      val resp = EndpointHelpers
+        .executeAndRespond(reqWithCallContext)(_ => Future.successful(Map("message" -> "ok")))
+        .unsafeRunSync()
+
+      Then("status is 200 and Content-Type is application/json")
+      resp.status.code should equal(200)
+      contentTypeOf(resp) should include("application/json")
+    }
+
+    scenario("executeFutureCreated (201 Created) sets application/json") {
+      Given("a 201 helper returning a JSON object")
+      When("the response is built")
+      val resp = EndpointHelpers
+        .executeFutureCreated(reqWithCallContext)(Future.successful(Map("id" -> "123")))
+        .unsafeRunSync()
+
+      Then("status is 201 and Content-Type is application/json")
+      resp.status.code should equal(201)
+      contentTypeOf(resp) should include("application/json")
+    }
+
+    scenario("executeFutureWithStatus sets application/json for a custom status") {
+      Given("a helper returning a JSON object with an explicit 202 status")
+      When("the response is built")
+      val resp = EndpointHelpers
+        .executeFutureWithStatus(reqWithCallContext)(Future.successful((Map("queued" -> "true"), 202)))
+        .unsafeRunSync()
+
+      Then("status is 202 and Content-Type is application/json")
+      resp.status.code should equal(202)
+      contentTypeOf(resp) should include("application/json")
+    }
+
+    scenario("regression: helpers must NOT fall back to text/plain") {
+      Given("the 201 helper (the path the dynamic-entity create endpoint uses)")
+      When("the response is built")
+      val resp = EndpointHelpers
+        .executeFutureCreated(reqWithCallContext)(Future.successful(Map("k" -> "v")))
+        .unsafeRunSync()
+
+      Then("the Content-Type must not be text/plain")
+      contentTypeOf(resp) should not include "text/plain"
+    }
+  }
+}

obp-api/src/test/scala/code/api/v4_0_0/DynamicEntityTest.scala

@@ -1145,11 +1145,16 @@
         val requestCreateFoobar = (dynamicEntity_Request / "FooBar").POST <@(user1)
         val responseCreateFoobar = makePostRequest(requestCreateFoobar, write(foobarObject))
         responseCreateFoobar.code should equal(201)
+        // Regression: the native http4s dynamic-entity endpoints must label JSON responses as
+        // application/json. http4s' default String EntityEncoder otherwise sets text/plain, which
+        // strict JSON clients (e.g. the API Manager frontend) reject even though the body is valid JSON.
+        responseCreateFoobar.headers.map(_.get("Content-Type")).getOrElse("").toLowerCase should include("application/json")
         val dynamicEntityId = (responseCreateFoobar.body \ "foo_bar" \ "foo_bar_id").asInstanceOf[JString].s
 
         val requestGetFoobars = (dynamicEntity_Request / "FooBar").GET <@(user1)
         val responseGetFoobars = makeGetRequest(requestGetFoobars)
         responseGetFoobars.code should equal(200)
+        responseGetFoobars.headers.map(_.get("Content-Type")).getOrElse("").toLowerCase should include("application/json")
 
         val requestGetFoobar = (dynamicEntity_Request / "FooBar" / dynamicEntityId ).GET <@(user1)
         val responseGetFoobar = makeGetRequest(requestGetFoobar)
@@ -1171,6 +1176,10 @@
           val requestCreateFoobar = (dynamicEntity_Request / "FooBar").POST <@(user2)
           val responseCreateFoobar = makePostRequest(requestCreateFoobar, write(foobarObject))
           responseCreateFoobar.code should equal(403)
+          // Regression: error responses on the native http4s dynamic-entity path (rendered by
+          // ErrorResponseConverter, which bypasses ResourceDocMiddleware) must also be labelled
+          // application/json, not http4s' default text/plain.
+          responseCreateFoobar.headers.map(_.get("Content-Type")).getOrElse("").toLowerCase should include("application/json")
           And("error should be " + UserHasMissingRoles)
           responseCreateFoobar.body.extract[ErrorMessage].message contains (UserHasMissingRoles) should be (true)
           responseCreateFoobar.body.extract[ErrorMessage].message contains ("CanCreateDynamicEntity_SystemFooBar") should be (true)

todo_shared_ensure_json_content_type.md

@@ -0,0 +1,60 @@
+# TODO: single shared `ensureJsonContentType` (DRY the JSON content-type guarantee)
+
+## Context
+
+OBP guarantees `Content-Type: application/json` on http4s responses in **three independent
+places**, each with its own private copy of the same `Content-Type(MediaType.application.json)`
+value and/or the same "if not JSON, force JSON" logic:
+
+1. `code/api/util/http4s/ResourceDocMiddleware.scala`
+   - `jsonContentType` (~line 52) + `ensureJsonContentType(response)` (~line 561), applied to
+     every response from the per-version services (v1.2.1 → v7, UK OB, Berlin Group).
+2. `code/api/util/http4s/ErrorResponseConverter.scala`
+   - `jsonContentType` (~line 40), set on every error response branch (~lines 122, 156, 171,
+     188, 204).
+3. `code/api/util/http4s/Http4sSupport.scala` (`EndpointHelpers`)
+   - `jsonContentType` (added when fixing the dynamic-entity `text/plain` bug), applied in
+     `toJsonOk` / `executeFutureCreated` / `executeFutureWithStatus`.
+
+This duplication is why the `text/plain` bug was possible: the per-version services were
+silently saved by `ResourceDocMiddleware.ensureJsonContentType`, while the services that
+**bypass** that middleware leaked `text/plain` until `EndpointHelpers` was fixed at the source.
+
+## Why it matters
+
+The services that intentionally bypass `ResourceDocMiddleware` (runtime-mutable / lighter
+gates) have **no runtime net** equivalent to the version services — they rely entirely on every
+response being built through `EndpointHelpers` or `ErrorResponseConverter`. Today that holds
+(verified: `Http4sDynamicEntity` builds no raw responses of its own), and it is now pinned by
+content-type assertions in `DynamicEntityTest` (success path + 403 error path). But a future
+contributor adding a new response path that skips both helpers would regress silently for:
+
+- `dynamicEntityRoutes`   (`Http4sApp.scala:148`)
+- `dynamicEndpointRoutes` (`Http4sApp.scala:149`)
+- `DirectLoginRoutes`     (`Http4sApp.scala:150`)
+- `AliveCheckRoutes`      (`Http4sApp.scala:151`)
+
+## Proposed work
+
+1. Extract one shared helper (e.g. `Http4sJson.ensureJsonContentType(resp): Response[IO]` and a
+   single shared `jsonContentType` val) into a small util, and have all three sites above call it
+   instead of their private copies. Pure refactor, no behaviour change.
+2. (Optional, decided separately) Give the middleware-bypassing services the same runtime net the
+   version services have: wrap the four routes at the `Http4sApp` wiring (lines 148–151) with the
+   shared `ensureJsonContentType`, rather than minting a per-service wrapper. This makes the
+   source-level fix in `EndpointHelpers`/`ErrorResponseConverter` belt-and-suspenders rather than
+   load-bearing for those routes.
+
+## Acceptance
+
+- Exactly one definition of `jsonContentType` and one `ensureJsonContentType` in the codebase.
+- `ResourceDocMiddleware`, `ErrorResponseConverter`, and `EndpointHelpers` all delegate to it.
+- Existing content-type assertions (`Http4sJsonContentTypeTest`, `DynamicEntityTest` success +
+  error path, `Http4sResponseConversionTest`, `AliveCheckRoutesTest`) stay green.
+
+## Origin
+
+Follow-up to the dynamic-entity `text/plain` content-type fix in `Http4sSupport.scala`
+(JSON responses were mislabelled because http4s' default `EntityEncoder[String]` sets
+`text/plain`; the dynamic-entity service bypasses `ResourceDocMiddleware` so nothing
+re-normalised it).