Skip to content

Commit

Permalink
Merge pull request #1 from udaan-com/main_thread_blocking_resource
Browse files Browse the repository at this point in the history
[feat] added new plugin to check for jersey main thread blocking call
  • Loading branch information
priyesh-vp authored Sep 2, 2021
2 parents 6f0dccf + 5cda72d commit f27de56
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 1 deletion.
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Inspections provided:
- **BlockingCallContextReclaimable**: infer blocking calls within `Dispatchers.IO` context which can be migrated to
non-blocking alternatives
- **JerseyMethodParameterDefaultValue**: infer if a probable jersey method contains a parameter with a default value
- **JerseyMainThreadBlockingCall**: infer if a probable jersey resource method contains a main thread blocking call(runblocking)

## detekt run

Expand Down Expand Up @@ -105,3 +106,13 @@ and if not, throw a "Parameter specified as non-null is null" error on invocatio

- [KT-27947](https://youtrack.jetbrains.com/issue/KT-27947)
- [KT-28684](https://youtrack.jetbrains.com/issue/KT-28684)


## JerseyMainThreadBlockingCall

```yml
sidekt:
JerseyMainThreadBlockingCall:
active: true
# debug: 'stderr'
```
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package io.github.thewisenerd.linters.sidekt

import io.github.thewisenerd.linters.sidekt.rules.BlockingCallContext
import io.github.thewisenerd.linters.sidekt.rules.BlockingCallContextReclaimable
import io.github.thewisenerd.linters.sidekt.rules.JerseyMainThreadBlockingCall
import io.github.thewisenerd.linters.sidekt.rules.JerseyMethodParameterDefaultValue
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.RuleSet
Expand All @@ -15,7 +16,8 @@ class SideKtRuleSetProvider : RuleSetProvider {
listOf(
BlockingCallContext(config),
BlockingCallContextReclaimable(config),
JerseyMethodParameterDefaultValue(config)
JerseyMethodParameterDefaultValue(config),
JerseyMainThreadBlockingCall(config)
)
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package io.github.thewisenerd.linters.sidekt.rules

import io.github.thewisenerd.linters.sidekt.helpers.Debugger
import io.gitlab.arturbosch.detekt.api.*
import io.gitlab.arturbosch.detekt.rules.hasAnnotation
import org.jetbrains.kotlin.psi.*

class JerseyMainThreadBlockingCall(config: Config) : Rule(config) {

companion object {
const val RUN_BLOCKING_EXP_ID = "runBlocking"
val httpMethodList = arrayOf("GET", "POST", "PUT", "DELETE", "HEAD", "OPTIONS")
}

private val debugStream by lazy {
valueOrNull<String>("debug")?.let {
Debugger.getOutputStreamForDebugger(it)
}
}

override val issue: Issue = Issue(
id = JerseyMainThreadBlockingCall::class.java.simpleName,
severity = Severity.Performance,
description = "main thread blocking call in resource method",
debt = Debt.TEN_MINS
)


override fun visitNamedFunction(resourceMethod: KtNamedFunction) {
super.visitNamedFunction(resourceMethod)
val dbg = Debugger.make(JerseyMainThreadBlockingCall::class.java.simpleName, debugStream)

val hasPathAnnotation = resourceMethod.hasAnnotation("Path")
val hasAnyOfHttpMethodAnnotation = resourceMethod.hasAnnotation(*httpMethodList)

if (!hasPathAnnotation && !hasAnyOfHttpMethodAnnotation) {
dbg.i(" hasPathAnnotation=false or none of http method annotation available")
return
}

var methodBlockingMainThread = false
resourceMethod.children.forEach {
if(it is KtCallExpression) {
val isUsingRunBlockingExpression = (it.calleeExpression as KtNameReferenceExpression).getReferencedNameAsName().identifier == RUN_BLOCKING_EXP_ID
if(isUsingRunBlockingExpression) {
methodBlockingMainThread = true
}
}

if(it is KtBlockExpression) {
it.children.map { blkExp ->
if(blkExp is KtCallExpression) {
val identifiedBlockingExp = (blkExp.calleeExpression as KtNameReferenceExpression).getReferencedNameAsName().identifier == RUN_BLOCKING_EXP_ID
if(identifiedBlockingExp){
methodBlockingMainThread = true
}
}

if(blkExp is KtReturnExpression) {
blkExp.children.filterIsInstance<KtCallExpression>().map { retExp ->
val identifiedBlockingExp = (retExp.calleeExpression as KtNameReferenceExpression).getReferencedNameAsName().identifier == RUN_BLOCKING_EXP_ID
if(identifiedBlockingExp){
methodBlockingMainThread = true
}
}
}
}
}
}

if(methodBlockingMainThread) {
dbg.i("blocks main application thread")
report(
CodeSmell(
issue = issue,
entity = Entity.from(resourceMethod),
message = "jersey resource method (${resourceMethod.name}) have main thread blocking call)"
)
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package io.github.thewisenerd.linters.sidekt

import io.github.thewisenerd.linters.sidekt.rules.JerseyMainThreadBlockingCall
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.api.SourceLocation
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.junit.Test

class JerseyMainThreadBlockingCallTest {
companion object {
private val blockingJerseyMethod = JerseyMainThreadBlockingCall::class.java.simpleName

private fun ensureJerseyMethodParameterDefaultValueFindings(findings: List<Finding>, requiredFindings: List<SourceLocation>) =
TestUtils.ensureFindings(blockingJerseyMethod, findings, requiredFindings)
}

private val testConfig = object : Config {
override fun subConfig(key: String): Config = this

@Suppress("UNCHECKED_CAST")
override fun <T : Any> valueOrNull(key: String): T? {
return when (key) {
"active" -> true as? T
"debug" -> "stderr" as? T
else -> null
}
}
}
private val subject = JerseyMainThreadBlockingCall(testConfig)


@Test
fun simple06() {
val code = TestUtils.readFile("simple06.kt")
val findings = subject.compileAndLintWithContext(TestUtils.env, code)
ensureJerseyMethodParameterDefaultValueFindings(findings, listOf(
SourceLocation(3, 5),
SourceLocation(8, 5),
SourceLocation(13, 5),
SourceLocation(18, 5),
SourceLocation(25, 5),
SourceLocation(42, 5)
))
}

}
51 changes: 51 additions & 0 deletions src/test/resources/simple06.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import kotlinx.coroutines.runBlocking

@Path("/variant01")
fun testRunBlockingVariant01() = runBlocking {
"This is variation 01"
}

@Path("/variant02")
fun testRunBlockingVariant02() : String = runBlocking{
"This is variation 02"
}

@Path("/variant03")
fun testRunBlockingVariant03() = runBlocking{
return@runBlocking "This is variation 03"
}

@Path("/variant04")
fun testRunBlockingVariant04() : String {
return runBlocking{
return@runBlocking "This is variation 04"
}
}

@Path("/variant05")
fun testRunBlockingVariant05() : String {
println("This is variation 05")

runBlocking {
"This is variation 05"
}

return "This is variation 05"
}

@Path("/variant06")
fun testRunBlockingVariant06() : String {
println("This is variation 06")
return "This is variation 06"
}

@GET
fun testRunBlockingVariant07() = runBlocking {
println("This is variation 07")
}

@GET
fun testRunBlockingVariant08() : String {
println("This is variation 08")
return "This is variation 08"
}

0 comments on commit f27de56

Please sign in to comment.