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.

Sign up for GitHub

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

Jump to bottom

Fix: Ignore @Cacheable Annotation on Controller Methods #209

Open
wants to merge 1 commit into
base: 7.0.x
Choose a base branch
from
Open

Fix: Ignore @Cacheable Annotation on Controller Methods #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ grails-app/conf/BootStrap.groovy
grails-app/conf/spring/
grails-app/views/error.gsp
grails-app/views/index.gsp
bin/
7 changes: 7 additions & 0 deletions grails-app/controllers/com/demo/DemoController.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package com.demo

import grails.plugin.cache.Cacheable

class DemoController {

def basicCachingService
def grailsCacheAdminService

@Cacheable('show')
def show (params) {
[param: params.id]
}

def clearBlocksCache() {
grailsCacheAdminService.clearBlocksCache()
render "cleared blocks cache"
Expand Down
1 change: 1 addition & 0 deletions grails-app/views/demo/show.gsp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello World!${param}
7 changes: 7 additions & 0 deletions src/ast/groovy/org/grails/plugin/cache/compiler/CacheableTransformation.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ class CacheableTransformation extends AbstractCacheTransformation {
@Override
protected Expression buildDelegatingMethodCall(SourceUnit sourceUnit, AnnotationNode annotationNode, ClassNode classNode, MethodNode methodNode, MethodCallExpression originalMethodCallExpr, BlockStatement newMethodBody) {

boolean isControllerClass = classNode.name.endsWith('Controller')
boolean isServiceClass = classNode.name.endsWith('Service')

if (isControllerClass && !isServiceClass) {
Copy link
Member

Choose a reason for hiding this comment

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

I am confused about the if (isControllerClass && !isServiceClass) clause. I think that says if the class name ends in 'Controller' and doesn't end in 'Service', then return the original method. What is the point of checking if the class name ends in 'Service' if the condition only applies if the class name ends in 'Controller'?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your observation. You are right, the condition if (isControllerClass && !isServiceClass) is redundant. Does it make sense to you to simplify it to just if (isControllerClass)?

return originalMethodCallExpr
}

VariableExpression cacheManagerVariableExpression = varX(GRAILS_CACHE_MANAGER_PROPERTY_NAME)
handleCacheCondition(sourceUnit, annotationNode, methodNode, originalMethodCallExpr, newMethodBody)

Expand Down
22 changes: 22 additions & 0 deletions src/integration-test/groovy/com/demo/NotCachingControllerIntegrationSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.demo

import geb.spock.GebSpec
import grails.testing.mixin.integration.Integration

@Integration
class NotCachingControllerIntegrationSpec extends GebSpec {

void 'test action controller with different parameters'() {
when:
go '/demo/show/1'

then:
$().text().contains 'Hello World!1'

when:
go '/demo/show/2'

then:
$().text().contains 'Hello World!2'
}
}