Skip to content

Commit

Permalink
Reject version for FixedCommand if same or older than any affected ve…
Browse files Browse the repository at this point in the history
…rsion (#728)

Check can be skipped by adding ` force` suffix to the command.
  • Loading branch information
Marcono1234 authored Jul 19, 2021
1 parent 7df93d8 commit e6c0902
Show file tree
Hide file tree
Showing 11 changed files with 220 additions and 38 deletions.
14 changes: 10 additions & 4 deletions docs/Commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,16 @@ This command currently only exists because of a technical limitation. It will be
Lowercases sentences in which the first letter of each word is capitalized.

## $ARISA_FIXED
| Entry | Value |
| ----------- | ------------------------ |
| Syntax | `$ARISA_FIXED <version>` |
| Permissions | Mod+ |
| Entry | Value |
| ----------- | -------------------------------- |
| Syntax | `$ARISA_FIXED <version> [force]` |
| Permissions | Mod+ |

Resolves an issue as Fixed in the specified version. This command is useful when the version has already been
archived, and the web interface does not allow choosing that version.

When one of the affected versions of the issue was erroneously added and the fixed version is the same or an earlier
version, the command fails. To skip this check, run with a trailing `force` literal, e.g. `$ARISA_FIXED 12w34a force`.

## $ARISA_LIST_USER_ACTIVITY
| Entry | Value |
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/io/github/mojira/arisa/domain/Issue.kt
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ data class Issue(
val addRestrictedComment: (options: CommentOptions) -> Unit,
val addNotEnglishComment: (language: String) -> Unit,
val addRawRestrictedComment: (body: String, restriction: String) -> Unit,
val markAsFixedWithSpecificVersion: (fixVersion: String) -> Unit,
val markAsFixedWithSpecificVersion: (fixVersionName: String) -> Unit,
val changeReporter: (reporter: String) -> Unit,
val addAttachment: (file: File, cleanupCallback: () -> Unit) -> Unit
)
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,8 @@ fun getGroups(jiraClient: JiraClient, username: String) = runBlocking {
}
}

