Refactoring the logic related to the button to update active user profiles in ootb #946
Conversation
gitCarrot
force-pushed
the
dev-hjeon-1712
branch
2 times, most recently
from
e4b4203 to
ecc2581
Compare
| try { | ||
| mapFromJsonFile("ootb.active.user.profiles.json").forEach((key, userData) -> { | ||
| users.put(key, createUserFromMap(userData)); | ||
| }); | ||
| } catch (IOException e) { | ||
| logger.error("Error while initiating active user profiles from a json file: ", e); | ||
| } | ||
| } | ||
|
|
||
| private Map<String, Map<String, Object>> mapFromJsonFile(String filepath) throws IOException { | ||
| File file = new ClassPathResource(filepath).getFile(); | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
|
|
||
| // Map<ootb active profile type, Map<profile property, property's value>> | ||
| return objectMapper.readValue(file, new TypeReference<>() {}); | ||
| } | ||
|
|
||
| private User createUserFromMap(Map<String, Object> userData) { | ||
| String uid = (String) userData.get("uid"); | ||
| String uhUuid = (String) userData.get("uhUuid"); | ||
| List<String> roles = (List<String>) userData.get("authorities"); | ||
| Map<String, String> attributes = (Map<String, String>) userData.get("attributes"); | ||
|
|
||
| User.Builder builder = new User.Builder(uid) | ||
| .uhUuid(uhUuid) | ||
| .addAuthorities(roles); | ||
|
|
||
| attributes.forEach(builder::addAttribute); | ||
|
|
||
| return builder.build(); |
There was a problem hiding this comment.
I will say I do prefer your old code before the refactor because it was a lot simpler than the code that reads JSON files. You can make the call whether you think this refactor is needed or not.
I think your goal with this ticket was to put the default users inside a properties or JSON file. I would have gone with how you load data from the data.harness on the API side and put the JSON objects inside the application-ootb.properties or add a data.harness file to the UI side. Then use the JsonUtil class to load it. That way we can reuse the code for loading JSON files instead of adding new code here.
There was a problem hiding this comment.
Looks nice so far... but maybe using the Spring "@resource" annotation would be good here. e.g., @resource("${ootb.active.user.profiles.json}").
The casts after reading the file are a bit iffy. I think we have a helper method in the JsonUtil class to create objects. I think we should use that to create the User objects. If the helper method doesn't do exactly what you need, maybe you can create another helper method in JsonUtil.
There was a problem hiding this comment.
I will say I do prefer your old code before the refactor because it was a lot simpler than the code that reads JSON files. You can make the call whether you think this refactor is needed or not.
I think your goal with this ticket was to put the default users inside a properties or JSON file. I would have gone with how you load data from the
data.harnesson the API side and put the JSON objects inside theapplication-ootb.propertiesor add adata.harnessfile to the UI side. Then use the JsonUtil class to load it. That way we can reuse the code for loading JSON files instead of adding new code here.
I also actually prefer the code before the refactor. However, during the last meeting, it was mentioned that the ootb project should serve as a repository with a variety of purposes beyond just showing the groupings project without the grouper client connection. The goal is for developers to be able to create various scenarios, use and test them directly in the development of groupings. During that discussion, it was mentioned that using JSON would make it easier for developers to view and modify the data, and easily import it with an overrides file. Additionally, when we later create something like a QA tool, it would be synergistic to have it in JSON format. Initially, I considered loading the JSON file into the properties file like the existing data harness method and calling it with propertyLocator, but I thought it would be difficult for developers to modify the data later if we compact the JSON data into a single line. So instead of properties, I decided to create it as a separate JSON file.
gitCarrot
force-pushed
the
dev-hjeon-1712
branch
2 times, most recently
from
7f1ab37 to
1bbd987
Compare
gitCarrot
requested review from
fduckart and
JorWo
and removed request for
JorWo
|
Make sure to update the tests for User.java, JsonUtil.java, Realm.java, and add a set of tests for OotbActivePorfile.java, thanks! |
…files in ootb Implement loadAvailableProfiles in general.controller.js and getAvailableOotbActiveProfiles in groupingsService to call api in ui and store the available data in .availableProfiles Add ootb.active.user.profiles.json that contains multiple profiles information Apply ng-repeat with the values from scope.availableProfiles that contains the profiles from api call Add isOotb() function to check current spring boot active profile and apply to the menubar.html Add tests for loadAvailableProfiles and getAvailableOotbActiveProfiles in general.controller.test.js and groupings.service.test.js Enhance the Builder class functions in the User class (access folder) Refactor the file inputstream function in OotbActiveUserProfileService Implement getAvailableProfiles function test in the controller test, add @Mockbean OotbActiveUserProfileService and mock the map that contains the ootb active profiles Complete adding multiple ootb active profiles into the users map dynamically from json file with null safe and wrong path of file error Fix cadacy code style Fix general controller test to check gs.getAvailableOotbActiveProfiles Modify the functions to initiate default active user by mapping JSON data to OotbActiveProfile with JsonUtil.asList() function Modify the end-point to get available profiles from OotbActiveUserProfileService Redesign the specification of JSON data for mapping to OotbActiveProfile Change the strategy to set property of Json file and the way to inject the file in the service layer Modify the tests for controller and service with dynamic active profile from JSON data Modify raw type of exception Implement the tests for ootb multiple active profiles in UserTest, RealmTest, JsonUtilTest, and OotbActiveProfileTest
gitCarrot
force-pushed
the
dev-hjeon-1712
branch
from
1bbd987 to
d159065
Compare
Ticket Link
Ticket-1712
List of squashed commits
Implement loadAvailableProfiles in general.controller.js and getAvailableOotbActiveProfiles in groupingsService to call api in ui and store the available data in .availableProfiles
Add ootb.active.user.profiles.json that contains multiple profiles information
Apply ng-repeat with the values from scope.availableProfiles that contains the profiles from api call
Add isOotb() function to check current spring boot active profile and apply to the menubar.html
Add tests for loadAvailableProfiles and getAvailableOotbActiveProfiles in general.controller.test.js and groupings.service.test.js
Enhance the Builder class functions in the User class (access folder)
Refactor the file inputstream function in OotbActiveUserProfileService
Implement getAvailableProfiles function test in the controller test, add @Mockbean OotbActiveUserProfileService and mock the map that contains the ootb active profiles
Complete adding multiple ootb active profiles into the users map dynamically from json file with null safe and wrong path of file error
Fix codacy code style
Fix general controller test to check gs.getAvailableOotbActiveProfiles
Modify the functions to initiate default active user by mapping JSON data to OotbActiveProfile with JsonUtil.asList() function
Modify the end-point to get available profiles from OotbActiveUserProfileService
Redesign the specification of JSON data for mapping to OotbActiveProfile
Change the strategy to set property of Json file and the way to inject the file in the service layer
Modify the tests for controller and service with dynamic active profile from JSON data
Fix raw type of exception
Implement the tests for ootb multiple active profiles in UserTest, RealmTest, JsonUtilTest, and OotbActiveProfileTest
Test Checklist