{"action":"created","issue":{"url":"https://api.github.com/repos/decebals/pippo/issues/254","repository_url":"https://api.github.com/repos/decebals/pippo","labels_url":"https://api.github.com/repos/decebals/pippo/issues/254/labels{/name}","comments_url":"https://api.github.com/repos/decebals/pippo/issues/254/comments","events_url":"https://api.github.com/repos/decebals/pippo/issues/254/events","html_url":"https://github.com/decebals/pippo/issues/254","id":134537851,"number":254,"title":"Ideas for Route improvement","user":{"login":"decebals","id":1248311,"avatar_url":"https://avatars.githubusercontent.com/u/1248311?v=3","gravatar_id":"","url":"https://api.github.com/users/decebals","html_url":"https://github.com/decebals","followers_url":"https://api.github.com/users/decebals/followers","following_url":"https://api.github.com/users/decebals/following{/other_user}","gists_url":"https://api.github.com/users/decebals/gists{/gist_id}","starred_url":"https://api.github.com/users/decebals/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/decebals/subscriptions","organizations_url":"https://api.github.com/users/decebals/orgs","repos_url":"https://api.github.com/users/decebals/repos","events_url":"https://api.github.com/users/decebals/events{/privacy}","received_events_url":"https://api.github.com/users/decebals/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/decebals/pippo/labels/enhancement","name":"enhancement","color":"84b6eb"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-02-18T10:22:15Z","updated_at":"2016-02-18T11:04:27Z","closed_at":null,"body":"I want to move the discussion started by https://github.com/decebals/pippo/issues/253#issuecomment-185409221 here.\r\n\r\nI will copy/paste the content of comment\r\n\r\n> Yeah, I think that is good.\r\n> \r\n> I also think we should consider doing deferred Route addition.\r\n> \r\n> 1. Observe the return type of any of [these methods](https://github.com/gitblit/fathom/blob/master/fathom-rest/src/main/java/fathom/rest/RoutesModule.java#L207).\r\n> 2. Which is [this](https://github.com/gitblit/fathom/blob/master/fathom-rest/src/main/java/fathom/rest/RouteRegistration.java#L37) class.\r\n> 3. And the asynchronous deferred Route [compilation/addition](https://github.com/gitblit/fathom/blob/master/fathom-rest/src/main/java/fathom/rest/RoutesModule.java#L315\r\n> ) step.\r\n> \r\n> The basic idea is to allow a route to be configured declaratively before being added to the Router.  You seem to be heading that direction.\r\n> \r\n\r\nI want to say that from my review of RouteGroup implementation made by @ScienJus I observed that we must add the routes (DefaultRouter) in two steps:\r\n1. `collect` all routes in Router (we can create a separate entity RouteCollection but for now Router is OK as home)\r\n```java\r\nGET(\"/login\", ...); // <-- application route\r\n\r\n// OR\r\naddRoute(...);\r\n\r\n// OR\r\nRouteGroup group = new RouteGroup(\"/useres\");\r\ngroup.GET(\"/{id}\", ...); // <- group route\r\naddRouteGroup(group);\r\n```\r\n2. `compile/process` added routes; in this point we know all details about collected routes (groups, ...) and we can do some preparation.\r\n\r\nTo implement two steps routes adding we need a way to call `Router.compileRoutes` method in the process of Application initialization.\r\n```java\r\n public final void init() {\r\n        initializers.addAll(ServiceLocator.locateAll(Initializer.class));\r\n        for (Initializer initializer : initializers) {\r\n            log.debug(\"Initializing '{}'\", initializer.getClass().getName());\r\n            try {\r\n                initializer.init(this);\r\n            } catch (Exception e) {\r\n                log.error(\"Failed to initialize '{}'\", initializer.getClass().getName(), e);\r\n            }\r\n        }\r\n\r\n        onInit();\r\n\r\n        getRouter().compileRoutes(); // <-- HERE\r\n    }\r\n```\r\n\r\nWith this construction we can move `Route.setGroupUriPattern` in `compileRoutes`. Now it's safe, and also we can add some validation (see https://github.com/decebals/pippo/issues/248 for example) and take some decisions.\r\n\r\nNow, my inline feedback for @gitblit :\r\n> 1. Observe the return type of any of [these methods](https://github.com/gitblit/fathom/blob/master/fathom-rest/src/main/java/fathom/rest/RoutesModule.java#L207).\r\n> 2. Which is [this](https://github.com/gitblit/fathom/blob/master/fathom-rest/src/main/java/fathom/rest/RouteRegistration.java#L37) class.\r\n\r\nI prefer to not introduce a new entity `RouteRegistration` in project for this goal and to use `Route` for this. The `Route` class is for metadata. If we want a new entity, we can add `CompiledRoute` (it's a possible name) in this scenario we have `Route->Router.compileRoutes()->CompiledRoute`. We can discuss more about the content of `CompiledRoute` later if we agree that it's a good idea.\r\n\r\n> 3. And the asynchronous deferred Route [compilation/addition](https://github.com/gitblit/fathom/blob/master/fathom-rest/src/main/java/fathom/rest/RoutesModule.java#L315) step.\r\n\r\nHere, I see some improvements in the future :smile: My idea is to split the code and to add a functional `RouteCompiler`s (a possible name, or RouteProcessor). In this case in `Router` you can add programmatically (or via `ServiceLoader`)  a list with route compilers (`setRouteCompilers`) and on `Router.compileRouters` we iterate through this collection and call `compileRoute`. It's just an idea and I don't have the big picture yet.\r\n"},"comment":{"url":"https://api.github.com/repos/decebals/pippo/issues/comments/185659336","html_url":"https://github.com/decebals/pippo/issues/254#issuecomment-185659336","issue_url":"https://api.github.com/repos/decebals/pippo/issues/254","id":185659336,"user":{"login":"ScienJus","id":8350653,"avatar_url":"https://avatars.githubusercontent.com/u/8350653?v=3","gravatar_id":"","url":"https://api.github.com/users/ScienJus","html_url":"https://github.com/ScienJus","followers_url":"https://api.github.com/users/ScienJus/followers","following_url":"https://api.github.com/users/ScienJus/following{/other_user}","gists_url":"https://api.github.com/users/ScienJus/gists{/gist_id}","starred_url":"https://api.github.com/users/ScienJus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ScienJus/subscriptions","organizations_url":"https://api.github.com/users/ScienJus/orgs","repos_url":"https://api.github.com/users/ScienJus/repos","events_url":"https://api.github.com/users/ScienJus/events{/privacy}","received_events_url":"https://api.github.com/users/ScienJus/received_events","type":"User","site_admin":false},"created_at":"2016-02-18T11:04:27Z","updated_at":"2016-02-18T11:04:27Z","body":"it's great, `Route` and `RouteGroup` should be the tools to help people creating routes, then the real route will be only responsible store the metadata and used in url mapping."}}