You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by "Daniel L. Rall" <dl...@finemaltcoding.com> on 2004/02/03 10:30:32 UTC
Re: cvs commit: jakarta-turbine-fulcrum/security/nt/src/test/org/apache/fulcrum/security/nt/dynamic
NTUserManagerTest.java
This is an incredibly unscalable API (not picking on Eric here, just
commenting on the interface). What if you have 100k users in your system? I
recommend deprecating this method and changing all implementations to throw
some sort of exception. In the very unlikely case that someone does need this
method (you're paginating your lists of users, right? ;), a method which
returns an Iterator can be used. In the past I've also hacked up solutions
which cursor over a result set, but this requires that you explicitly invoke
some sort of cleanup method when done with the List, or risk leaking
connections to your data source.
Thoughts?
epugh@apache.org wrote:
> epugh 2004/01/10 08:15:17
>
> Modified: security/nt/src/java/org/apache/fulcrum/security/nt
> NTUserManagerImpl.java
> security/nt/src/test/org/apache/fulcrum/security/nt/dynamic
> NTUserManagerTest.java
> Log:
> add getAllUsers
>
> Revision Changes Path
> 1.2 +13 -1 jakarta-turbine-fulcrum/security/nt/src/java/org/apache/fulcrum/security/nt/NTUserManagerImpl.java
>
> Index: NTUserManagerImpl.java
> ===================================================================
> RCS file: /home/cvs/jakarta-turbine-fulcrum/security/nt/src/java/org/apache/fulcrum/security/nt/NTUserManagerImpl.java,v
> retrieving revision 1.1
> retrieving revision 1.2
> diff -u -r1.1 -r1.2
> --- NTUserManagerImpl.java 5 Dec 2003 23:22:18 -0000 1.1
> +++ NTUserManagerImpl.java 10 Jan 2004 16:15:17 -0000 1.2
> @@ -65,6 +65,7 @@
> import org.apache.fulcrum.security.util.EntityExistsException;
> import org.apache.fulcrum.security.util.PasswordMismatchException;
> import org.apache.fulcrum.security.util.UnknownEntityException;
> +import org.apache.fulcrum.security.util.UserSet;
>
> import com.tagish.auth.win32.NTSystem;
> /**
> @@ -270,5 +271,16 @@
> {
> throw new RuntimeException("Not supported by NT User Manager");
> }
> +
> + /**
> + * Retrieves all users defined in the system.
> + *
> + * @return the names of all users defined in the system.
> + * @throws DataBackendException if there was an error accessing the data backend.
> + */
> + public UserSet getAllUsers() throws DataBackendException
> + {
> + throw new RuntimeException("Not supported by NT User Manager");
> + }
>
> }
>
>
>
> 1.2 +17 -2 jakarta-turbine-fulcrum/security/nt/src/test/org/apache/fulcrum/security/nt/dynamic/NTUserManagerTest.java
>
> Index: NTUserManagerTest.java
> ===================================================================
> RCS file: /home/cvs/jakarta-turbine-fulcrum/security/nt/src/test/org/apache/fulcrum/security/nt/dynamic/NTUserManagerTest.java,v
> retrieving revision 1.1
> retrieving revision 1.2
> diff -u -r1.1 -r1.2
> --- NTUserManagerTest.java 5 Dec 2003 23:22:18 -0000 1.1
> +++ NTUserManagerTest.java 10 Jan 2004 16:15:17 -0000 1.2
> @@ -199,7 +199,7 @@
> /*
> * Class to test for User retrieve(String, String)
> */
> - public void testRetrieveStringString() throws Exception
> + public void testGetUserStringString() throws Exception
> {
> try
> {
> @@ -239,10 +239,25 @@
> }
> }
> /** ******* ALL BELOW HERE THROW RUNTIME EXCEPTIONS ******** */
> + /*
> + * Class to test for User retrieve(String, String)
> + */
> + public void testGetAllUsers() throws Exception
> + {
> + try
> + {
> + userManager.getAllUsers();
> + fail("Should throw runtime exception");
> + }
> + catch (RuntimeException re)
> + {
> + assertTrue(re.getMessage().equals(ERROR_MSG));
> + }
> + }
> /*
> * Class to test for User retrieve(String)
> */
> - public void testRetrieveString() throws Exception
> + public void testGetUserString() throws Exception
> {
> try
> {
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
RE: cvs commit: jakarta-turbine-fulcrum/security/nt/src/test/org/apache/fulcrum/security/nt/dynamic NTUserManagerTest.java
Posted by Eric Pugh <ep...@upstate.com>.
I actually totally agree with this.. Unfortunantly I didn't start from
scratch on the API. i basically took in what I had inherited from the
original Fulcrum Security api. My focus was more on providing multiple
implementations versus reworking the API.
There are definitly parts of the external API that I don't like.
Fortunantly, this really isn't released, so I think we could just ditch the
method and replace it with something better.
I was thinking that things like getAllUsers, which may occasionally be
needed, should be something that is provided via the model. In other words,
in one model it would be by group, another might be by role, versus just a
blanket "getAllUsers" which is probably not what most people need.
Would your recommendation be to replace it with Iterator then?
I appreciate the feedback as a lot of things where inherited or developed in
more vacuum then I like.
Eric
> -----Original Message-----
> From: Daniel L. Rall [mailto:dlr@finemaltcoding.com]
> Sent: Tuesday, February 03, 2004 10:31 AM
> To: Turbine Developers List
> Subject: Re: cvs commit:
> jakarta-turbine-fulcrum/security/nt/src/test/org/apache/fulcru
> m/security
> /nt/dynamic NTUserManagerTest.java
>
>
> This is an incredibly unscalable API (not picking on Eric here, just
> commenting on the interface). What if you have 100k users in
> your system? I
> recommend deprecating this method and changing all
> implementations to throw
> some sort of exception. In the very unlikely case that
> someone does need this
> method (you're paginating your lists of users, right? ;), a
> method which
> returns an Iterator can be used. In the past I've also
> hacked up solutions
> which cursor over a result set, but this requires that you
> explicitly invoke
> some sort of cleanup method when done with the List, or risk leaking
> connections to your data source.
>
> Thoughts?
>
> epugh@apache.org wrote:
> > epugh 2004/01/10 08:15:17
> >
> > Modified: security/nt/src/java/org/apache/fulcrum/security/nt
> > NTUserManagerImpl.java
> >
> security/nt/src/test/org/apache/fulcrum/security/nt/dynamic
> > NTUserManagerTest.java
> > Log:
> > add getAllUsers
> >
> > Revision Changes Path
> > 1.2 +13 -1
> jakarta-turbine-fulcrum/security/nt/src/java/org/apache/fulcru
> m/security/nt/NTUserManagerImpl.java
> >
> > Index: NTUserManagerImpl.java
> >
> ===================================================================
> > RCS file:
> /home/cvs/jakarta-turbine-fulcrum/security/nt/src/java/org/apa
> che/fulcrum/security/nt/NTUserManagerImpl.java,v
> > retrieving revision 1.1
> > retrieving revision 1.2
> > diff -u -r1.1 -r1.2
> > --- NTUserManagerImpl.java 5 Dec 2003 23:22:18
> -0000 1.1
> > +++ NTUserManagerImpl.java 10 Jan 2004 16:15:17
> -0000 1.2
> > @@ -65,6 +65,7 @@
> > import org.apache.fulcrum.security.util.EntityExistsException;
> > import
> org.apache.fulcrum.security.util.PasswordMismatchException;
> > import org.apache.fulcrum.security.util.UnknownEntityException;
> > +import org.apache.fulcrum.security.util.UserSet;
> >
> > import com.tagish.auth.win32.NTSystem;
> > /**
> > @@ -270,5 +271,16 @@
> > {
> > throw new RuntimeException("Not supported by NT
> User Manager");
> > }
> > +
> > + /**
> > + * Retrieves all users defined in the system.
> > + *
> > + * @return the names of all users defined in the system.
> > + * @throws DataBackendException if there was an error
> accessing the data backend.
> > + */
> > + public UserSet getAllUsers() throws DataBackendException
> > + {
> > + throw new RuntimeException("Not supported by NT
> User Manager");
> > + }
> >
> > }
> >
> >
> >
> > 1.2 +17 -2
> jakarta-turbine-fulcrum/security/nt/src/test/org/apache/fulcru
> m/security/nt/dynamic/NTUserManagerTest.java
> >
> > Index: NTUserManagerTest.java
> >
> ===================================================================
> > RCS file:
> /home/cvs/jakarta-turbine-fulcrum/security/nt/src/test/org/apa
> che/fulcrum/security/nt/dynamic/NTUserManagerTest.java,v
> > retrieving revision 1.1
> > retrieving revision 1.2
> > diff -u -r1.1 -r1.2
> > --- NTUserManagerTest.java 5 Dec 2003 23:22:18
> -0000 1.1
> > +++ NTUserManagerTest.java 10 Jan 2004 16:15:17
> -0000 1.2
> > @@ -199,7 +199,7 @@
> > /*
> > * Class to test for User retrieve(String, String)
> > */
> > - public void testRetrieveStringString() throws Exception
> > + public void testGetUserStringString() throws Exception
> > {
> > try
> > {
> > @@ -239,10 +239,25 @@
> > }
> > }
> > /** ******* ALL BELOW HERE THROW RUNTIME EXCEPTIONS
> ******** */
> > + /*
> > + * Class to test for User retrieve(String, String)
> > + */
> > + public void testGetAllUsers() throws Exception
> > + {
> > + try
> > + {
> > + userManager.getAllUsers();
> > + fail("Should throw runtime exception");
> > + }
> > + catch (RuntimeException re)
> > + {
> > + assertTrue(re.getMessage().equals(ERROR_MSG));
> > + }
> > + }
> > /*
> > * Class to test for User retrieve(String)
> > */
> > - public void testRetrieveString() throws Exception
> > + public void testGetUserString() throws Exception
> > {
> > try
> > {
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org