Skip to content

Commit

Permalink
API-7065 (hmrc#456)
Browse files Browse the repository at this point in the history
* API-7064 - Changing how Redoc download button works

* API-7065 - Adding tests for open api controller

* API-7065 - PR Comments
  • Loading branch information
peteslater-ee authored May 26, 2023
1 parent 4b6eec7 commit 43f8745
Show file tree
Hide file tree
Showing 11 changed files with 246 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ trait ApplicationConfig {
def feedbackSurveyUrl: String

def cookieSettingsUrl: String

def oasFetchResolvedMaxDuration: Long
}

@Singleton
Expand Down Expand Up @@ -103,6 +105,8 @@ class ApplicationConfigImpl @Inject() (config: Configuration)

val cookieSettingsUrl: String = s"/${getString("tracking-consent-frontend.cookie-settings-path")}"

val oasFetchResolvedMaxDuration: Long = config.getMillis("oasFetchResolvedMaxDurationMilliseconds")

private def platformBaseUrl(key: String) = {
(getConfigDefaulted(s"$key.protocol", ""), getConfigDefaulted(s"$key.host", "")) match {
case (p, h) if !p.isEmpty && !h.isEmpty => s"$p://$h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@

package uk.gov.hmrc.apidocumentation.config

import play.api.inject.Module
import play.api.{Configuration, Environment}
import com.google.inject.AbstractModule
import io.swagger.v3.parser.OpenAPIV3Parser
import io.swagger.v3.parser.core.extensions.SwaggerParserExtension

import uk.gov.hmrc.apidocumentation.connectors.XmlServicesConnector

class ConfigurationModule extends Module {
class ConfigurationModule extends AbstractModule {

override def bindings(environment: Environment, configuration: Configuration) = {

Seq(
bind[XmlServicesConnector.Config].toProvider[XmlServicesConnectorConfigProvider]
)
override def configure(): Unit = {
bind(classOf[XmlServicesConnector.Config]).toProvider(classOf[XmlServicesConnectorConfigProvider])
bind(classOf[SwaggerParserExtension]).toInstance(new OpenAPIV3Parser)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,19 @@

package uk.gov.hmrc.apidocumentation.controllers

import java.io.FileNotFoundException
import java.util.concurrent.TimeUnit
import javax.inject.{Inject, Singleton}
import scala.concurrent.Future.successful
import scala.concurrent.{ExecutionContext, Future}
import scala.concurrent.duration.FiniteDuration
import scala.concurrent.{ExecutionContext, Future, blocking}

import akka.actor.ActorSystem
import io.swagger.v3.core.util.Yaml
import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.parser.core.extensions.SwaggerParserExtension
import io.swagger.v3.parser.core.models.ParseOptions
import io.swagger.v3.parser.exception.ReadContentException

import play.api.mvc._
import play.mvc.Http.HeaderNames
Expand All @@ -44,9 +54,11 @@ class OpenApiDocumentationController @Inject() (
apiDefinitionService: ApiDefinitionService,
loggedInUserService: LoggedInUserService,
errorHandler: ErrorHandler,
val navigationService: NavigationService
val navigationService: NavigationService,
openAPIV3Parser: SwaggerParserExtension
)(implicit val ec: ExecutionContext,
appConfig: ApplicationConfig
appConfig: ApplicationConfig,
system: ActorSystem
) extends FrontendController(mcc) with HeaderNavigation with HomeCrumb with ApplicationLogger {

private val buildPageAttributes = (navLinks: Seq[NavLink]) =>
Expand Down Expand Up @@ -115,6 +127,57 @@ class OpenApiDocumentationController @Inject() (
}
}

def fetchOasResolved(service: String, version: String) = Action.async { implicit request =>
def handleSuccess(openApi: OpenAPI): Result =
Ok(Yaml.pretty.writeValueAsString(openApi)).withHeaders(HeaderNames.CONTENT_DISPOSITION -> "attachment; filename=\"application.yaml\"")
val handleFailure: Result = NotFound

val parseOptions = new ParseOptions()
parseOptions.setResolve(true)
parseOptions.setResolveFully(true)

val emptyAuthList = java.util.Collections.emptyList[io.swagger.v3.parser.core.models.AuthorizationValue]()

val oasFileLocation = routes.OpenApiDocumentationController.fetchOas(service, version).absoluteURL()

val futureParsing = Future {
blocking {
try {
val parserResult = openAPIV3Parser.readLocation(oasFileLocation, emptyAuthList, parseOptions)

Option(parserResult.getOpenAPI()) match {
// The OAS specification has been found and parsed by Swagger - return the fully resolved specification to the caller.
case Some(openApi) => {
logger.info("Successfully parsed the OAS specification.")
handleSuccess(openApi)
}
// The OAS specification has been found but there was a parsing problem - return an empty specification to the caller.
case None => {
logger.info(s"There was a problem parsing the OAS specification.")
handleFailure
}
}
} catch {
// The OAS specification has not been found.
case e: FileNotFoundException => {
logger.info("The OAS specification could not be found.")
handleFailure
}
case e: ReadContentException => {
logger.info("The OAS specification could not be found.")
handleFailure
}
}
}
}

val futureTimer: Future[Result] = akka.pattern.after(FiniteDuration(appConfig.oasFetchResolvedMaxDuration, TimeUnit.MILLISECONDS), using = system.scheduler)(
Future.failed(new IllegalStateException("Exceeded OAS parse time"))
)

Future.firstCompletedOf(List(futureParsing, futureTimer))
}

def previewApiDocumentationPage(): Action[AnyContent] = headerNavigation { implicit request => navLinks =>
if (appConfig.openApiPreviewEnabled) {
val pageAttributes = buildPageAttributes(navLinks)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import play.api.mvc._
import uk.gov.hmrc.play.bootstrap.frontend.controller.FrontendController

import uk.gov.hmrc.apidocumentation.config.ApplicationConfig
import uk.gov.hmrc.apidocumentation.models._
import uk.gov.hmrc.apidocumentation.services.NavigationService
import uk.gov.hmrc.apidocumentation.util.ApplicationLogger
import uk.gov.hmrc.apidocumentation.views.html._
import uk.gov.hmrc.apidocumentation.models._

@Singleton
class TestingPagesController @Inject() (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@
https://github.com/Redocly/redoc/issues/1108#issuecomment-585990742
*@
<script @CSPNonce.attr>
var oasUrl = "@uk.gov.hmrc.apidocumentation.controllers.routes.OpenApiDocumentationController.fetchOas(serviceName, version)"
var oasArchiveUrl = "@uk.gov.hmrc.apidocumentation.controllers.routes.OpenApiDocumentationController.fetchOasResolved(serviceName, version)"

var options = {
hideDownloadButton: true,
hideDownloadButton: false,
downloadDefinitionUrl: oasArchiveUrl,
theme: {
spacing: {
sectionVertical: '35px',
Expand All @@ -49,8 +53,6 @@
}
};

var oasUrl = "@uk.gov.hmrc.apidocumentation.controllers.routes.OpenApiDocumentationController.fetchOas(serviceName, version)"

Redoc.init(oasUrl, options, document.getElementById("redoc"), applyRedocFixes);

</script>
1 change: 1 addition & 0 deletions conf/app.routes
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ GET /docs/api/service/:service/:version/oas/page uk.gov.h
GET /docs/openapi/preview uk.gov.hmrc.apidocumentation.controllers.OpenApiDocumentationController.previewApiDocumentationPage()
GET /docs/openapi/preview/action uk.gov.hmrc.apidocumentation.controllers.OpenApiDocumentationController.previewApiDocumentationAction(url: Option[String])
GET /docs/api/service/:service/:version/oas/file uk.gov.hmrc.apidocumentation.controllers.OpenApiDocumentationController.fetchOas(service: String, version: String)
GET /docs/api/service/:service/:version/oas/resolved uk.gov.hmrc.apidocumentation.controllers.OpenApiDocumentationController.fetchOasResolved(service: String, version: String)
GET /docs/api/service/:service/:version/oas/*resource uk.gov.hmrc.apidocumentation.controllers.DownloadController.downloadResource(service: String, version: String, resource: String)

GET /docs/preview uk.gov.hmrc.apidocumentation.controllers.ApiDocumentationController.previewApiDocumentation(url: Option[String])
Expand Down
1 change: 1 addition & 0 deletions conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ play.filters.csp.directives.script-src = ${play.filters.csp.nonce.pattern} "'str

retryCount = 3
retryDelayMilliseconds = 500
oasFetchResolvedMaxDurationMilliseconds = 20000

apidocumentation.base.url = "http://localhost:9680"

Expand Down
17 changes: 16 additions & 1 deletion project/AppDependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ object AppDependencies {
lazy val playJsonVersion = "2.9.2"
lazy val bootstrapVersion = "7.14.0"
lazy val seleniumVersion = "4.2.0"
lazy val jacksonDatabindVersion = "2.10.5.1"
lazy val jacksonVersion = "2.10.5"

lazy val compile = Seq(
ws,
Expand All @@ -19,7 +21,20 @@ object AppDependencies {
"org.typelevel" %% "cats-core" % "2.6.1",
"org.commonjava.googlecode.markdown4j" % "markdown4j" % "2.2-cj-1.1",
"com.typesafe.play" %% "play-json" % playJsonVersion,
"com.typesafe.play" %% "play-json-joda" % playJsonVersion
"com.typesafe.play" %% "play-json-joda" % playJsonVersion,
"io.swagger.parser.v3" % "swagger-parser" % "2.1.9"
excludeAll(
ExclusionRule("com.fasterxml.jackson.core", "jackson-databind"),
ExclusionRule("com.fasterxml.jackson.core", "jackson-core"),
ExclusionRule("com.fasterxml.jackson.core", "jackson-annotations"),
ExclusionRule("com.fasterxml.jackson.dataformat", "jackson-dataformat-yaml"),
ExclusionRule("com.fasterxml.jackson.datatype", "jackson-datatype-jsr310")
),
"com.fasterxml.jackson.core" % "jackson-core" % jacksonVersion,
"com.fasterxml.jackson.core" % "jackson-databind" % jacksonDatabindVersion,
"com.fasterxml.jackson.core" % "jackson-annotations" % jacksonVersion,
"com.fasterxml.jackson.dataformat" % "jackson-dataformat-yaml" % jacksonVersion,
"com.fasterxml.jackson.datatype" % "jackson-datatype-jsr310" % jacksonVersion
)

lazy val test = Seq(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.Future
import scala.concurrent.duration._

import play.api.http.Status.MOVED_PERMANENTLY
import play.api.http.HeaderNames.LOCATION
import play.api.http.Status.MOVED_PERMANENTLY
import play.api.mvc._
import play.api.test.Helpers._
import play.twirl.api.Html
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright 2023 HM Revenue & Customs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package uk.gov.hmrc.apidocumentation.controllers

import java.io.FileNotFoundException
import scala.concurrent.ExecutionContext.Implicits.global

import akka.actor.ActorSystem
import io.swagger.v3.oas.models.OpenAPI
import io.swagger.v3.parser.core.extensions.SwaggerParserExtension
import io.swagger.v3.parser.core.models.SwaggerParseResult

import play.api.http.Status.{NOT_FOUND, OK}
import play.api.mvc.MessagesControllerComponents
import play.api.test.Helpers._

import uk.gov.hmrc.apidocumentation.ErrorHandler
import uk.gov.hmrc.apidocumentation.mocks.config._
import uk.gov.hmrc.apidocumentation.mocks.connectors.DownloadConnectorMockModule
import uk.gov.hmrc.apidocumentation.mocks.services._
import uk.gov.hmrc.apidocumentation.views.html._

class OpenApiDocumentationControllerSpec extends CommonControllerBaseSpec {

trait Setup
extends DownloadConnectorMockModule
with ApiDefinitionServiceMock
with LoggedInUserServiceMock
with NavigationServiceMock
with AppConfigMock {

val emptySwaggerParseResult = new SwaggerParseResult()
val openApiSwaggerParseResult = new SwaggerParseResult()
openApiSwaggerParseResult.setOpenAPI(new OpenAPI())

lazy val openApiViewRedoc = app.injector.instanceOf[OpenApiViewRedoc]
lazy val openApiPreviewRedoc = app.injector.instanceOf[OpenApiPreviewRedoc]
lazy val openApiPreviewView = app.injector.instanceOf[OpenApiPreviewView]
lazy val retiredVersionJumpView = app.injector.instanceOf[RetiredVersionJumpView]
lazy val mcc = app.injector.instanceOf[MessagesControllerComponents]
lazy val errorHandler = app.injector.instanceOf[ErrorHandler]
lazy val openAPIV3ParserMock = mock[SwaggerParserExtension]

implicit lazy val system = app.injector.instanceOf[ActorSystem]

val underTest = new OpenApiDocumentationController(
openApiViewRedoc,
openApiPreviewRedoc,
openApiPreviewView,
retiredVersionJumpView,
DownloadConnectorMock.aMock,
mcc,
apiDefinitionService,
loggedInUserService,
errorHandler,
navigationService,
openAPIV3ParserMock
)
}

"OpenApiDocumentationController" should {
"successfully fetch resolved OAS specification" in new Setup {
when(appConfig.oasFetchResolvedMaxDuration).thenReturn(1000)
when(openAPIV3ParserMock.readLocation(*, *, *)).thenReturn(openApiSwaggerParseResult)

val result = underTest.fetchOasResolved("Test-Service", "Test-Version")(request)

status(result) shouldBe OK
}

"successfully handle and error when fetching resolved OAS specification" in new Setup {
when(appConfig.oasFetchResolvedMaxDuration).thenReturn(1000)
when(openAPIV3ParserMock.readLocation(*, *, *)).thenReturn(emptySwaggerParseResult)

val result = underTest.fetchOasResolved("Test-Service", "Test-Version")(request)

status(result) shouldBe NOT_FOUND
}

"successfully handle FileNotFoundException when fetching resolved OAS specification" in new Setup {
when(appConfig.oasFetchResolvedMaxDuration).thenReturn(1000)
when(openAPIV3ParserMock.readLocation(*, *, *)).thenThrow(new FileNotFoundException())

val result = underTest.fetchOasResolved("Test-Service", "Test-Version")(request)

status(result) shouldBe NOT_FOUND
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2023 HM Revenue & Customs
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package uk.gov.hmrc.apidocumentation.mocks.connectors

import scala.concurrent.Future.successful

import org.mockito.{ArgumentMatchersSugar, MockitoSugar}

import uk.gov.hmrc.apidocumentation.connectors.DownloadConnector

trait DownloadConnectorMockModule extends MockitoSugar with ArgumentMatchersSugar {

trait AbstractDownloadConnectorMock {
def aMock: DownloadConnector

object Fetch {

def returnsNoneIfNotFound() = {
when(aMock.fetch(*, *, *)).thenReturn(successful(None))
}
}
}

object DownloadConnectorMock extends AbstractDownloadConnectorMock {
val aMock = mock[DownloadConnector]
}
}

0 comments on commit 43f8745

Please sign in to comment.