feat(advisor): add support for advisor rules configuration

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/AdvisorContext.kt

@@ -1,6 +1,7 @@
 package com.roche.ambassador.advisor
 
 import com.roche.ambassador.advisor.common.AdvisorException
+import com.roche.ambassador.advisor.configuration.RulesProperties
 import com.roche.ambassador.advisor.dsl.AdviceKey
 import com.roche.ambassador.advisor.messages.AdviceMessage
 import com.roche.ambassador.advisor.messages.AdviceMessageLookup
@@ -19,7 +20,8 @@ class AdvisorContext(
     val source: ProjectSource,
     private val givenAdvisoryMessages: Map<AdvisoryMessageEntity.Type, List<AdvisoryMessageEntity>>,
     private val adviceMessageLookup: AdviceMessageLookup,
-    private val templateEngine: TemplateEngine
+    private val templateEngine: TemplateEngine,
+    val rulesConfiguration: RulesProperties
 ) {
 
     companion object {

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/AdvisorManager.kt

@@ -1,5 +1,6 @@
 package com.roche.ambassador.advisor
 
+import com.roche.ambassador.advisor.configuration.AdvisorProperties
 import com.roche.ambassador.advisor.messages.AdviceMessage
 import com.roche.ambassador.advisor.messages.AdviceMessageLookup
 import com.roche.ambassador.advisor.templates.TemplateEngine
@@ -15,7 +16,8 @@ class AdvisorManager(
     private val advisoryMessageRepository: AdvisoryMessageRepository,
     private val sources: ProjectSources,
     private val lookup: AdviceMessageLookup,
-    private val templateEngine: TemplateEngine
+    private val templateEngine: TemplateEngine,
+    private val properties: AdvisorProperties
 ) {
 
     companion object {
@@ -47,6 +49,6 @@ class AdvisorManager(
         } else {
             mapOf()
         }
-        return AdvisorContext(project, source, advisoryMessages, lookup, templateEngine)
+        return AdvisorContext(project, source, advisoryMessages, lookup, templateEngine, properties.rules)
     }
 }

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/IssueAdvisor.kt

@@ -2,15 +2,11 @@ package com.roche.ambassador.advisor
 
 import com.roche.ambassador.advisor.common.AdvisorException
 import com.roche.ambassador.advisor.configuration.AdvisorProperties
-import com.roche.ambassador.advisor.dsl.Dsl
-import com.roche.ambassador.advisor.dsl.RulesBuilder
 import com.roche.ambassador.advisor.messages.AdviceMessage
 import com.roche.ambassador.advisor.model.Advice
 import com.roche.ambassador.advisor.model.IssueAdvice
 import com.roche.ambassador.exceptions.AmbassadorException
 import com.roche.ambassador.extensions.LoggerDelegate
-import com.roche.ambassador.model.Visibility
-import com.roche.ambassador.model.project.Permissions
 import com.roche.ambassador.model.source.Issue
 import com.roche.ambassador.storage.advisor.AdvisoryMessageEntity
 import org.springframework.stereotype.Component

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/IssueAdvisoryRules.kt

@@ -13,14 +13,20 @@ object IssueAdvisoryRules {
         // FIXME rules should be part of model, but temporarily for simplicity are kept here
         Dsl.advise(issueAdvice, context) {
             // @formatter:off
-            has { visibility == Visibility.PRIVATE } then "visibility.private"
-            matchFirst({ description }) {
-                that { isNullOrBlank() } then "description.missing"
-                that { this!!.length < 30 } then "description.short"
+            has { config.visibility.enabled } and { visibility == Visibility.PRIVATE } then "visibility.private"
+            whenEnabled(config.description) {
+                matchFirst({ description }) {
+                    that { isNullOrBlank() } then "description.missing"
+                    that { this!!.length < config.description.shortLength } then "description.short"
+                }
+            }
+            has { config.topics.enabled } and { topics.isEmpty() } then "topics.empty"
+            whenEnabled(config.forking) {
+                createPermissionRule("forking") { forks }
+            }
+            whenEnabled(config.pullRequest) {
+                createPermissionRule("pullrequest") { pullRequests }
             }
-            has { topics.isEmpty() } then "topics.empty"
-            createPermissionRule("forking") { forks }
-            createPermissionRule("pullrequest") { pullRequests }
             // @formatter:on
         }
         return issueAdvice

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/configuration/AdvisorProperties.kt

@@ -2,13 +2,19 @@ package com.roche.ambassador.advisor.configuration
 
 import org.springframework.boot.context.properties.ConfigurationProperties
 import org.springframework.boot.context.properties.ConstructorBinding
+import org.springframework.boot.context.properties.NestedConfigurationProperty
 import org.springframework.validation.annotation.Validated
+import javax.validation.Valid
 
 @ConfigurationProperties(prefix = "ambassador.advisor")
 @ConstructorBinding
 @Validated
 data class AdvisorProperties(
-    val mode: Mode = Mode.NORMAL
+    val mode: Mode = Mode.NORMAL,
+
+    @NestedConfigurationProperty
+    @Valid
+    val rules: RulesProperties = RulesProperties(),
 ) {
 
     fun isEnabled(): Boolean = mode != Mode.DISABLED

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/configuration/RulesProperties.kt

@@ -0,0 +1,17 @@
+package com.roche.ambassador.advisor.configuration
+
+data class RulesProperties(
+    val visibility: Rule = Rule(true),
+    val description: DescriptionRule = DescriptionRule(true, 30),
+    val topics: Rule = Rule(),
+    val forking: Rule = Rule(),
+    val pullRequest: Rule = Rule(),
+) {
+
+    open class Rule(
+        val enabled: Boolean = true
+    )
+
+    class DescriptionRule(enabled: Boolean, val shortLength: Int) : Rule(enabled)
+}
+

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/dsl/Dsl.kt

@@ -27,8 +27,9 @@ interface ThatSupport<A : BuildableAdvice, T> {
     infix fun thatNot(predicate: T.() -> Boolean): Has<A, T> = that(not(predicate))
 }
 
-interface ThenSupport<A : BuildableAdvice> {
-    infix fun then(adviceKey: String): Then<A>
+interface ThenSupport {
+    infix fun then(adviceKey: String): Then
+    fun thenDoNothing(): Then
 }
 
 interface HasSupport<A : BuildableAdvice> {

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/dsl/Has.kt

@@ -2,29 +2,53 @@ package com.roche.ambassador.advisor.dsl
 
 import com.roche.ambassador.advisor.model.BuildableAdvice
 
+typealias Predicate<T> = T.() -> Boolean
+
 class Has<A : BuildableAdvice, T> internal constructor(
-    private val predicate: T.() -> Boolean,
+    predicate: Predicate<T>,
     private val testValue: T?,
     private val rulesBuilder: RulesBuilder<A>
-) : Invokable, ThenSupport<A> {
+) : Invokable, ThenSupport {
 
+    private val chainedPredicates: PredicateChain<T> = PredicateChain(predicate, testValue)
     private var action: Invokable? = null
 
-    infix fun and(andPredicate: T.() -> Boolean): Has<A, T> = Has({ predicate(this) && andPredicate(this) }, testValue, rulesBuilder)
+    infix fun and(andPredicate: Predicate<T>): Has<A, T> {
+        chainedPredicates.and(andPredicate)
+        return this
+    }
 
-    infix fun or(orPredicate: T.() -> Boolean): Has<A, T> = Has({ predicate(this) || orPredicate(this) }, testValue, rulesBuilder)
+    override fun thenDoNothing(): Then {
+        val then = Then.nothing()
+        this.action = then
+        return then
+    }
 
-    override infix fun then(adviceKey: String): Then<A> {
-        val then = Then(adviceKey, rulesBuilder)
+    override infix fun then(adviceKey: String): Then {
+        val then = Then.adviceMessage(adviceKey, rulesBuilder)
         this.action = then
         return then
     }
 
     override operator fun invoke(): Boolean {
-        if (testValue != null && action != null && predicate(testValue)) {
+        if (testValue != null && action != null && chainedPredicates.invoke()) {
             action!!()
             return true
         }
         return false
     }
+
+    private class PredicateChain<T>(initial: Predicate<T>, val target: T?): Invokable {
+
+        private val and: MutableList<Predicate<T>> = mutableListOf(initial)
+
+        fun and(predicate: Predicate<T>) {
+            and.add(predicate)
+        }
+
+        override fun invoke(): Boolean {
+            return target != null && and.all { it(target) }
+        }
+
+    }
 }

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/dsl/RulesBuilder.kt

@@ -1,6 +1,7 @@
 package com.roche.ambassador.advisor.dsl
 
 import com.roche.ambassador.advisor.AdvisorContext
+import com.roche.ambassador.advisor.configuration.RulesProperties
 import com.roche.ambassador.advisor.model.BuildableAdvice
 import com.roche.ambassador.model.Feature
 import com.roche.ambassador.model.project.Project
@@ -15,12 +16,20 @@ open class RulesBuilder<A : BuildableAdvice> constructor(
 
     fun <T, F : Feature<T>> readFeature(featureType: KClass<F>): Optional<T> = context.project.features.findValue(featureType)
 
+    val config = context.rulesConfiguration
+
     fun <T> alwaysFalse() = apply(alwaysFalse<A, T>(this))
 
     fun anyAlwaysFalse() = this.alwaysFalse<Any>()
 
     fun anyAlwaysTrue() = apply(alwaysTrue(Any(), this))
 
+    fun whenEnabled(rule: RulesProperties.Rule, handler: RulesBuilder<A>.() -> Unit) {
+        if (rule.enabled) {
+            handler(this)
+        }
+    }
+
     override fun <T> with(valueExtractor: Project.() -> T, with: With<A, T>.() -> Unit) {
         val handler = With(valueExtractor, this)
         with(handler)

ambassador-advisor/src/main/kotlin/com/roche/ambassador/advisor/dsl/Then.kt

@@ -3,26 +3,60 @@ package com.roche.ambassador.advisor.dsl
 import com.roche.ambassador.advisor.model.BuildableAdvice
 import com.roche.ambassador.model.project.Project
 
-class Then<A : BuildableAdvice>(
+sealed interface Then : Invokable {
+
+    infix fun with(args: Iterable<Any>)
+
+    infix fun with(args: Array<Any>)
+
+    infix fun with(arg: Any)
+
+    infix fun with(argProvider: Project.() -> Any)
+
+    companion object {
+        fun nothing(): Then = ThenNothing
+        fun <A: BuildableAdvice> adviceMessage(adviceKey: String, rulesBuilder: RulesBuilder<A>): Then = ThenAdviceMessage(adviceKey, rulesBuilder)
+    }
+}
+
+internal object ThenNothing : Then {
+    override fun with(args: Iterable<Any>) {
+    }
+
+    override fun with(args: Array<Any>) {
+    }
+
+    override fun with(arg: Any) {
+    }
+
+    override fun with(argProvider: Project.() -> Any) {
+    }
+
+    override fun invoke(): Boolean {
+        return true
+    }
+}
+
+internal class ThenAdviceMessage<A : BuildableAdvice>(
     val adviceKey: String,
     val rulesBuilder: RulesBuilder<A>
-) : Invokable {
+) : Then {
 
     private val args: MutableList<Any> = mutableListOf()
 
-    infix fun with(args: Iterable<Any>) {
+    override infix fun with(args: Iterable<Any>) {
         this.args.addAll(args)
     }
 
-    infix fun with(args: Array<Any>) {
+    override infix fun with(args: Array<Any>) {
         this.args.addAll(args)
     }
 
-    infix fun with(arg: Any) {
+    override infix fun with(arg: Any) {
         this.args.add(arg)
     }
 
-    infix fun with(argProvider: Project.() -> Any) {
+    override infix fun with(argProvider: Project.() -> Any) {
         val arg = argProvider(rulesBuilder.context.project)
         with(arg)
     }

ambassador-advisor/src/test/kotlin/com/roche/ambassador/advisor/dsl/DslTest.kt

@@ -1,5 +1,6 @@
 package com.roche.ambassador.advisor.dsl
 
+import com.roche.ambassador.advisor.configuration.RulesProperties
 import com.roche.ambassador.model.feature.TopicsFeature
 import org.junit.jupiter.api.Test
 
@@ -25,6 +26,19 @@ class DslTest {
         assertThat(advice).hasProblemsSize(0)
     }
 
+    @Test
+    fun shouldVerifyMultipleRules() {
+        // when
+        val advice = testAdvise {
+            anyAlwaysTrue() and { false } then "test1"
+            anyAlwaysTrue() and { true } then "test2"
+            anyAlwaysTrue() and { true } and { false } then "test3"
+            anyAlwaysTrue() and { true } and { true } then "test4"
+        }
+        // then
+        assertThat(advice).problems().hasNames("test2", "test4")
+    }
+
     @Test
     fun shouldAddPlainArgumentToProblem() {
         // when
@@ -69,6 +83,67 @@ class DslTest {
         assertThat(advice).problems().has("test2")
     }
 
+    @Test
+    fun shouldNotAddProblemWhenActionIsDoNothing() {
+        // when
+        val advice = testAdvise {
+            matchFirst {
+                anyAlwaysTrue().thenDoNothing()
+                anyAlwaysTrue() then "test2"
+            }
+        }
+        // then
+        assertThat(advice).hasProblemsSize(0)
+    }
+
+    @Test
+    fun shouldNotExecuteRulesWhenRuleIsDisabled() {
+        // when
+        val advice = testAdvise {
+            whenEnabled(RulesProperties.Rule(false)) {
+                anyAlwaysTrue() then "test1"
+                matchFirst {
+                    anyAlwaysTrue() then "test2"
+                }
+            }
+        }
+        // then
+        assertThat(advice).hasNoProblems()
+    }
+
+    @Test
+    fun shouldExecuteRuleWhenRuleIsEnabled() {
+        // when
+        val advice = testAdvise {
+            whenEnabled(RulesProperties.Rule(true)) {
+                anyAlwaysTrue() then "test1"
+                matchFirst {
+                    anyAlwaysTrue() then "test2"
+                }
+            }
+        }
+        // then
+        assertThat(advice).problems().hasNames("test1", "test2")
+    }
+
+    @Test
+    fun shouldCheckIfNestedRuleIsEnabled() {
+        // when
+        val advice = testAdvise {
+            whenEnabled(RulesProperties.Rule(true)) {
+                anyAlwaysTrue() then "test1"
+                whenEnabled(RulesProperties.Rule(false)) {
+                    anyAlwaysTrue() then "test2"
+                    whenEnabled(RulesProperties.Rule(true)) {
+                        anyAlwaysTrue() then "test3" // disabled cause nested
+                    }
+                }
+            }
+        }
+        // then
+        assertThat(advice).problems().hasNames("test1")
+    }
+
     @Test
     fun shouldMatchFirstRuleOutsideNestedMatchFirstClause() {
         // when