bripkens / health-check-adapter

Connect health check endpoints to Slack
Apache License 2.0
3 stars 3 forks source link

Configuration parsing test #4

Closed britter closed 8 years ago

britter commented 8 years ago

While working on #3 I realized that I will have to change the way ReporterConfigurations are implemented at the moment. For this reason I need a test suit that makes sure configuration parsing still works as expected. This needs to be merged before I can continue working on #3.

bripkens commented 8 years ago

I'd be okay with merging this - even with the null check as this the current behavior of the application. If you want to, feel free to inspect and change the property name to type. Would be helpful to understand the reason for this behavior :).

Anyhow, thank you very much for your help! :)

britter commented 8 years ago

feel free to inspect and change the property name to type

Changing the implementation to:

@JsonCreator
@JsonTypeInfo(
  use = JsonTypeInfo.Id.NAME,
  include = JsonTypeInfo.As.PROPERTY,
  property = "type"
)
@JsonSubTypes(value = Array(
  new JsonSubTypes.Type(value = classOf[SlackReporterConfig], name = "slack"),
  new JsonSubTypes.Type(value = classOf[ConsoleReporterConfig], name = "console")
))
abstract class AbstractReporterConfig(@JsonProperty("type") `type`: String) {
  val implementation: Class[_]
}

case class SlackReporterConfig(@JsonProperty("type") `type`: String,
                               @JsonProperty("channel") channel: String,
                               @JsonProperty("webhookUrl") webhookUrl: String,
                               @JsonProperty("botName") botName: String,
                               @JsonProperty("botImage") botImage: String)
  extends AbstractReporterConfig(`type`) {
  override val implementation = classOf[SlackReporter]
}

case class ConsoleReporterConfig(@JsonProperty("type") `type`: String)
  extends AbstractReporterConfig(`type`) {
  override val implementation = classOf[ConsoleReporter]
}

yields the same result: type ist still null. I think I'm okay with this for now, since I'd like to focus on #3 instead to figuring out what is going on with Jackson :-)

bripkens commented 8 years ago

Thanks :)