Skip to content

Commit

Permalink
Refactored method names so they're more sensible
Browse files Browse the repository at this point in the history
  • Loading branch information
OliviaYtterbrink authored and henriknyman committed Jun 16, 2016
1 parent f295355 commit a32a7e2
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 85 deletions.
Expand Up @@ -103,7 +103,7 @@ public AuthSubject login( Map<String,Object> authToken ) throws InvalidAuthToken
String username = AuthToken.safeCast( AuthToken.PRINCIPAL, authToken );
String password = AuthToken.safeCast( AuthToken.CREDENTIALS, authToken );

User user = users.findByName( username );
User user = users.getUserByName( username );
AuthenticationResult result = AuthenticationResult.FAILURE;
if ( user != null )
{
Expand Down Expand Up @@ -135,15 +135,15 @@ public User newUser( String username, String initialPassword, boolean requirePas
public boolean deleteUser( String username ) throws IOException
{
assertAuthEnabled();
User user = users.findByName( username );
User user = users.getUserByName( username );
return user != null && users.delete( user );
}

@Override
public User getUser( String username )
{
assertAuthEnabled();
return users.findByName( username );
return users.getUserByName( username );
}

public void setPassword( AuthSubject authSubject, String username, String password ) throws IOException,
Expand All @@ -164,7 +164,7 @@ public void setUserPassword( String username, String password ) throws IOExcepti
IllegalCredentialsException
{
assertAuthEnabled();
User existingUser = users.findByName( username );
User existingUser = users.getUserByName( username );
if ( existingUser == null )
{
throw new IllegalCredentialsException( "User " + username + " does not exist" );
Expand Down Expand Up @@ -201,7 +201,7 @@ protected void assertAuthEnabled()

private void assertValidName( String name )
{
if ( !users.isValidName( name ) )
if ( !users.isValidUsername( name ) )
{
throw new IllegalArgumentException( "User name contains illegal characters. Please use simple ascii characters and numbers." );
}
Expand Down
Expand Up @@ -27,6 +27,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import org.neo4j.kernel.lifecycle.LifecycleAdapter;
Expand Down Expand Up @@ -56,16 +57,18 @@ public class FileUserRepository extends LifecycleAdapter implements UserReposito

private final UserSerialization serialization = new UserSerialization();

private final Pattern usernamePattern = Pattern.compile( "^[a-zA-Z0-9_]+$" );

public FileUserRepository( Path file, LogProvider logProvider )
{
this.authFile = file.toAbsolutePath();
this.log = logProvider.getLog( getClass() );
}

@Override
public User findByName( String name )
public User getUserByName( String username )
{
return usersByName.get( name );
return usersByName.get( username );
}

@Override
Expand All @@ -80,7 +83,7 @@ public void start() throws Throwable
@Override
public void create( User user ) throws IllegalArgumentException, IOException
{
if ( !isValidName( user.name() ) )
if ( !isValidUsername( user.name() ) )
{
throw new IllegalArgumentException( "'" + user.name() + "' is not a valid user name." );
}
Expand Down Expand Up @@ -181,9 +184,9 @@ public int numberOfUsers()
}

@Override
public boolean isValidName( String name )
public boolean isValidUsername( String username )
{
return name.matches( "^[a-zA-Z0-9_]+$" );
return usernamePattern.matcher( username ).matches();
}

@Override
Expand Down
Expand Up @@ -31,7 +31,7 @@
*/
public interface UserRepository extends Lifecycle
{
User findByName( String name );
User getUserByName( String username );

/**
* Create a user, given that the users token is unique.
Expand All @@ -58,7 +58,7 @@ public interface UserRepository extends Lifecycle
int numberOfUsers();

/** Utility for API consumers to tell if #create() will accept a given username */
boolean isValidName( String name );
boolean isValidUsername( String username );

Set<String> getAllUsernames();
}
Expand Up @@ -49,7 +49,7 @@ public void shouldCreateDefaultUserIfNoneExist() throws Throwable
manager.start();

// Then
final User user = users.findByName( "neo4j" );
final User user = users.getUserByName( "neo4j" );
assertNotNull( user );
assertTrue( user.credentials().matchesPassword( "neo4j" ) );
assertTrue( user.passwordChangeRequired() );
Expand Down Expand Up @@ -147,7 +147,7 @@ public void shouldCreateUser() throws Throwable
manager.newUser( "foo", "bar", true );

// Then
User user = users.findByName( "foo" );
User user = users.getUserByName( "foo" );
assertNotNull( user );
assertTrue( user.passwordChangeRequired() );
assertTrue( user.credentials().matchesPassword( "bar" ) );
Expand All @@ -168,7 +168,7 @@ public void shouldDeleteUser() throws Throwable
manager.deleteUser( "jake" );

// Then
assertNull( users.findByName( "jake" ) );
assertNull( users.getUserByName( "jake" ) );
}

@Test
Expand All @@ -186,7 +186,7 @@ public void shouldDeleteUnknownUser() throws Throwable
manager.deleteUser( "unknown" );

// Then
assertNotNull( users.findByName( "jake" ) );
assertNotNull( users.getUserByName( "jake" ) );
}

@Test
Expand All @@ -205,7 +205,7 @@ public void shouldSetPassword() throws Throwable
// Then
User user = manager.getUser( "jake" );
assertTrue( user.credentials().matchesPassword( "hello, world!" ) );
assertThat( users.findByName( "jake" ), equalTo( user ) );
assertThat( users.getUserByName( "jake" ), equalTo( user ) );
}

@Test
Expand Down
Expand Up @@ -87,7 +87,7 @@ public void shouldStoreAndRetriveUsersByName() throws Exception
users.create( user );

// When
User result = users.findByName( user.name() );
User result = users.getUserByName( user.name() );

// Then
assertThat( result, equalTo( user ) );
Expand All @@ -105,7 +105,7 @@ public void shouldPersistUsers() throws Throwable
users.start();

// When
User resultByName = users.findByName( user.name() );
User resultByName = users.getUserByName( user.name() );

// Then
assertThat( resultByName, equalTo( user ) );
Expand All @@ -123,7 +123,7 @@ public void shouldNotFindUserAfterDelete() throws Throwable
users.delete( user );

// Then
assertThat( users.findByName( user.name() ), nullValue() );
assertThat( users.getUserByName( user.name() ), nullValue() );
}

@Test
Expand All @@ -133,14 +133,14 @@ public void shouldNotAllowComplexNames() throws Exception
FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );

