Skip to content

Commit

Permalink
Merge pull request Data-Science-Platform#20 from Data-Science-Platfor…
Browse files Browse the repository at this point in the history
…m/issue/19

Add API for refreshing templates
  • Loading branch information
aurelianrl authored Oct 7, 2019
2 parents 1a58010 + 41d6fe7 commit 9db642f
Show file tree
Hide file tree
Showing 15 changed files with 141 additions and 21 deletions.
1 change: 1 addition & 0 deletions server/src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ broccoli {
max-output-size = 0
nested-interpretation-enabled = true
}
reload-token = "CHANGETHISTOKEN"
}

# Configuration for Broccoli's Websocket connection with the webui
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,26 @@ package de.frosner.broccoli.controllers
import com.mohiva.play.silhouette.api.Silhouette
import de.frosner.broccoli.auth.{BroccoliSimpleAuthorization, DefaultEnv}
import javax.inject.Inject
import de.frosner.broccoli.models.Template
import de.frosner.broccoli.models.{RefreshRequest, Template}
import de.frosner.broccoli.models.Refresh.refreshRequestReads
import de.frosner.broccoli.models.Template.templateApiWrites
import de.frosner.broccoli.services.{SecurityService, TemplateService}
import de.frosner.broccoli.templates.TemplateConfiguration
import play.api.Environment
import play.api.cache.SyncCacheApi
import play.api.libs.json.Json
import play.api.mvc.{BaseController, ControllerComponents}

import scala.concurrent.ExecutionContext
import scala.concurrent.{ExecutionContext, Future}

case class TemplateController @Inject()(
templateService: TemplateService,
templateConfiguration: TemplateConfiguration,
override val securityService: SecurityService,
override val playEnv: Environment,
override val cacheApi: SyncCacheApi,
override val controllerComponents: ControllerComponents,
override val executionContext: ExecutionContext,
implicit val executionContext: ExecutionContext,
override val silhouette: Silhouette[DefaultEnv]
) extends BaseController
with BroccoliSimpleAuthorization {
Expand All @@ -36,6 +39,21 @@ case class TemplateController @Inject()(
}
}

def refresh = Action.async(parse.json[RefreshRequest]) { implicit request =>
Future {
if (request.body.token == templateConfiguration.reloadToken) {
val templates = templateService.getTemplates(true)
if (request.body.returnTemplates) {
Ok(Json.toJson(templates))
} else {
Ok
}
} else {
Unauthorized
}
}
}

}

