CODEWITHSUNDEEP
javadesign-patternsspring-security
sare3th
sare3th

Reputation: 828

avoiding many if statements, code improvement

I have oracle database that I can't change. In this db I have a user table with authority fields.

CREATE TABLE USER (
    ID NUMBER(38,0), 
    LOGIN VARCHAR2(6 BYTE) NOT NULL ENABLE, 
    PASSWORD VARCHAR2(200 BYTE) NOT NULL ENABLE, 

    BOOK_READ NUMBER(1,0), 
    BOOK_UPD NUMBER(1,0), 
    R_READ NUMBER(1,0), 
    R_UPD NUMBER(1,0), 
    ... 
    ...
)

For example:

book_read = 1;  - means that user has authority 
book_upd  = 0;  - means that user he does not
report_read = 1;

Right now I am checking like this:

List<GrantedAuthority> authorities = new ArrayList<GrantedAuthority>();
if (user.getBook_read() == 1) {
    authorities.add(new SimpleGrantedAuthority("ROLE_BOOK_READ"));
}
if (user.getBook_upd() == 1) {
    authorities.add(new SimpleGrantedAuthority("ROLE_BOOK_UPD"));
}
if (user.getReport_read() == 1) {
    authorities.add(new SimpleGrantedAuthority("ROLE_REPORT_READ"));
}

I have a lot of Authorities (about 20, they all have a separate column in the table). Сan someone suggest how I can improve the code?

Upvotes: 0

Views: 101

Answers (2)

Andreas
Andreas

Reputation: 159135

It seems that right now you have 20 authority columns in the database, and a User Model object with 20 fields, 20 getters, and 20 setter, and you need 20 if statements to build the List<GrantedAuthority>.

To simplify that, I'd suggest starting with the Model object and the DAO.

For improved flexibility, create an enum with the 20 authorities, and explicitly name the database column name and the Role name, e.g.

public enum User_Authority {
    BOOK_READ("BOOK_READ", "ROLE_BOOK_READ"),
    BOOK_UPD ("BOOK_UPD" , "ROLE_BOOK_UPD"),
    // more enums

    // fields here

    private User_Authority(String columnName, String roleName) {
        // assign to fields here
    }

    // getters here
}

Your User Model object can then use an EnumSet<User_Authority> to store which authorities are granted to the user.

Your Spring JDBC Template can then use a RowMapper with logic like this:

public final class UserRowMapper implements RowMapper<User> {
    @Override
    public User mapRow(ResultSet rs, int rowNum) {
        User user = new User();
        // get other columns here
        EnumSet<User_Authority> granted = EnumSet.noneOf(User_Authority.class);
        for (User_Authority auth : User_Authority.values())
            if (rs.getInt(auth.getColumnName()) == 1)
                granted.add(auth);
        user.setAuthorities(granted);
        return user;
    }
}

The code to build the List<GrantedAuthority> authorities is now as simple as this:

List<GrantedAuthority> authorities = new ArrayList<>();
for (User_Authority auth : user.getAuthorities())
    authorities.add(new SimpleGrantedAuthority(auth.getRoleName()));

As you can see, you now only have one place with 20 "repeats": The enum values.

If you need to add more later on, you of course add new column to user table and then you only need to add two items to the code:

  • Add column to SELECT clause of SQL statement given to Spring JDBC Template.
  • Add new enum value.

That is quite a simplification/improvement to the code, right?

Upvotes: 1

Alex
Alex

Reputation: 803

How about creating a List of class storing the relationship between the method and SimpleGrantedAuthority 

public class RoleChecker
{
    Function<User, Integer> roleCheckFunction;
    SimpleGrantedAuthority theAuthority;

    public RoleChecker(Function<User, Integer> roleCheckFunction, SimpleGrantedAuthority theAuthority)
    {}
}

and execute the checking using loop.

List<RoleChecker> roles = new ArrayList<>();
roles.add(new RoleChecker(User::getBook_read, new SimpleGrantedAuthority("ROLE_BOOK_READ")));
roles.add(new RoleChecker(User::getBook_upd, new SimpleGrantedAuthority("ROLE_BOOK_UPD")));
// ....
for (RoleChecker check : roles)
{
    if (check.roleCheckFunction.apply(user) == 1)
        authorities.add(check.theAuthority);
}

Upvotes: 0

Related Questions