Suggestion to improve `LoginUserFinder` logic

[moonmanbu]

目前 aaa 框架中, 使用 Entity#id 作为 user.key 时, 直接使用 id.equels() 方式判断并获取 idValue, 这种方式存在如下问题:

1. 当 Entity#id 字段用其他名称命名, 则该方法会失效, 例如实体为 User, id 字段命名为 userId时, principal.getProperty("id") 返回 idValue值为null

2. 如果把 id 字段作为通用的 username, 即 propName:propValue, 因为数据类型的原因, 无法获取 principal 实例 (aaa 框架中 propName:propValue 的数据类型默认为 String 类型)

建议:

1. 通过反射 @Id 判断是否为 id

2. 通用 propName:propValue 方式加入 prop 数据类型的获取和转换, 兼容非 String 类型 prop 作为 user.key 的情况

另外, user.key 应具有唯一性, 可以考虑将 id 为 user.key 的情况作为通用情况处理, 在加入数据类型处理的前提下, 不将 id 作为特例情况单独判断, 统一使用 dao.findOneBy(key, value) 获取 User, 则配置项aaa.user.key 可以取消, 统一采用 principal.getName() 返回 "propName:propValue" 的方式来获取USER,以此降低 aaa 框架使用的复杂度!

[moonmanbu]

想象中的实现方式, 可以进一步优化 
act.aaa.util.LoginUserFinder#get()

...
    @Override
    public Object get() {
        AAAContext aaaContext = AAA.context();
        if (null != aaaContext) {
            Principal principal = aaaContext.getCurrentPrincipal();
            if (null != principal) {
                if (principal instanceof UserBase) {
                    return principal;
                }
                String fieldName = this.querySpec;
                AAAPlugin plugin = Act.getInstance(AAAPlugin.class);
                List<Field> idFields = $.fieldsOf(dao.modelType(), field -> field.getAnnotation(Id.class) != null);
                if (C.notEmpty(idFields)) {
                    Field idField = idFields.get(0);
                    if (S.eq(idField.getName(), fieldName)) {
                        Object idValue = $.getProperty(principal, fieldName);// $.convert($.getFieldValue(principal, idField)).to(dao.idType());
                        String cacheKey = S.string(idValue);
                        Object cached = plugin.cachedUser(cacheKey);
                        if (null == cached) {
                            cached = dao.findById(idValue);
                            if (null != cached) {
                                plugin.cacheUser(cacheKey, cached);
                            }
                        }
                        return cached;
                    }
                }
                fieldName = principal.getName();
                int pos = fieldName.indexOf(':');
                Object fieldValue;
                if (pos > 0) {
                    fieldName = fieldName.substring(0, pos);
                    fieldValue = fieldName.substring(pos + 1);
                } else {
                    fieldValue = $.getProperty(dao.modelType(), fieldName);
                }
                String cacheKey = S.concat(fieldName, "::", fieldValue);
                Object cached = plugin.cachedUser(cacheKey);
                if (null == cached) {
                    Field field = $.fieldOf(dao.modelType(), fieldName);
                    if (field != null) {
                        fieldValue = $.convert(fieldValue).to(field.getType());
                        cached = dao.findOneBy(fieldName, fieldValue);
                        if (null != cached) {
                            plugin.cacheUser(cacheKey, cached);
                        }
                    }
                }
                return cached;
            }
        }
        return null;
    }

...

[greenlaw110]

@moonmanbu

Changes ActAAAService.buildPrincipalFrom(USER_TYPE user):

Previously:

principal.setProperty("id", S.string($.getProperty(user, "id")));

Now:

 String id = S.string($.getProperty(user, "id"));
 if (S.isBlank(id)) {
     // let's try @Id annotation
     List<Field> fields = $.fieldsOf(user.getClass(), new Lang.Predicate<Field>() {
         @Override
         public boolean test(Field field) {
             return field.getAnnotation(Id.class) != null;
         }
     });
     E.unexpectedIf(fields.isEmpty(), "Unable to determine 'id' of user: %s", user);
     id = $.getFieldValue(user, fields.get(0));
 }
 principal.setProperty("id", id);

Changes to LoginUserFinder.initialized:

Previously:

Class rawType = spec.rawType();
dao = app.dbServiceManager().dao(rawType);

Now:

userType = spec.rawType(); // cache user model class to class field
userKeyType = $.fieldOf(userType, AAAConfig.user.key.get()).getType(); // cache user key class to class field
dao = app.dbServiceManager().dao(userType);

Changes to LoginUserFinder.get() - part one

Previously:

if (principal instanceof UserBase) {
    return principal;
}

Now:

if (userType.isInstance(principal)) {
    return principal;
}

Changes to LoginUserFinder.get() - part two

Previously:

cached = dao.findOneBy(querySpec, name);

Now:

Object val = userKeyType == String.class ? name : $.convert(name).to(userKeyType);
cached = dao.findOneBy(querySpec, val);

Refer: https://github.com/actframework/act-aaa-plugin/commit/9c6e9473e11c7f4026e2772b31fa0656a20ec820

Note the change is now availabe in act-aaa-1.8.1-SNAPSHOT.

[moonmanbu]

userKeyType = $.fieldOf(userType, AAAConfig.user.key.get()).getType();没必要定于为全局字段和在initialized()中初始化

    @Override
    protected void initialized() {
        App app = App.instance();

        userType = spec.rawType();
        userKeyType = $.fieldOf(userType, AAAConfig.user.key.get()).getType();
        dao = app.dbServiceManager().dao(userType);

        querySpec = S.string(options.get(KEY_USER_KEY));
        if (S.blank(querySpec)) {
            querySpec = AAAConfig.user.key.get();
        }
    }

应当放到如下位置, 并且将查询的field name修改为querySpec

                String name = principal.getName();
                int pos = name.indexOf(':');
                if (pos > 0) {
                    querySpec = name.substring(0, pos);
                    name = name.substring(pos + 1);
                }

Class<?> userKeyType = $.fieldOf(userType, querySpec).getType();

                String cacheKey = S.concat(querySpec, "::", name);
                Object cached = plugin.cachedUser(cacheKey);
                if (null == cached) {
                    Object val = userKeyType == String.class ? name : $.convert(name).to(userKeyType);
                    cached = dao.findOneBy(querySpec, val);
                    if (null != cached) {
                        plugin.cacheUser(cacheKey, cached);
                    }
                }