casdoor / casdoor-java-sdk

Java client SDK for Casdoor
https://github.com/casdoor/casdoor
Apache License 2.0
27 stars 43 forks source link

Is a more generic service required? #51

Closed towerhe closed 1 year ago

towerhe commented 1 year ago

I found a lot of duplicated codes in each services. IMO we can provide a more generic service ( Maybe named CasdoorCrudService ) to achive the common crud actions of entities.

public class CasdoorCrudService<T> extends CasdoorService {
    public CasdoorResponse<List<T>, Integer> query(String action, Map<String, String> queryParams) { }

    public T get(String action) { }

    public T get(String action, String id) {}

    public boolean create(String action, T t) {}

    public boolean update(String action, String id, T t) {}

    public boolean delete(String action, T t) {}
}
casbin-bot commented 1 year ago

@seriouszyx @ComradeProgrammer @Resulte

hsluoyz commented 1 year ago

@towerhe can add some base methods, refer to Go SDK: https://github.com/casdoor/casdoor-go-sdk/blob/master/casdoorsdk/base.go

/cc @WinterOfBit

towerhe commented 1 year ago

If we provide a generic service as follows:

public class CasdoorCrudService<T> extends CasdoorService {

    public CasdoorCrudService(CasdoorConfig casdoorConfig) {
        super(casdoorConfig);
    }

    public CasdoorResponse<List<T>, Integer> query(String action, @Nullable java.util.Map<String, String> query)
            throws IOException {
        return doGet(action, query, new TypeReference<CasdoorResponse<List<T>, Integer>>() {
        });
    }

    public T get(String action) throws IOException {
        return get(action, null);
    }

    public T get(String action, @Nullable String id) throws IOException {
        return doGet(action, Map.of("id", id), new TypeReference<CasdoorResponse<T, Object>>() {
        }).getData();
    }

    public boolean add(String action, T t) throws IOException {
        CasdoorResponse<String, Object> response = doPost(action, null, objectMapper.writeValueAsString(t),
                new TypeReference<CasdoorResponse<String, Object>>() {
                });

        return response.getStatus() == "ok";
    }

    public boolean update(String action, String id, T t) throws IOException {
        CasdoorResponse<String, Object> response = doPost(action, Map.of("id", id), objectMapper.writeValueAsString(t),
                new TypeReference<CasdoorResponse<String, Object>>() {
                });

        return response.getStatus() == "ok";
    }

    public boolean delete(String action, T t) throws IOException {
        CasdoorResponse<String, Object> response = doPost(action, null, objectMapper.writeValueAsString(t),
                new TypeReference<CasdoorResponse<String, Object>>() {
                });

        return response.getStatus() == "ok";
    }

}

When do crud actions of entities, we do not need to create a service for each entity.

CasdoorCrudService<CasdoorApplication> service = new CasdoorCrudService<>(config);
CasdoorResponse<List<CasdoorApplication>, Integer> response = service.query("get-applications", null);

CasdoorApplication application = service.get("get-application", "built-in/app-built-in");
service,update("update-application", "built-in/app-built-in", application);
service.delete("delete-application", application);
wintbiit commented 1 year ago

I think it's great to go further and make operations on applictions, permissions, users... etc more structured and simplified. These resources have characteristics suitable for curd operations, and it is suitable to use a curd service to operate them. The api design also seems suitable for this?

towerhe commented 1 year ago

I try to use Retrofit to re-implement all the services. Later, I will create a PR.

hsluoyz commented 1 year ago

@towerhe is that https://square.github.io/retrofit/? I don't think we need too fancy things for this small library. Being stable and easy to maintain will be the first priority.