fun markAsFixedWithSpecificVersion(context: Lazy<IssueUpdateContext>, fixVersion: String) {
context.value.resolve.field(Field.FIX_VERSIONS, listOf(mapOf("name" to fixVersion)))
fun markAsFixedWithSpecificVersion(context: Lazy<IssueUpdateContext>, fixVersionName: String) {
context.value.resolve.field(Field.FIX_VERSIONS, listOf(mapOf("name" to fixVersionName)))
context.value.transitionName = "Resolve Issue"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import io.github.mojira.arisa.infrastructure.jira.getIssuesFromJql
import io.github.mojira.arisa.jiraClient
import io.github.mojira.arisa.modules.commands.arguments.LinkList
import io.github.mojira.arisa.modules.commands.arguments.LinkListArgumentType
import io.github.mojira.arisa.modules.commands.arguments.StringWithFlag
import io.github.mojira.arisa.modules.commands.arguments.greedyStringWithFlag

@Suppress("LongMethod")
fun getCommandDispatcher(
Expand Down Expand Up @@ -121,11 +123,13 @@ fun getCommandDispatcher(
literal<CommandSource>("${prefix}_FIXED")
.requires(::sentByModerator)
.then(
argument<CommandSource, String>("version", greedyString())
argument<CommandSource, StringWithFlag>("version", greedyStringWithFlag("force"))
.executes {
val (version, force) = it.getStringWithFlag("version")
fixedCommand(
it.source.issue,
it.getString("version")
version,
force
)
}
)
Expand Down Expand Up @@ -232,3 +236,4 @@ private fun sentByModerator(source: CommandSource) =
private fun CommandContext<*>.getInt(name: String) = getArgument(name, Int::class.java)
private fun CommandContext<*>.getLinkList(name: String) = getArgument(name, LinkList::class.java)
private fun CommandContext<*>.getString(name: String) = getArgument(name, String::class.java)
private fun CommandContext<*>.getStringWithFlag(name: String) = getArgument(name, StringWithFlag::class.java)
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.github.mojira.arisa.modules.commands

import com.mojang.brigadier.LiteralMessage
import com.mojang.brigadier.exceptions.Dynamic2CommandExceptionType
import com.mojang.brigadier.exceptions.DynamicCommandExceptionType
import com.mojang.brigadier.exceptions.SimpleCommandExceptionType

Expand All @@ -21,9 +22,10 @@ object CommandExceptions {
LiteralMessage("Could not query activity of user \"$it\"")
}

val FIX_VERSION_BEFORE_FIRST_AFFECTED_VERSION = DynamicCommandExceptionType {
LiteralMessage("Cannot add fix version $it because the first affected " +
"version of the issue was released after it")
val FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION = Dynamic2CommandExceptionType {
fixVersionName, affectedVersionName -> LiteralMessage("Cannot add fix version $fixVersionName " +
"because the affected version $affectedVersionName of the issue is the same or was released after " +
"it; run with `<version> force` to add the fix version anyways")
}

val INVALID_LINK_TYPE = SimpleCommandExceptionType(
Expand All @@ -34,6 +36,10 @@ object CommandExceptions {
LiteralMessage("Found invalid ticket key")
)

val GREEDY_STRING_ONLY_FLAG = DynamicCommandExceptionType {
LiteralMessage("Argument consists only of flag '$it' but does not contain a string")
}

val LEFT_EITHER = DynamicCommandExceptionType {
LiteralMessage("Something went wrong, but I'm too lazy to interpret the details for you (>ω<): $it")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,34 @@
package io.github.mojira.arisa.modules.commands

import io.github.mojira.arisa.domain.Issue
import java.time.Instant

class FixedCommand {
@Suppress("ThrowsCount")
operator fun invoke(issue: Issue, version: String): Int {
if (issue.fixVersions.any { it.name == version }) {
throw CommandExceptions.ALREADY_FIXED_IN.create(version)
}
if (issue.project.versions.none { it.name == version }) {
throw CommandExceptions.NO_SUCH_VERSION.create(version)
operator fun invoke(issue: Issue, fixVersionName: String, force: Boolean): Int {
if (issue.fixVersions.any { it.name == fixVersionName }) {
throw CommandExceptions.ALREADY_FIXED_IN.create(fixVersionName)
}

val fixVersion = issue.project.versions.firstOrNull { it.name == fixVersionName }
?: throw CommandExceptions.NO_SUCH_VERSION.create(fixVersionName)

if (issue.resolution !in listOf(null, "", "Unresolved")) {
throw CommandExceptions.ALREADY_RESOLVED.create(issue.resolution)
}
if (issue.affectedVersions.first().releaseDate!!.isAfter(
issue.project.versions.first { it.name == version }.releaseDate
)) {
throw CommandExceptions.FIX_VERSION_BEFORE_FIRST_AFFECTED_VERSION.create(version)

if (!force) {
// Fail if any affected version is same or newer than fix version
// Since archived fix versions cannot be removed again this prevents accidentally adding an incorrect
// fix version
issue.affectedVersions.firstOrNull { it.releaseDate!!.isSameOrAfter(fixVersion.releaseDate!!) }?.let {
throw CommandExceptions.FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION.create(fixVersionName, it.name)
}
}

issue.markAsFixedWithSpecificVersion(version)
issue.markAsFixedWithSpecificVersion(fixVersionName)
return 1
}
}

private fun Instant.isSameOrAfter(other: Instant) = !this.isBefore(other)
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package io.github.mojira.arisa.modules.commands.arguments

import com.mojang.brigadier.StringReader
import com.mojang.brigadier.arguments.ArgumentType
import io.github.mojira.arisa.modules.commands.CommandExceptions

/**
* Similar to [com.mojang.brigadier.arguments.StringArgumentType.greedyString], except that the string
* may end with an optional flag separated by a space.
*/
fun greedyStringWithFlag(flag: String): ArgumentType<StringWithFlag> = GreedyStringWithTrailingFlagArgumentType(flag)

private class GreedyStringWithTrailingFlagArgumentType(private val flag: String) : ArgumentType<StringWithFlag> {
private val flagSuffix = " $flag"

override fun parse(reader: StringReader): StringWithFlag {
var text = reader.remaining

// Throw exception when input consists only of flag, to prevent accidental incorrect usage
if (text == flag) {
throw CommandExceptions.GREEDY_STRING_ONLY_FLAG.createWithContext(reader, flag)
}

// Consume complete input
reader.cursor = reader.totalLength

var isFlagSet = false
if (text.endsWith(flagSuffix)) {
text = text.removeSuffix(flagSuffix)
isFlagSet = true
}

return StringWithFlag(text, isFlagSet)
}
}

data class StringWithFlag(
val string: String,
/** `true` if the flag was explicitly provided in the command */
val isFlagSet: Boolean
)
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,24 @@ private val TWO_YEARS_AGO = RIGHT_NOW.minus(730, ChronoUnit.DAYS)
class FixedCommandTest : StringSpec({
"should add version" {
val command = FixedCommand()
var fixVersion: String? = null

val affectedVersion1 = getVersion(released = true, archived = false, "12w34a", releaseDate = TWO_YEARS_AGO)
val affectedVersion2 = getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO)
val issue = mockIssue(
project = mockProject(
versions = listOf(
getVersion(released = true, archived = false),
getVersion(released = true, archived = false, "12w34b")
affectedVersion1,
affectedVersion2
)
),
affectedVersions = listOf(getVersion(released = true, archived = false))
affectedVersions = listOf(affectedVersion1),
markAsFixedInASpecificVersion = { fixVersion = it }
)

val result = command(issue, "12w34b")

val result = command(issue, affectedVersion2.name, false)
result shouldBe 1
fixVersion shouldBe affectedVersion2.name
}

"should throw ALREADY_FIXED_IN when ticket is already fixed in such version" {
Expand All @@ -48,7 +52,7 @@ class FixedCommandTest : StringSpec({
)

val exception = shouldThrow<CommandSyntaxException> {
command(issue, "12w34b")
command(issue, "12w34b", false)
}
exception.message shouldBe "The ticket was already marked as fixed in 12w34b"
}
Expand All @@ -66,7 +70,7 @@ class FixedCommandTest : StringSpec({
)

val exception = shouldThrow<CommandSyntaxException> {
command(issue, "12w34b")
command(issue, "12w34b", false)
}
exception.message shouldBe "The version 12w34b doesn't exist in this project"
}
Expand All @@ -86,28 +90,71 @@ class FixedCommandTest : StringSpec({
)

val exception = shouldThrow<CommandSyntaxException> {
command(issue, "12w34b")
command(issue, "12w34b", false)
}
exception.message shouldBe "The ticket was already resolved as Cannot Reproduce"
}

"should throw FIX_VERSION_BEFORE_FIRST_AFFECTED_VERSION when the fix version was released before the first affected version" {
"should throw FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION when the fix version is same as an affected version" {
val command = FixedCommand()

val affectedVersion = getVersion(released = true, archived = false, "12w34a")
val issue = mockIssue(
project = mockProject(
versions = listOf(
getVersion(released = true, archived = false, "12w34a", releaseDate = TWO_YEARS_AGO),
getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO)
affectedVersion
)
),
affectedVersions = listOf(getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO))
affectedVersions = listOf(affectedVersion)
)

val exception = shouldThrow<CommandSyntaxException> {
command(issue, "12w34a")
command(issue, affectedVersion.name, false)
}
exception.message shouldBe "Cannot add fix version 12w34a because the first affected version of the issue was released after it"
exception.message shouldBe "Cannot add fix version 12w34a because the affected version 12w34a of the issue " +
"is the same or was released after it; run with `<version> force` to add the fix version anyways"
}

"should throw FIX_VERSION_SAME_OR_BEFORE_AFFECTED_VERSION when the fix version is before an affected version" {
val command = FixedCommand()

val affectedVersion1 = getVersion(released = true, archived = false, "12w34a", releaseDate = TWO_YEARS_AGO)
val affectedVersion2 = getVersion(released = true, archived = false, "12w34b", releaseDate = TEN_SECONDS_AGO)
val issue = mockIssue(
project = mockProject(
versions = listOf(
affectedVersion1,
affectedVersion2
)
),
affectedVersions = listOf(affectedVersion2)
)

val exception = shouldThrow<CommandSyntaxException> {
command(issue, affectedVersion1.name, false)
}
exception.message shouldBe "Cannot add fix version 12w34a because the affected version 12w34b of the issue " +
"is the same or was released after it; run with `<version> force` to add the fix version anyways"
}

"should not throw when the fix version is same as an affected version, but 'force' is used" {
val command = FixedCommand()
var fixVersion: String? = null

val affectedVersion = getVersion(released = true, archived = false, "12w34a")
val issue = mockIssue(
project = mockProject(
versions = listOf(
affectedVersion
)
),
affectedVersions = listOf(affectedVersion),
markAsFixedInASpecificVersion = { fixVersion = it }
)

val result = command(issue, affectedVersion.name, true)
result shouldBe 1
fixVersion shouldBe affectedVersion.name
}
})

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package io.github.mojira.arisa.modules.commands.arguments

import com.mojang.brigadier.StringReader
import com.mojang.brigadier.exceptions.CommandSyntaxException
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.StringSpec
import io.kotest.matchers.shouldBe

class GreedyStringWithTrailingFlagArgumentTypeTest : StringSpec({
val flag = "my-flag"
val argumentType = greedyStringWithFlag(flag)

"should work without flag" {
val reader = StringReader("some text")
val result = argumentType.parse(reader)
result shouldBe StringWithFlag(
"some text",
false
)
reader.remainingLength shouldBe 0
}

"should work with flag" {
val reader = StringReader("some text $flag")
val result = argumentType.parse(reader)
result shouldBe StringWithFlag(
"some text",
true
)
reader.remainingLength shouldBe 0
}

"should not detect as flag without space" {
val reader = StringReader("some text$flag")
val result = argumentType.parse(reader)
result shouldBe StringWithFlag(
"some text$flag",
false
)
reader.remainingLength shouldBe 0
}

"should work with empty text and flag" {
val reader = StringReader(" $flag")
val result = argumentType.parse(reader)
result shouldBe StringWithFlag(
"",
true
)
reader.remainingLength shouldBe 0
}

"should fail when only flag is used" {
val reader = StringReader(flag)
val exception = shouldThrow<CommandSyntaxException> {
argumentType.parse(reader)
}
exception.message shouldBe "Argument consists only of flag 'my-flag' but does not contain a string at position 0: <--[HERE]"

// Cursor should not have been changed
reader.cursor shouldBe 0
}
})
Loading

0 comments on commit e6c0902

Please sign in to comment.