object TemplateController {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package de.frosner.broccoli.models

import play.api.libs.json.Json

case class RefreshRequest(token: String, returnTemplates: Boolean)

object Refresh {
implicit val refreshRequestReads = Json.reads[RefreshRequest]
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ class ApiV1Router @Inject()(
) extends SimpleRouter {
override def routes: Routes = {
// Templates
case GET(p"/templates") => templates.get.list
case GET(p"/templates/$id") => templates.get.show(id)
case GET(p"/templates") => templates.get.list
case GET(p"/templates/$id") => templates.get.show(id)
case POST(p"templates/refresh") => templates.get.refresh
// Instances
case GET(p"/instances" ? q_o"templateId=$id") => instances.get.list(id)
case GET(p"/instances/$id") => instances.get.show(id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import de.frosner.broccoli.templates._

@Singleton
class TemplateService @Inject()(templateSource: TemplateSource) {
def getTemplates: Seq[Template] = templateSource.loadTemplates
def getTemplates: Seq[Template] = getTemplates(false)

def getTemplates(refreshed: Boolean): Seq[Template] = templateSource.loadTemplates(refreshed)

def template(id: String): Option[Template] = getTemplates.find(_.id == id)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ class CachedTemplateSource(source: TemplateSource) extends TemplateSource {

override val templateRenderer: TemplateRenderer = source.templateRenderer

override def loadTemplates: Seq[Template] =
override def loadTemplates(refreshed: Boolean): Seq[Template] = {
if (refreshed) {
templatesCache = None
}
templatesCache match {
case Some(templates) => templates
case None =>
refresh()
templatesCache.get
}
}

def refresh(): Unit = templatesCache = Some(source.loadTemplates)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package de.frosner.broccoli.templates

import java.nio.file.{FileSystems, Files}

import com.typesafe.config.{ConfigFactory}
import com.typesafe.config.ConfigFactory
import pureconfig._
import de.frosner.broccoli.models.Template

Expand All @@ -22,9 +22,12 @@ class DirectoryTemplateSource(directory: String, val templateRenderer: TemplateR
log.info(s"Starting $this")

/**
* Templates loaded from the directory are always up to date.
* We ignore the refresh capability here
*
* @return The sequence of templates found in the directory
*/
override def loadTemplates: Seq[Template] = {
override def loadTemplates(refreshed: Boolean): Seq[Template] = {
val rootTemplatesDirectory = FileSystems.getDefault.getPath(directory).toAbsolutePath

if (!Files.isDirectory(rootTemplatesDirectory)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ class SignalRefreshedTemplateSource(source: CachedTemplateSource, signalManager:
}
})

override def loadTemplates: Seq[Template] = source.loadTemplates
override def loadTemplates(refreshed: Boolean): Seq[Template] = source.loadTemplates(refreshed)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ import de.frosner.broccoli.templates.jinjava.JinjavaConfiguration
* @param path The filesystem path to read the templates from
* @param jinjava Configuration specific to jinjava template library
*/
final case class TemplateConfiguration(path: String, jinjava: JinjavaConfiguration)
final case class TemplateConfiguration(path: String, jinjava: JinjavaConfiguration, reloadToken: String)
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package de.frosner.broccoli.templates

import javax.inject.Singleton

import com.google.inject.{AbstractModule, Provides}
import de.frosner.broccoli.BroccoliConfiguration
import de.frosner.broccoli.signal.UnixSignalManager
Expand All @@ -28,4 +27,13 @@ class TemplateModule extends AbstractModule with ScalaModule {
new CachedTemplateSource(new DirectoryTemplateSource(config.templates.path, templateRenderer)),
signalManager
)

/**
* Provide Template configuration.
*
* @param config The whole broccoli configuration
* @return The templates part of that configuration
*/
@Provides
def provideTemplateConfiguration(config: BroccoliConfiguration): TemplateConfiguration = config.templates
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ trait TemplateSource {
*
* Does not guarantee to return the same templates on repeated invocations. Can potentially perform I/O
*/
def loadTemplates(): Seq[Template]
def loadTemplates(): Seq[Template] = loadTemplates(false)

def loadTemplates(refreshed: Boolean): Seq[Template]

def loadTemplate(templateId: String,
templateString: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package de.frosner.broccoli.controllers

import de.frosner.broccoli.auth.{Account, Role}
import de.frosner.broccoli.auth.Account
import de.frosner.broccoli.models._
import de.frosner.broccoli.services.TemplateService
import play.api.test.{PlaySpecification, WithApplication}
import de.frosner.broccoli.services.{SecurityService, TemplateService}
import de.frosner.broccoli.templates.TemplateConfiguration
import de.frosner.broccoli.templates.jinjava.JinjavaConfiguration
import play.api.test.{FakeRequest, PlaySpecification, WithApplication}
import org.mockito.Mockito._
import play.api.libs.json._
import play.api.test.Helpers.stubControllerComponents
Expand All @@ -14,6 +16,8 @@ class TemplateControllerSpec extends PlaySpecification with AuthUtils {

sequential // http://stackoverflow.com/questions/31041842/error-with-play-2-4-tests-the-cachemanager-has-been-shut-down-it-can-no-longe

val templateConfig = TemplateConfiguration("/dummy/path/to/templates", JinjavaConfiguration(), "TOKEN")

"list" should {

"list all available templates" in new WithApplication {
Expand All @@ -34,6 +38,7 @@ class TemplateControllerSpec extends PlaySpecification with AuthUtils {
testWithAllAuths { (securityService, account) =>
TemplateController(
withTemplates(mock[TemplateService], List(template)),
templateConfig,
securityService,
playEnv,
cacheApi,
Expand Down Expand Up @@ -88,6 +93,7 @@ class TemplateControllerSpec extends PlaySpecification with AuthUtils {
testWithAllAuths { (securityService, account) =>
TemplateController(
withTemplates(mock[TemplateService], List(template)),
templateConfig,
securityService,
playEnv,
cacheApi,
Expand Down Expand Up @@ -126,6 +132,7 @@ class TemplateControllerSpec extends PlaySpecification with AuthUtils {
testWithAllAuths { (securityService, account) =>
TemplateController(
templateService,
templateConfig,
securityService,
playEnv,
cacheApi,
Expand All @@ -142,4 +149,69 @@ class TemplateControllerSpec extends PlaySpecification with AuthUtils {

}

"refresh" should {
val template1 = Template(
id = "id1",
template = "template1 {{id}}",
description = "description",
parameterInfos = Map(
"id" -> ParameterInfo(id = "id",
name = Some("myname"),
default = Some(StringParameterValue("myid")),
secret = Some(false),
`type` = ParameterType.String,
orderIndex = None)
)
)

val template2 = Template(
id = "id2",
template = "template2 {{id}}",
description = "description",
parameterInfos = Map(
"id" -> ParameterInfo(id = "id",
name = Some("myname"),
default = Some(StringParameterValue("myid")),
secret = Some(false),
`type` = ParameterType.String,
orderIndex = None)
)
)
val templateService = mock[TemplateService]
when(templateService.getTemplates(false)).thenReturn(Seq(template1))
when(templateService.getTemplates(true)).thenReturn(Seq(template1, template2))

"reload the templates if auth token is correct" in new WithApplication {
val controller = TemplateController(
templateService,
templateConfig,
withAuthNone(mock[SecurityService]),
playEnv,
cacheApi,
stubControllerComponents(),
ExecutionContext.global,
withIdentities(Account.anonymous)
)
val result = controller.refresh(FakeRequest().withBody(RefreshRequest("TOKEN", true)))
status(result) must be equalTo 200 and {
contentAsJson(result).as[Seq[Template]].size must be equalTo 2
}
}

"401 if auth token is incorrect" in new WithApplication {
val controller = TemplateController(
templateService,
templateConfig,
withAuthNone(mock[SecurityService]),
playEnv,
cacheApi,
stubControllerComponents(),
ExecutionContext.global,
withIdentities(Account.anonymous)
)
val result = controller.refresh(FakeRequest().withBody(RefreshRequest("BADTOKEN", true)))
status(result) must be equalTo UNAUTHORIZED
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class DirectoryTemplateSourceSpec extends Specification with TemporaryTemplatesC
"fail if the passed directory is not directory" in {
val directory = FileSystems.getDefault.getPath("not-a-directory")
Files.exists(directory) must beFalse
new DirectoryTemplateSource(directory.toString, mock[TemplateRenderer]).loadTemplates must throwA(
new DirectoryTemplateSource(directory.toString, mock[TemplateRenderer]).loadTemplates() must throwA(
new IllegalStateException(s"Templates directory ${directory.toAbsolutePath} is not a directory"))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ class SignalRefreshedTemplateSourceSpec extends Specification with Mockito {
signalRefreshedTemplateSource.loadTemplates()

there was no(testTemplateSource).refresh()
there was one(testTemplateSource).loadTemplates()
verify(testTemplateSource, times(1)).loadTemplates()
there was one(testTemplateSource).loadTemplates(false)
verify(testTemplateSource, times(1)).loadTemplates(false)

handler.getValue.handle(new Signal("USR2"))
there was one(testTemplateSource).refresh()
there was one(testTemplateSource).loadTemplates()
there was one(testTemplateSource).loadTemplates(false)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class TemplateSourceSpec extends Specification with Mockito {

def templateSource(renderer: TemplateRenderer): TemplateSource = new TemplateSource {
val templateRenderer: TemplateRenderer = renderer
override def loadTemplates(): Seq[Template] = Seq.empty
override def loadTemplates(refreshTemplate: Boolean): Seq[Template] = Seq.empty
}
def defaultTemplateSource = templateSource(defaultTemplateRenderer)

Expand Down

0 comments on commit 9db642f

Please sign in to comment.