From 50d26c9d28e04edf9ec7434ca9002b18bebaa00a Mon Sep 17 00:00:00 2001 From: Karl Goffin Date: Mon, 29 Oct 2018 23:54:55 -0400 Subject: [PATCH] Polish Logging and Tests Removing debug statements which would have prematurely terminated the stream, changing to AssertJ, and adding another test. Issue: gh-3743 --- ...efaultMethodSecurityExpressionHandler.java | 11 +------- ...tMethodSecurityExpressionHandlerTests.java | 25 +++++++++++++++---- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java index fbac0af8f1..af48f2a31a 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java @@ -175,21 +175,12 @@ public class DefaultMethodSecurityExpressionHandler extends if (filterTarget instanceof Stream) { final Stream original = (Stream) filterTarget; - if (debug) { - logger.debug("Filtering stream with " + original.count() + " elements"); - } - Stream filtered = original.filter(filterObject -> { + return original.filter(filterObject -> { rootObject.setFilterObject(filterObject); return ExpressionUtils.evaluateAsBoolean(filterExpression, ctx); }) .onClose(original::close); - - if (debug) { - logger.debug("Retaining elements: " + filtered.collect(Collectors.toList())); - } - - return filtered; } throw new IllegalArgumentException( diff --git a/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java b/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java index b7ff6377bf..a4b232874c 100644 --- a/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java +++ b/core/src/test/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandlerTests.java @@ -15,11 +15,13 @@ */ package org.springframework.security.access.expression.method; -import static org.mockito.Mockito.verify; +import static org.assertj.core.api.Assertions.*; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; import org.aopalliance.intercept.MethodInvocation; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -85,9 +87,22 @@ public class DefaultMethodSecurityExpressionHandlerTests { Object filtered = handler.filter(stream, expression, context); - Assert.assertTrue("response was wrong type", Stream.class.isAssignableFrom(filtered.getClass())); + assertThat(filtered).isInstanceOf(Stream.class); List list = ((Stream) filtered).collect(Collectors.toList()); - Assert.assertEquals(2, list.size()); - Assert.assertFalse("contains filtered element", list.contains("2")); + assertThat(list).containsExactly("1", "3"); + } + + @Test + public void filterStreamWhenClosedThenUpstreamGetsClosed() { + final Stream upstream = mock(Stream.class); + doReturn(Stream.empty()).when(upstream).filter(any()); + + Expression expression = handler.getExpressionParser().parseExpression("true"); + + EvaluationContext context = handler.createEvaluationContext(authentication, + methodInvocation); + + ((Stream) handler.filter(upstream, expression, context)).close(); + verify(upstream).close(); } }