Merge pull request #11 from agorapulse/fix/cast-error-on-non-iterable…

…-container

Fix cast error on non iterable continer

docs/guide/src/docs/asciidoc/usage.adoc

@@ -28,6 +28,8 @@ The permission can be any `String`. The semantics are given by the implementatio
 For methods with multiple arguments, every argument must pass verification by the defined  <<_permission_advisors, permission advisor>>.
 There must be a <<_permission_advisors, permission advisor>> defined for at least one of the arguments.
 
+It is possible to validate a typed Iterable argument, <<_permission_advisors, permission advisor>> will apply to all of its items based on their type defined in method signature.
+
 [[_permission_advisors]]
 == Permission Advisors
 

libs/micronaut-permissions/src/main/groovy/com/agorapulse/permissions/DefaultPermissionChecker.java

@@ -36,7 +36,7 @@ public DefaultPermissionChecker(List<PermissionAdvisor<?>> advisors) {
     @SuppressWarnings("unchecked")
     @Override
     public <T> PermissionCheckResult checkPermission(String permissionDefinition, T value, Argument<T> valueType) {
-        if (valueType.isContainerType() && valueType.hasTypeVariables()) {
+        if (Iterable.class.isAssignableFrom(valueType.getType()) && valueType.hasTypeVariables()) {
             return checkPermissionOnContainer(permissionDefinition, (Iterable<Object>) value, (Argument<Iterable<Object>>) valueType);
         }
 

libs/micronaut-permissions/src/test/groovy/com/agorapulse/permissions/HandleCollectionRequest.java → libs/micronaut-permissions/src/test/groovy/com/agorapulse/permissions/HandleIterableRequest.java

@@ -17,16 +17,16 @@
  */
 package com.agorapulse.permissions;
 
-import java.util.Collection;
+import java.util.List;
 
-public class HandleCollectionRequest {
-    private Collection<Long> ids;
+public class HandleIterableRequest {
+    private List<Long> ids;
 
-    public Collection<Long> getIds() {
+    public List<Long> getIds() {
         return ids;
     }
 
-    public void setIds(Collection<Long> ids) {
+    public void setIds(List<Long> ids) {
         this.ids = ids;
     }
 }

libs/micronaut-permissions/src/test/groovy/com/agorapulse/permissions/PostController.java

@@ -24,6 +24,7 @@
 import io.micronaut.http.hateoas.JsonError;
 
 import javax.annotation.Nullable;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -63,13 +64,21 @@ public Post merge(@Nullable @Header("X-User-Id") Long userId, @Body PostMergeReq
         return postRepository.save(mergedPost);
     }
 
-    @io.micronaut.http.annotation.Post("/handle-collection")
-    public void handleCollection(@Body HandleCollectionRequest handleCollectionRequest) {
-        List<Post> posts = handleCollectionRequest.getIds() != null ? handleCollectionRequest.getIds()
+    @io.micronaut.http.annotation.Post("/handle-iterable-container")
+    public void handleIterableContainer(@Body HandleIterableRequest handleIterableRequest) {
+        List<Post> posts = handleIterableRequest.getIds() != null ? handleIterableRequest.getIds()
             .stream()
             .map(postRepository::get)
             .collect(Collectors.toList()) : null;
-        postService.handleCollection(posts);
+        postService.handleIterableContainer(posts);
+    }
+
+    @Get("/handle-non-iterable-container")
+    public void handleContainerNonIterable() {
+        Post post = postService.create(1L, "message");
+        LinkedHashMap<String, String> nonIterableContainer = new LinkedHashMap<>();
+        nonIterableContainer.put("test", "test");
+        postService.handleContainerNonIterable(post, nonIterableContainer);
     }
 
     // tag::error[]

libs/micronaut-permissions/src/test/groovy/com/agorapulse/permissions/PostControllerSpec.groovy

@@ -21,19 +21,26 @@ import com.agorapulse.gru.Gru
 import io.micronaut.test.annotation.MicronautTest
 import spock.lang.AutoCleanup
 import spock.lang.Specification
-import spock.lang.Stepwise
 
 import javax.inject.Inject
 
-@Stepwise
 @MicronautTest
 @SuppressWarnings([
     'BuilderMethodWithSideEffects',
     'FactoryMethodName',
 ])
 class PostControllerSpec extends Specification {
 
+    private static final String AUTH_ID_1 = '1'
+    private static final String AUTH_ID_2 = '2'
+    private static final String HELLO_MESSAGE = 'Hello'
+
     @Inject @AutoCleanup Gru gru
+    @Inject PostRepository postRepository
+
+    void cleanup() {
+        postRepository.clean()
+    }
 
     void 'create post without any auth'() {
         expect:
@@ -62,35 +69,10 @@ class PostControllerSpec extends Specification {
             }
     }
 
-    void 'create post with same auth'() {
-        expect:
-            gru.test {
-                post '/post', {
-                    headers 'X-User-Id': '1'
-                    json message: 'Hello'
-                }
-                expect {
-                    status CREATED
-                    json 'newPost2.json'
-                }
-            }
-    }
-
-    void 'create post with another auth'() {
-        expect:
-            gru.test {
-                post '/post', {
-                    headers 'X-User-Id': '2'
-                    json message: 'Hello'
-                }
-                expect {
-                    status CREATED
-                    json 'newPostOtherAuth.json'
-                }
-            }
-    }
-
     void 'publish post without any auth'() {
+        given:
+            Post post = Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE)
+            postRepository.save(post)
         expect:
             gru.test {
                 put '/post/1'
@@ -102,10 +84,13 @@ class PostControllerSpec extends Specification {
     }
 
     void 'publish post'() {
+        given:
+            Post post = Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE)
+            postRepository.save(post)
         expect:
             gru.test {
                 put '/post/1', {
-                    headers 'X-User-Id': '1'
+                    headers 'X-User-Id': AUTH_ID_1
                 }
                 expect {
                     json 'publishedPost.json'
@@ -114,10 +99,13 @@ class PostControllerSpec extends Specification {
     }
 
     void 'publish post with wrong auth'() {
+        given:
+            Post post = Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE)
+            postRepository.save(post)
         expect:
             gru.test {
                 put '/post/1', {
-                    headers 'X-User-Id': '3'
+                    headers 'X-User-Id': AUTH_ID_2
                 }
                 expect {
                     status UNAUTHORIZED
@@ -127,6 +115,9 @@ class PostControllerSpec extends Specification {
     }
 
     void 'archive post without any auth'() {
+        given:
+            Post post = Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE)
+            postRepository.save(post)
         expect:
             gru.test {
                 delete '/post/1'
@@ -138,6 +129,9 @@ class PostControllerSpec extends Specification {
     }
 
     void 'archive post'() {
+        given:
+            Post post = Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE)
+            postRepository.save(post)
         expect:
             gru.test {
                 delete '/post/1', {
@@ -150,11 +144,14 @@ class PostControllerSpec extends Specification {
     }
 
     void 'merge posts with one not allowed'() {
+        given:
+            postRepository.save(Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE))
+            postRepository.save(Post.createDraft(AUTH_ID_2.toLong(), HELLO_MESSAGE))
         expect:
             gru.test {
                 post '/post/merge', {
                     headers 'X-User-Id': '1'
-                    json id1: '1', id2: '3'
+                    json id1: '1', id2: '2'
                 }
                 expect {
                     status UNAUTHORIZED
@@ -164,6 +161,9 @@ class PostControllerSpec extends Specification {
     }
 
     void 'merge posts'() {
+        given:
+            postRepository.save(Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE))
+            postRepository.save(Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE))
         expect:
             gru.test {
                 post '/post/merge', {
@@ -177,10 +177,10 @@ class PostControllerSpec extends Specification {
             }
     }
 
-    void 'handle null collection'() {
+    void 'handle null iterable container'() {
         expect:
             gru.test {
-                post '/post/handle-collection', {
+                post '/post/handle-iterable-container', {
                     headers 'X-User-Id': '1'
                     json ids: null
                 }
@@ -190,10 +190,10 @@ class PostControllerSpec extends Specification {
             }
     }
 
-    void 'handle empty collection'() {
+    void 'handle empty iterable container'() {
         expect:
             gru.test {
-                post '/post/handle-collection', {
+                post '/post/handle-iterable-container', {
                     headers 'X-User-Id': '1'
                     json ids: []
                 }
@@ -203,23 +203,29 @@ class PostControllerSpec extends Specification {
             }
     }
 
-    void 'handle collection containing one not allowed'() {
+    void 'handle iterable containing one not allowed'() {
+        given:
+            postRepository.save(Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE))
+            postRepository.save(Post.createDraft(AUTH_ID_2.toLong(), HELLO_MESSAGE))
         expect:
             gru.test {
-                post '/post/handle-collection', {
+                post '/post/handle-iterable-container', {
                     headers 'X-User-Id': '1'
-                    json ids: [1, 3]
+                    json ids: [1, 2]
                 }
                 expect {
                     status UNAUTHORIZED
                 }
             }
     }
 
-    void 'handle collection containing only allowed'() {
+    void 'handle iterable container containing only allowed'() {
+        given:
+            postRepository.save(Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE))
+            postRepository.save(Post.createDraft(AUTH_ID_1.toLong(), HELLO_MESSAGE))
         expect:
             gru.test {
-                post '/post/handle-collection', {
+                post '/post/handle-iterable-container', {
                     headers 'X-User-Id': '1'
                     json ids: [1, 2]
                 }
@@ -229,4 +235,16 @@ class PostControllerSpec extends Specification {
             }
     }
 
+    void 'ignore non iterable container'() {
+        expect:
+            gru.test {
+                get '/post/handle-non-iterable-container', {
+                    headers 'X-User-Id': '1'
+                }
+                expect {
+                    status OK
+                }
+            }
+    }
+
 }

libs/micronaut-permissions/src/test/groovy/com/agorapulse/permissions/PostRepository.java

@@ -24,7 +24,7 @@
 @Singleton
 public class PostRepository {
 
-    private final Map<Long, Post> posts = new HashMap<>();
+    private Map<Long, Post> posts = new HashMap<>();
     private long counter;
 
     public Post save(Post post) {
@@ -39,4 +39,9 @@ public Post get(Long id) {
         return posts.get(id);
     }
 
+    public void clean() {
+        posts = new HashMap<>();
+        counter = 0;
+    }
+
 }

libs/micronaut-permissions/src/test/groovy/com/agorapulse/permissions/PostService.java

@@ -19,6 +19,7 @@
 
 import javax.inject.Singleton;
 import java.util.Collection;
+import java.util.Map;
 
 @Singleton
 public class PostService {
@@ -36,9 +37,14 @@ public Post archive(Post post) {
     }
 
     @RequiresPermission("edit")
-    public void handleCollection(Collection<Post> posts) {
+    public void handleIterableContainer(Collection<Post> posts) {
     }
 
+    @RequiresPermission("edit")
+    public void handleContainerNonIterable(Post post, Map<String, String> couldBeIterableContainer) {
+    }
+
+
     @RequiresPermission("edit")
     public Post publish(Post post) {
         return post.publish();

libs/micronaut-permissions/src/test/resources/com/agorapulse/permissions/PostControllerSpec/mergedPost.json

@@ -1 +1 @@
-{"id":4,"status":"DRAFT","authorId":1,"message":"HelloHello"}
+{"id":3,"status":"DRAFT","authorId":1,"message":"HelloHello"}