Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE-576: Allow complex ADB server commands execution #597

Merged
merged 14 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ package com.kaspersky.adbserver.commandtypes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General feedback.

I guess we can use Runtime::exec(String[] cmdarray) instead of Runtime::exec(String command) in CmdCommandPerformer.perform. Have a look at java.lang.Runtime, where both methods call the same method and command param in Runtime::exec(String command) is parsing into the array of strings.

I don't support introducing separate classes like ComplexAdbCommand. I think we need to add the new constructors like Command(command: String, arguments: List<String>) and the correspondent methods in AdbTerminal and etc.

I don't support syntaxis like (command: List<String>) and (command): String. It's confusing. Also, the most popular syntaxis in these cases is (command: String, arguments: List<String>). Yes, another command is an argument of the main command. Cases like sh -c "adb shell dumpsys deviceidle | grep mForceIdle" should be described in the documentation and examples.

import com.kaspersky.adbserver.common.api.Command
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also reflect all these cases in tests? I mean testing a single command, a command with arguments and a command with commands in arguments (complex commands with pipes and etc.).


data class AdbCommand(override val body: String) : Command(body)
data class AdbCommand(
override val command: String,
override val arguments: List<String> = emptyList()
) : Command(command, arguments)
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ package com.kaspersky.adbserver.commandtypes

import com.kaspersky.adbserver.common.api.Command

data class CmdCommand(override val body: String) : Command(body)
data class CmdCommand(
override val command: String,
override val arguments: List<String> = emptyList()
) : Command(command, arguments)
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ import java.io.Serializable
/**
* Command to execute by AdbServer
*/
abstract class Command(open val body: String) : Serializable
abstract class Command(open val command: String, open val arguments: List<String> = emptyList()) : Serializable
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.kaspersky.adbserver.device

import com.kaspersky.adbserver.common.api.CommandResult
import com.kaspersky.adbserver.commandtypes.AdbCommand
import com.kaspersky.adbserver.commandtypes.CmdCommand
import com.kaspersky.adbserver.common.api.CommandResult
import com.kaspersky.adbserver.common.log.LoggerFactory
import com.kaspersky.adbserver.common.log.logger.LogLevel
import com.kaspersky.adbserver.common.log.logger.Logger
Expand All @@ -27,10 +27,32 @@ object AdbTerminal {
AdbCommand(command)
) ?: throw IllegalStateException("Please first of all call [connect] method to establish a connection")

/**
* Allows more control over how arguments are parsed. Each element in the [arguments] list
* is used as is without tokenizing.
* Refer to the https://docs.oracle.com/javase/7/docs/api/java/lang/Runtime.html#exec(java.lang.String[])
*
* Please first of all call [connect] method to establish a connection
*/
fun executeAdb(command: String, arguments: List<String>): CommandResult = device?.fulfill(
AdbCommand(command, arguments)
) ?: throw IllegalStateException("Please first of all call [connect] method to establish a connection")

/**
* Please first of all call [connect] method to establish a connection
*/
fun executeCmd(command: String): CommandResult = device?.fulfill(
CmdCommand(command)
) ?: throw IllegalStateException("Please first of all call [connect] method to establish a connection")

/**
* Allows more control over how arguments are parsed. Each element in the [arguments] list
* is used as is without tokenizing.
* Refer to the https://docs.oracle.com/javase/7/docs/api/java/lang/Runtime.html#exec(java.lang.String[])
*
* Please first of all call [connect] method to establish a connection
*/
fun executeCmd(command: String, arguments: List<String>): CommandResult = device?.fulfill(
CmdCommand(command, arguments)
) ?: throw IllegalStateException("Please first of all call [connect] method to establish a connection")
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package com.kaspersky.adbserver.desktop

import com.kaspersky.adbserver.common.api.CommandResult
import com.kaspersky.adbserver.common.api.ExecutorResultStatus
import com.kaspersky.adbserver.common.log.logger.Logger
import java.util.Arrays
import java.util.concurrent.TimeUnit

