{"action":"opened","issue":{"url":"https://api.github.com/repos/lagom/online-auction-java/issues/120","repository_url":"https://api.github.com/repos/lagom/online-auction-java","labels_url":"https://api.github.com/repos/lagom/online-auction-java/issues/120/labels{/name}","comments_url":"https://api.github.com/repos/lagom/online-auction-java/issues/120/comments","events_url":"https://api.github.com/repos/lagom/online-auction-java/issues/120/events","html_url":"https://github.com/lagom/online-auction-java/issues/120","id":239681385,"number":120,"title":"Replace ItemService.getUsers","user":{"login":"TimMoore","id":44385,"avatar_url":"https://avatars3.githubusercontent.com/u/44385?v=3","gravatar_id":"","url":"https://api.github.com/users/TimMoore","html_url":"https://github.com/TimMoore","followers_url":"https://api.github.com/users/TimMoore/followers","following_url":"https://api.github.com/users/TimMoore/following{/other_user}","gists_url":"https://api.github.com/users/TimMoore/gists{/gist_id}","starred_url":"https://api.github.com/users/TimMoore/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TimMoore/subscriptions","organizations_url":"https://api.github.com/users/TimMoore/orgs","repos_url":"https://api.github.com/users/TimMoore/repos","events_url":"https://api.github.com/users/TimMoore/events{/privacy}","received_events_url":"https://api.github.com/users/TimMoore/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-06-30T05:32:57Z","updated_at":"2017-06-30T05:32:57Z","closed_at":null,"body":"Right now, the `getUsers` service implementation is not ideal:\r\n\r\n```java\r\n        // Note this should never make production....\r\n        return req -> currentIdsQuery.currentPersistenceIds()\r\n                .filter(id -> id.startsWith(\"PUserEntity\"))\r\n                .mapAsync(4, id -> entityRef(id.substring(11)).ask(PUserCommand.GetPUser.INSTANCE))\r\n                .filter(Optional::isPresent)\r\n                .map(user -> Mappers.toApi(user.get()))\r\n                .runWith(Sink.seq(), mat)\r\n                .thenApply(TreePVector::from);\r\n```\r\n\r\nAs you can tell from the comment, this code is a quick hack... not something that should be considered a good example of writing a Lagom service.\r\n\r\nIt has a few problems:\r\n\r\n- It is slow. This requires loading each user entity into memory to query it for its details. In a multi-node cluster, this will require multiple calls across the network.\r\n- It is fragile. It assumes too much about the format of the persistence ID string. See #115 for an example of the problems this causes\r\n- It does not scale well. In a realistic system with a large number of users, this will return a very long list with no pagination.\r\n- It is written in a very functional style that may be unfamiliar to many Java developers.\r\n\r\nThis service call is used by the web-gateway application to load a list of all users in the `Nav`. This is then used in turn for different purposes:\r\n\r\n1. To create a list of users in the navigation bar, which allows you to log in as different users\r\n2. To look up user details given a user ID, as an optimization of calling `userService.getUser` for each ID, since all of the user information was already loaded and cached locally for the navigation bar\r\n\r\nThese can be replaced by different means.\r\n\r\n## Navigation bar\r\n\r\nIn the navigation bar, it might be better to show a bounded list of recently-created users. For example, we could put the ten most recently-created users in the list, rather than all of the users in the system. For most demo purposes, it will be exactly the same, since most people won't create that many users. However, if we decide to extend the demo to create users automatically (see #110 for example) then we might want to include a script that creates many more than ten users.\r\n\r\nThis is a perfect case for a read-side table in Lagom. This would look similar to the existing read-side processor in the item service and the one currently under review in the transaction service (#106). It would maintain a table of users and their creation date that could be queried by the service implementation in a new `getRecentUsers` service call. We would need to extend the `PUserCreated` event to carry a `createdAt` timestamp.\r\n\r\n## User lookups\r\n\r\nI think that using the navigation to look up user details, rather than making calls to `userService.getUser(id)`, could be considered a premature optimization. I suggest we undo that and stick with making the calls for now.\r\n\r\nIf we want to demonstrate optimizations in the future, we have a few options:\r\n\r\n1. We could keep calling `getUser(id)` but maintain a local cache, so it doesn't have to be called as often\r\n2. We could replace the `getUsers()` call that returns all users with a call that takes a list of user IDs to return\r\n3. We could publish user events to Kafka, and have the `web-gateway` listen to these events and maintain its own internal copy of user display names"}}
{"actor":{"display_login":"TimMoore"}}