// When
assertTrue( users.isValidName( "neo4j" ) );
assertTrue( users.isValidName( "johnosbourne" ) );
assertTrue( users.isValidName( "john_osbourne" ) );

assertFalse( users.isValidName( ":" ) );
assertFalse( users.isValidName( "" ) );
assertFalse( users.isValidName( "john osbourne" ) );
assertFalse( users.isValidName( "john:osbourne" ) );
assertTrue( users.isValidUsername( "neo4j" ) );
assertTrue( users.isValidUsername( "johnosbourne" ) );
assertTrue( users.isValidUsername( "john_osbourne" ) );

assertFalse( users.isValidUsername( ":" ) );
assertFalse( users.isValidUsername( "" ) );
assertFalse( users.isValidUsername( "john osbourne" ) );
assertFalse( users.isValidUsername( "john:osbourne" ) );
}

@Test
Expand Down Expand Up @@ -209,7 +209,7 @@ public void shouldThrowIfUpdateChangesName() throws Throwable
// Then continue
}

assertThat( users.findByName( user.name() ), equalTo( user ) );
assertThat( users.getUserByName( user.name() ), equalTo( user ) );
}

@Test
Expand Down
Expand Up @@ -33,9 +33,9 @@ public class InMemoryUserRepository extends LifecycleAdapter implements UserRepo
private final ConcurrentHashMap<String, User> users = new ConcurrentHashMap<>();

@Override
public User findByName( String name )
public User getUserByName( String username )
{
return users.get( name );
return users.get( username );
}

@Override
Expand Down Expand Up @@ -101,7 +101,7 @@ public int numberOfUsers()
}

@Override
public boolean isValidName( String name )
public boolean isValidUsername( String username )
{
// This repo can store any name
return true;
Expand Down
Expand Up @@ -45,13 +45,13 @@ public abstract class AbstractRoleRepository extends LifecycleAdapter implements
protected volatile List<RoleRecord> roles = new ArrayList<>();

@Override
public RoleRecord findByName( String name )
public RoleRecord getRoleByName( String roleName )
{
return rolesByName.get( name );
return rolesByName.get( roleName );
}

@Override
public Set<String> findRoleNamesByUsername( String username )
public Set<String> getRoleNamesByUsername( String username )
{
Set<String> roleNames = rolesByUsername.get( username );
return roleNames != null ? roleNames : Collections.emptySet();
Expand All @@ -60,7 +60,7 @@ public Set<String> findRoleNamesByUsername( String username )
@Override
public void create( RoleRecord role ) throws IllegalArgumentException, IOException
{
if ( !isValidName( role.name() ) )
if ( !isValidRoleName( role.name() ) )
{
throw new IllegalArgumentException( "'" + role.name() + "' is not a valid role name." );
}
Expand Down
Expand Up @@ -23,6 +23,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.regex.Pattern;

import org.neo4j.logging.Log;
import org.neo4j.logging.LogProvider;
Expand All @@ -44,6 +45,8 @@ public class FileRoleRepository extends AbstractRoleRepository

private final RoleSerialization serialization = new RoleSerialization();

private final Pattern roleNamePattern = Pattern.compile( "^[a-zA-Z0-9_]+$" );

public FileRoleRepository( Path file, LogProvider logProvider )
{
this.roleFile = file.toAbsolutePath();
Expand All @@ -60,9 +63,9 @@ public void start() throws Throwable
}

@Override
public boolean isValidName( String name )
public boolean isValidRoleName( String roleName )
{
return name.matches( "^[a-zA-Z0-9_]+$" );
return roleNamePattern.matcher( roleName ).matches();
}

@Override
Expand Down

0 comments on commit a32a7e2

Please sign in to comment.