From 4431738a81fb7f8668c963dad73725340bce1706 Mon Sep 17 00:00:00 2001 From: Vipin Sharma <43703044+shharma-vipin@users.noreply.github.com> Date: Tue, 31 Aug 2021 13:37:15 +0530 Subject: [PATCH 1/3] [feat] added new plugin to check for resource method blocking call --- .../rules/JerseyMainThreadBlockingCall.kt | 82 +++++++++++++++++++ .../JerseyMainThreadBlockingCallTest.kt | 47 +++++++++++ src/test/resources/simple06.kt | 51 ++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 src/main/kotlin/io/github/thewisenerd/linters/sidekt/rules/JerseyMainThreadBlockingCall.kt create mode 100644 src/test/kotlin/io/github/thewisenerd/linters/sidekt/JerseyMainThreadBlockingCallTest.kt create mode 100644 src/test/resources/simple06.kt diff --git a/src/main/kotlin/io/github/thewisenerd/linters/sidekt/rules/JerseyMainThreadBlockingCall.kt b/src/main/kotlin/io/github/thewisenerd/linters/sidekt/rules/JerseyMainThreadBlockingCall.kt new file mode 100644 index 0000000..137c142 --- /dev/null +++ b/src/main/kotlin/io/github/thewisenerd/linters/sidekt/rules/JerseyMainThreadBlockingCall.kt @@ -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("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().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)" + ) + ) + } + } +} \ No newline at end of file diff --git a/src/test/kotlin/io/github/thewisenerd/linters/sidekt/JerseyMainThreadBlockingCallTest.kt b/src/test/kotlin/io/github/thewisenerd/linters/sidekt/JerseyMainThreadBlockingCallTest.kt new file mode 100644 index 0000000..c54e140 --- /dev/null +++ b/src/test/kotlin/io/github/thewisenerd/linters/sidekt/JerseyMainThreadBlockingCallTest.kt @@ -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, requiredFindings: List) = + TestUtils.ensureFindings(blockingJerseyMethod, findings, requiredFindings) + } + + private val testConfig = object : Config { + override fun subConfig(key: String): Config = this + + @Suppress("UNCHECKED_CAST") + override fun 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) + )) + } + +} \ No newline at end of file diff --git a/src/test/resources/simple06.kt b/src/test/resources/simple06.kt new file mode 100644 index 0000000..206aa2f --- /dev/null +++ b/src/test/resources/simple06.kt @@ -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" + } \ No newline at end of file From d2935d7e072cc2eebb16544d7ef5c11bd5cbb1bd Mon Sep 17 00:00:00 2001 From: Vipin Sharma <43703044+shharma-vipin@users.noreply.github.com> Date: Tue, 31 Aug 2021 13:39:55 +0530 Subject: [PATCH 2/3] [feat] updated readme file --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 2bd290f..2ed86cb 100644 --- a/README.md +++ b/README.md @@ -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 From 5cda72d61f65f28aac7fe1b1d5c636afc6d44784 Mon Sep 17 00:00:00 2001 From: Vipin Sharma <43703044+shharma-vipin@users.noreply.github.com> Date: Thu, 2 Sep 2021 11:44:06 +0530 Subject: [PATCH 3/3] addressed review comments --- README.md | 10 ++++++++++ .../linters/sidekt/SideKtRuleSetProvider.kt | 4 +++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2ed86cb..5bf2df4 100644 --- a/README.md +++ b/README.md @@ -106,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' +``` diff --git a/src/main/kotlin/io/github/thewisenerd/linters/sidekt/SideKtRuleSetProvider.kt b/src/main/kotlin/io/github/thewisenerd/linters/sidekt/SideKtRuleSetProvider.kt index 06b2948..3a0d731 100644 --- a/src/main/kotlin/io/github/thewisenerd/linters/sidekt/SideKtRuleSetProvider.kt +++ b/src/main/kotlin/io/github/thewisenerd/linters/sidekt/SideKtRuleSetProvider.kt @@ -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 @@ -15,7 +16,8 @@ class SideKtRuleSetProvider : RuleSetProvider { listOf( BlockingCallContext(config), BlockingCallContextReclaimable(config), - JerseyMethodParameterDefaultValue(config) + JerseyMethodParameterDefaultValue(config), + JerseyMainThreadBlockingCall(config) ) ) } \ No newline at end of file