I don't believe there's a problem with Shiro using ThreadLocals.
My original intention of this discussion thread was to promote Unit Testing
and use of Mockito for any feature that's more complex than
StringUtils.isEmpty(String value). Just because the security class is
currently called GeodeSecur
I'm specifically advocating wrapping the class with an interface to
facilitate mocking for unit testing classes that depend on it. This way we
can use Mockito ArgumentCaptor to assert that the correct permission
strings are being passed to our security component. We can also provide
fake returns fr
Security is one of those exceptions to the rules about ThreadLocal. Almost
every implementation uses ThreadLocal to stash the current executing
context. Generally then there is a static class that gets the current
context. To test you should be able to just push your own mocked context
into the con
Shiro relies heavily on ThreadLocal. GeodeSecurityUtils is a wrapper around
Shiro. That's where all the static usage stems from. There is a trade-off
if we pass some sort session object around: calling code would need to keep
a reference of it and knows when/where/which to pass it down.
On Mon, Ju
IMHO static helper methods like those in Bytes are fine. We would never
want to substitute a mock implementation of one of those methods and they
don't have any collaborators.
Just glancing at GeodeSecurityUtils, what I don't like is that it's relying
heavily on mutable static singletons (security
Jinmei,
You describe a good solution for testing GeodeSecurityUtil directly in
Integration Tests. This does not however address unit testing of classes
that depend on GeodeSecurityUtil.
Such classes need to be completely isolated in the unit test. Then the
behavior needs to be tested including th
I was trying to describe any Java class in Geode which provides feature
functionality that should be unit tested and integration tested, but is
either currently implemented as a static class or which collaborates with a
non-trivial component currently implemented as a static class.
I wasn't descri
Great discussion! I’ve got two questions:
1) What do you think of Bytes.java
(:geode-core:com.gemstone.gemfire.internal.util)? Is it a reasonable use of
the Helper / Utility class pattern? If not, how would you change it? Creating
a new instance for each use seems inefficient and using a Si
Wouldn't the change be too intrusive? Now any class that uses security
would need to have this additional constructor, and if a no-arg constructor
is available, it's error prone that the class will be used without
security, and if we delete the no-arg constructor, that would cause a lot
of chain re
To avoid adding boilerplate code, we could just avoid writing any new
static methods or classes of static methods. Remember static is bad in OO
and is one of the main things that prevents true, good unit testing because
you can't replace the class or the method with a fake of any sort (mock,
spy, o
One other to bear in mind is to avoid adding too much boiler code into the
production solely for the purpose of testing. I am typically a OO person,
but I wouldn't sacrifice code clarity and readability for it.
On Fri, Jul 15, 2016 at 12:14 PM, Kirk Lund wrote:
> We recently introduced GeodeSecu
11 matches
Mail list logo
