Javalin: CrudHandler doesn't work with nested resources

[mkpaz]

Describe your issue
CrudHandler only works with paths that follows format resourceBase/resourceId due to hard-coded array indices. Using smth like this crud("foo/bar/baz/:id", ...) leads to inconsistent behavior. Please consider either determine resourceId by using smth like lastIndexOf(), or throwing IllegalArgumentException if specified path arg doesn't fit method expectations. Sure this can be avoided if one just use path() one more time, but such API doesn't look good:

routes()
    path()
        path()
            path()
                crud()

path() method itself supports long nested resources, it will be nice if crud() support them too.

[tipsy]

Sure, would you like to submit a PR?

[mkpaz]

I'd like to, but I can't right now since I'm working on my Javalin-based SPA full-time. I've written a "fixed" version, which serves as a small work-around pretty well due ApiBuilder static nature. This actually incredible 'cos it means I can implement my own generic handler groups.

private static void bulkCrud(@NotNull String path, @NotNull CrudHandler crudHandler) {
    String normalizedPath = path.startsWith("/") ? path : "/" + path;
    String separator = "/:";
    int idx = normalizedPath.lastIndexOf(separator);
    if (idx < 0 || normalizedPath.startsWith(separator) || normalizedPath.length() == idx + separator.length()) {
        throw new IllegalArgumentException("Invalid path: '" + normalizedPath + "'. Either resource base or id missing.");
    }

    final String resourceBase = normalizedPath.substring(0, idx);
    final String resourceId = normalizedPath.substring(idx + 1);

    get(normalizedPath, ctx -> crudHandler.getOne(ctx, ctx.pathParam(resourceId)));
    get(resourceBase, crudHandler::getAll);
    post(resourceBase, crudHandler::create);
    patch(normalizedPath, ctx -> crudHandler.update(ctx, ctx.pathParam(resourceId)));
    delete(normalizedPath, ctx -> crudHandler.delete(ctx, ctx.pathParam(resourceId)));
}

assertThat(split("foo/:id")).containsOnly(array("/foo", ":id"));
assertThat(split("foo/bar/:id")).containsOnly(array("/foo/bar", ":id"));
assertThat(split("foo/bar/baz/:id")).containsOnly(array("/foo/bar/baz", ":id"));
assertThat(split("/foo/bar/baz/:id")).containsOnly(array("/foo/bar/baz", ":id"));
assertThat(split("/foo/*/:id")).containsOnly(array("/foo/*", ":id"));

assertThatThrownBy(() -> split("/foo/:")).isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> split("/foo")).isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> split("foo/")).isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> split("/:")).isInstanceOf(IllegalArgumentException.class);
assertThatThrownBy(() -> split(":id")).isInstanceOf(IllegalArgumentException.class);

[tipsy]

Happy you found a solution. I'll leave the issue open for anyone who wants to work on it!

[tobiasdrechsel]

I submitted a pull request. I'm happy about your feedback