internal class CmdCommandPerformer(
private val desktopName: String
private val desktopName: String,
private val logger: Logger,
) {

companion object {
Expand All @@ -15,9 +18,24 @@ internal class CmdCommandPerformer(
/**
* Be aware it's a synchronous method
*/
fun perform(command: String): CommandResult {
fun perform(command: String, arguments: List<String> = emptyList()): CommandResult {
val serviceInfo = "The command was executed on desktop=$desktopName"
val process = Runtime.getRuntime().exec(command)

val process = try {
if (arguments.isEmpty()) {
Runtime.getRuntime().exec(command)
} else {
val fullCommand = buildList {
add(command)
addAll(arguments)
}.toTypedArray()
Runtime.getRuntime().exec(fullCommand)
}
} catch (ex: Throwable) {
logger.e(ex.stackTraceToString())
return CommandResult(ExecutorResultStatus.FAILURE, "failed to start process. See exception in AdbServer logs")
}

try {
if (process.waitFor(EXECUTION_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
val exitCode = process.exitValue()
Expand All @@ -43,7 +61,7 @@ internal class CmdCommandPerformer(
serviceInfo = serviceInfo
)
} finally {
process.destroy()
process?.destroy()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,32 @@ internal class CommandExecutorImpl(

override fun execute(command: Command): CommandResult {
return when (command) {
is CmdCommand -> cmdCommandPerformer.perform(command.body)
is CmdCommand -> cmdCommandPerformer.perform(command.command, command.arguments)

is AdbCommand -> {
val adbCommand = "$adbPath ${ adbServerPort?.let { "-P $adbServerPort " } ?: "" }-s $deviceName ${command.body}"
logger.d("The created adbCommand=$adbCommand")
cmdCommandPerformer.perform(adbCommand)
val adbCommand: String
val adbArguments: List<String>

if (command.arguments.isEmpty()) {
adbCommand = "$adbPath ${adbServerPort?.let { "-P $adbServerPort " } ?: ""}-s $deviceName ${command.command}"
adbArguments = emptyList()
} else {
adbCommand = adbPath
adbArguments = buildList {
adbServerPort?.let {
add("-P")
add(adbServerPort)
}
add("-s")
add(deviceName)
add(command.command)
addAll(command.arguments)
}
}
logger.d("The created adbCommand=$adbCommand, arguments=$adbArguments")
cmdCommandPerformer.perform(adbCommand, adbArguments)
}

else -> throw UnsupportedOperationException("The command=$command is unsupported command")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal fun main(args: Array<String>) {

desktopLogger.i("Desktop started with arguments: emulators=$emulators, adbServerPort=$port, adbPath=$adbPath")

val cmdCommandPerformer = CmdCommandPerformer(desktopName)
val cmdCommandPerformer = CmdCommandPerformer(desktopName, desktopLogger)
val desktop = Desktop(
cmdCommandPerformer = cmdCommandPerformer,
presetEmulators = emulators,
Expand Down
Binary file modified artifacts/adbserver-desktop.jar
Binary file not shown.
21 changes: 21 additions & 0 deletions docs/Wiki/Executing_adb_commands.en.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ You can notice that the app uses `AdbTerminal` class to execute ADB commands.
## Usage in Kaspresso
In Kaspresso, we wrap `AdbTerminal` into a special interface `AdbServer`.
`AdbServer`'s instance is available in `BaseTestContext` scope and `BaseTestCase` with `adbServer` property: <br>
There're two types of AbdServer methods signatures:

`fun perform(vararg commands: String): List<String>` and `perform(command: String, arguments: List<String>): String` with an important difference.
First signature accept one or more commands and execute them one by one. Example below:
```kotlin
@Test
fun test() =
Expand All @@ -50,6 +54,23 @@ fun test() =
// ....
}
```
This method passes each command to the java runtime which tokenizes it by whitespaces. It could be not ideal. It can't be used for the commands with
the whitespaces in their arguments (e.g. `adb pull "/sdcard/Documents/path_with whitespace to/file.txt"`) and it doesn't allow piping (e.g. `cat $BUILD_DIR/report.txt | grep filte > filtered.txt`).
That's why there's a second AdbServer methods signature type.

It executes a single command and allows you to pass a list of arguments. It doesn't tokenize your command or arguments, they are used "as is".
This allows you to use piping and complex arguments
See example below (yes, a command could be an argument for another command):
```kotlin
@Test
fun test() = before{
adbServer.performCmd("bash", listOf("-c", "adb shell dumpsys deviceidle | grep mForceIdle"))
}.after {
}.run {
// ...
}
```

Also, don't forget to grant necessary permission:
```
<uses-permission android:name="android.permission.INTERNET" />
Expand Down
21 changes: 21 additions & 0 deletions docs/Wiki/Executing_adb_commands.ru.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
## Использование в Kaspresso
В Kaspresso мы оборачиваем AdbTerminal в специальный интерфейс AdbServer.
Экземпляр `AdbServer` доступен в области `BaseTestContext` и `BaseTestCase` со свойством `adbServer`: <br>
У AdbServer два типа сигнатур методов:

`fun perform(vararg commands: String): List<String>` и `perform(command: String, arguments: List<String>): String` с важными различиями между ними.
Первая сигнатура принимает одну или более комманду и исполняет их одну за одной. Пример далее:
```kotlin
@Test
fun test() =
Expand All @@ -50,6 +54,23 @@ fun test() =
// ....
}
```
Этот метод передает каждую команду среде выполнения java, которая разбивает ее по пробелам. Это может быть не самым лучшим вариантом. Он не может использоваться для команд с
пробелами в их аргументах (например, `adb pull "/sdcard/Documents/path_with whitespace to/file.txt"`) и не допускает пайпинга комманд (например, `cat $BUILD_DIR/report.txt | grep filte > filtered.txt`).
Вот почему существует второй тип сигнатур методов AdbServer.

Он выполняет одну команду и позволяет вам передавать список аргументов. Он не разбивает вашу команду или аргументы, они используются в том виде, в
котором вы их передали. Это позволяет вам использовать пайпинг и сложные аргументы.
Смотрите пример ниже (да, команда может быть аргументом для другой команды):
```kotlin
@Test
fun test() = before{
adbServer.performCmd("bash", listOf("-c", "adb shell dumpsys deviceidle | grep mForceIdle"))
}.after {
}.run {
// ...
}
```

Также не забудьте предоставить необходимое разрешение:
```
<uses-permission android:name="android.permission.INTERNET" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.kaspersky.kaspresso.device.server

import android.annotation.SuppressLint

/**
* This is a comfortable wrapper to work with AdbServer repository.
*
Expand All @@ -20,36 +22,108 @@ interface AdbServer {
*
* Required Permissions: INTERNET.
*
* @see <a href="https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md">AdbServer documentation</a>
* @param commands commands to execute.
* @throws AdbServerException if a result status of any command in @param commands is Failed
* @return list of answers of commands' execution
*/
@SuppressLint("SdCardPath")
@Deprecated("This method doesn't work for the commands with the complex arguments containing " +
"whitespaces (e.g. 'adb pull \"/sdcard/Documents/path_with whitespace to/file.txt\") and doesn't allow commands piping like" +
"adbServer.performCmd(\"bash\", listOf(\"-c\", \"adb shell dumpsys deviceidle | grep mForceIdle\"))" +
"which the AdbServer.performCmd(String, List<String>) does. " +
"For more details, please check out https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md",
ReplaceWith("adbServer.performCmd(*commands, emptyList())")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that it is correct replacement? You put an array (*commands) where adbServer.performCmd expects command: String.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the best from the worst. If you leave just commands then IDE will replace adbServer.performCmd("abc") with adbServer.performCmd(arrayOf("abc"), emptyList()) which will be highlighted as an error. I think that 90% of adbServer.perform... calls are with a signle command. For those cases, when there're several commands, then it will be replaced with adbServer.performCmd("asd", "abc", emptyList()) which is ugly too, but less possible. It's a tradeoff. I don't want to leave ReplaceWith statement with a stub, because it will upset users by erasing their precious commands

)
fun performCmd(vararg commands: String): List<String>

/**
* Performs shell commands blocking current thread. Allows more control over how arguments are parsed.
matzuk marked this conversation as resolved.
Show resolved Hide resolved
* Each list element is used as is. Refer to the https://docs.oracle.com/javase/8/docs/api/java/lang/ProcessBuilder.html.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please unify your comments. Somewhere you write class and method name, somewhere you provide a link.

*
* Please be aware! If any command that is in @param commands failed then AdbServerException will be thrown
*
* Required Permissions: INTERNET.
*
* @see <a href="https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md">AdbServer documentation</a>
* @param commands commands to execute.
* @throws AdbServerException if a result status of any command in @param commands is Failed
* @return list of answers of commands' execution
*/
fun performCmd(command: String, arguments: List<String>): String

/**
* Performs adb commands blocking current thread.
* Please be aware! If any command that is in @param commands failed then AdbServerException will be thrown
*
* Required Permissions: INTERNET.
*
* @see <a href="https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md">AdbServer documentation</a>
* @param commands commands to execute.
* @throws AdbServerException if a result status of any command in @param commands is Failed
* @return list of answers of commands' execution
*/
@SuppressLint("SdCardPath")
@Deprecated("This method doesn't work for the commands with the complex arguments containing " +
"whitespaces (e.g. 'adb pull \"/sdcard/Documents/path_with whitespace to/file.txt\") and doesn't allow commands piping like" +
"adbServer.performCmd(\"bash\", listOf(\"-c\", \"adb shell dumpsys deviceidle | grep mForceIdle\"))" +
"which the AdbServer.performAdb(String, List<String>) does " +
"For more details, please check out https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md",
ReplaceWith("adbServer.performAdb(*commands, emptyList())")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I am not sure about your replacement.

)
fun performAdb(vararg commands: String): List<String>

/**
* Performs adb commands blocking current thread. Allows more control over how arguments are parsed.
* Each list element is used as is. Refer to the https://docs.oracle.com/javase/8/docs/api/java/lang/ProcessBuilder.html.
*
* Please be aware! If any command that is in @param commands failed then AdbServerException will be thrown
*
* Required Permissions: INTERNET.
*
* @see <a href="https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md">AdbServer documentation</a>
* @param commands commands to execute.
* @throws AdbServerException if a result status of any command in @param commands is Failed
* @return list of answers of commands' execution
*/
fun performAdb(command: String, arguments: List<String>): String

/**
* Performs shell commands blocking current thread.
* Please be aware! If any command that is in @param commands failed then AdbServerException will be thrown
*
* Required Permissions: INTERNET.
*
* @see <a href="https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md">AdbServer documentation</a>
* @param commands commands to execute.
* @throws AdbServerException if a result status of any command in @param commands is Failed
* @return list of answers of commands' execution
*/
@SuppressLint("SdCardPath")
@Deprecated("This method doesn't work for the commands with the complex arguments containing " +
"whitespaces (e.g. 'adb pull \"/sdcard/Documents/path_with whitespace to/file.txt\") and doesn't allow commands piping like" +
"adbServer.performCmd(\"bash\", listOf(\"-c\", \"adb shell dumpsys deviceidle | grep mForceIdle\"))" +
"which the AdbServer.performShell(String, List<String>) does " +
"For more details, please check out https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md",
ReplaceWith("adbServer.performShell(*commands, emptyList())")
)
fun performShell(vararg commands: String): List<String>

/**
* Performs shell commands blocking current thread. Allows more control over how arguments are parsed.
* Each list element is used as is. Refer to the https://docs.oracle.com/javase/8/docs/api/java/lang/ProcessBuilder.html.
*
* Please be aware! If any command that is in @param commands failed then AdbServerException will be thrown
*
* Required Permissions: INTERNET.
*
* @see <a href="https://github.com/KasperskyLab/Kaspresso/blob/master/docs/Wiki/Executing_adb_commands.en.md">AdbServer documentation</a>
* @param commands commands to execute.
* @throws AdbServerException if a result status of any command in @param commands is Failed
* @return list of answers of commands' execution
*/
fun performShell(command: String, arguments: List<String>): String

/**
* Disconnect from AdbServer.
* The method is called by Kaspresso after each test.
Expand Down
Loading
Loading