From f1f91e8d527aea01aff9e93ce9e9e23d68ecd44a Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Fri, 20 Jul 2018 11:20:24 +0200 Subject: [PATCH 1/2] unified action creators/actions --- scm-ui/src/users/modules/users.js | 36 ++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index 374f9210d5..54ac8b65ba 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -119,15 +119,18 @@ export function fetchUserSuccess(user: User) { export function fetchUserFailure(user: User, error: Error) { return { type: FETCH_USER_FAILURE, - user, - error + error: true, + payload: { + user, + error + } }; } export function requestAddUser(user: User) { return { type: ADD_USER, - user + payload: user }; } @@ -157,18 +160,21 @@ export function addUserSuccess() { }; } -export function addUserFailure(user: User, err: Error) { +export function addUserFailure(user: User, error: Error) { return { type: ADD_USER_FAILURE, - payload: err, - user + error: true, + payload: { + error, + user + } }; } function requestUpdateUser(user: User) { return { type: UPDATE_USER, - user + payload: user }; } @@ -191,15 +197,18 @@ export function updateUser(user: User) { function updateUserSuccess(user: User) { return { type: UPDATE_USER_SUCCESS, - user + payload: user }; } export function updateUserFailure(user: User, error: Error) { return { type: UPDATE_USER_FAILURE, - payload: error, - user + error: true, + payload: { + error, + user + } }; } @@ -220,6 +229,7 @@ export function deleteUserSuccess(user: User) { export function deleteUserFailure(user: User, error: Error) { return { type: DELETE_USER_FAILURE, + error: true, payload: { error, user @@ -373,9 +383,9 @@ export default function reducer(state: any = {}, action: any = {}) { usersByNames: ubn }; case FETCH_USER_FAILURE: - return reduceUsersByNames(state, action.user.name, { + return reduceUsersByNames(state, action.payload.user.name, { loading: true, - error: action.error + error: action.payload.error }); // Delete single user cases case DELETE_USER: @@ -439,7 +449,7 @@ export default function reducer(state: any = {}, action: any = {}) { users: { ...state.users, loading: false, - error: action.payload + error: action.payload.error } }; // Update single user cases From 3afd56002811865753c6ae9447b6d810f5ec216b Mon Sep 17 00:00:00 2001 From: Philipp Czora Date: Tue, 24 Jul 2018 13:02:50 +0200 Subject: [PATCH 2/2] Refactored /me-Resource --- scm-ui/src/components/Footer.js | 5 +- scm-ui/src/containers/App.js | 16 ++- scm-ui/src/types/me.js | 4 - scm-ui/src/users/modules/users.js | 44 ++++---- scm-ui/src/users/modules/users.test.js | 35 +++--- .../scm/api/v2/resources/MeResource.java | 51 ++++++--- .../scm/api/v2/resources/MeResourceTest.java | 103 ++++++++++++++++++ 7 files changed, 190 insertions(+), 68 deletions(-) delete mode 100644 scm-ui/src/types/me.js create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java diff --git a/scm-ui/src/components/Footer.js b/scm-ui/src/components/Footer.js index fd17c06d8b..db051ef791 100644 --- a/scm-ui/src/components/Footer.js +++ b/scm-ui/src/components/Footer.js @@ -1,9 +1,8 @@ //@flow import React from "react"; -import type { Me } from "../types/me"; type Props = { - me?: Me + me?: string }; class Footer extends React.Component { @@ -15,7 +14,7 @@ class Footer extends React.Component { return (
-

{me.username}

+

{me}

); diff --git a/scm-ui/src/containers/App.js b/scm-ui/src/containers/App.js index 4ff9b245f7..d01ece2902 100644 --- a/scm-ui/src/containers/App.js +++ b/scm-ui/src/containers/App.js @@ -17,7 +17,8 @@ type Props = { error: Error, loading: boolean, authenticated?: boolean, - fetchMe: () => void + fetchMe: () => void, + displayName: string }; class App extends Component { @@ -26,7 +27,7 @@ class App extends Component { } render() { - const { entry, loading, error, authenticated } = this.props; + const { me, loading, error, authenticated, displayName } = this.props; let content; const navigation = authenticated ? : ""; @@ -48,7 +49,7 @@ class App extends Component {
{navigation}
{content} -
+
); } @@ -62,10 +63,17 @@ const mapDispatchToProps = (dispatch: any) => { const mapStateToProps = state => { let mapped = state.auth.me || {}; + let displayName; if (state.auth.login) { mapped.authenticated = state.auth.login.authenticated; } - return mapped; + if (state.auth.me && state.auth.me.entry) { + displayName = state.auth.me.entry.entity.displayName; + } + return { + ...mapped, + displayName + }; }; export default withRouter( diff --git a/scm-ui/src/types/me.js b/scm-ui/src/types/me.js deleted file mode 100644 index d17798bf5f..0000000000 --- a/scm-ui/src/types/me.js +++ /dev/null @@ -1,4 +0,0 @@ -// @flow -export type Me = { - username: string -}; diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index 54ac8b65ba..8b3d78fe44 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -2,28 +2,26 @@ import { apiClient, NOT_FOUND_ERROR } from "../../apiclient"; import type { User } from "../types/User"; import type { UserEntry } from "../types/UserEntry"; -import { Dispatch } from "redux"; +import asyncActionCreator from "./asyncActionCreator"; -export const FETCH_USERS = "scm/users/FETCH"; +export const FETCH_USERS_PENDING = "scm/users/FETCH_PENDING"; export const FETCH_USERS_SUCCESS = "scm/users/FETCH_SUCCESS"; export const FETCH_USERS_FAILURE = "scm/users/FETCH_FAILURE"; export const FETCH_USERS_NOTFOUND = "scm/users/FETCH_NOTFOUND"; -export const FETCH_USER = "scm/users/FETCH_USER"; +export const FETCH_USER_PENDING = "scm/users/FETCH_USER_PENDING"; export const FETCH_USER_SUCCESS = "scm/users/FETCH_USER_SUCCESS"; export const FETCH_USER_FAILURE = "scm/users/FETCH_USER_FAILURE"; -export const ADD_USER = "scm/users/ADD"; +export const ADD_USER_PENDING = "scm/users/ADD_PENDING"; export const ADD_USER_SUCCESS = "scm/users/ADD_SUCCESS"; export const ADD_USER_FAILURE = "scm/users/ADD_FAILURE"; -export const EDIT_USER = "scm/users/EDIT"; - -export const UPDATE_USER = "scm/users/UPDATE"; +export const UPDATE_USER_PENDING = "scm/users/UPDATE_PENDING"; export const UPDATE_USER_SUCCESS = "scm/users/UPDATE_SUCCESS"; export const UPDATE_USER_FAILURE = "scm/users/UPDATE_FAILURE"; -export const DELETE_USER = "scm/users/DELETE"; +export const DELETE_USER_PENDING = "scm/users/DELETE_PENDING"; export const DELETE_USER_SUCCESS = "scm/users/DELETE_SUCCESS"; export const DELETE_USER_FAILURE = "scm/users/DELETE_FAILURE"; @@ -34,7 +32,7 @@ const CONTENT_TYPE_USER = "application/vnd.scmm-user+json;v=2"; export function requestUsers() { return { - type: FETCH_USERS + type: FETCH_USERS_PENDING }; } @@ -80,7 +78,7 @@ export function fetchUsersSuccess(users: any) { export function requestUser(name: string) { return { - type: FETCH_USER, + type: FETCH_USER_PENDING, payload: { name } }; } @@ -88,7 +86,7 @@ export function requestUser(name: string) { export function fetchUser(name: string) { const userUrl = USER_URL + name; return function(dispatch: any) { - dispatch(requestUsers()); + dispatch(requestUser(name)); return apiClient .get(userUrl) .then(response => { @@ -104,7 +102,7 @@ export function fetchUser(name: string) { }) .catch(cause => { const error = new Error(`could not fetch user: ${cause.message}`); - dispatch(failedToFetchUsers(USERS_URL, error)); + dispatch(fetchUserFailure(USERS_URL, error)); }); }; } @@ -116,12 +114,12 @@ export function fetchUserSuccess(user: User) { }; } -export function fetchUserFailure(user: User, error: Error) { +export function fetchUserFailure(username: string, error: Error) { return { type: FETCH_USER_FAILURE, error: true, payload: { - user, + username, error } }; @@ -129,7 +127,7 @@ export function fetchUserFailure(user: User, error: Error) { export function requestAddUser(user: User) { return { - type: ADD_USER, + type: ADD_USER_PENDING, payload: user }; } @@ -173,7 +171,7 @@ export function addUserFailure(user: User, error: Error) { function requestUpdateUser(user: User) { return { - type: UPDATE_USER, + type: UPDATE_USER_PENDING, payload: user }; } @@ -214,7 +212,7 @@ export function updateUserFailure(user: User, error: Error) { export function requestDeleteUser(user: User) { return { - type: DELETE_USER, + type: DELETE_USER_PENDING, payload: user }; } @@ -325,7 +323,7 @@ const reduceUsersByNames = ( export default function reducer(state: any = {}, action: any = {}) { switch (action.type) { // fetch user list cases - case FETCH_USERS: + case FETCH_USERS_PENDING: return { ...state, users: { @@ -361,7 +359,7 @@ export default function reducer(state: any = {}, action: any = {}) { } }; // Fetch single user cases - case FETCH_USER: + case FETCH_USER_PENDING: return reduceUsersByNames(state, action.payload.name, { loading: true, error: null @@ -383,12 +381,12 @@ export default function reducer(state: any = {}, action: any = {}) { usersByNames: ubn }; case FETCH_USER_FAILURE: - return reduceUsersByNames(state, action.payload.user.name, { + return reduceUsersByNames(state, action.payload.username, { loading: true, error: action.payload.error }); // Delete single user cases - case DELETE_USER: + case DELETE_USER_PENDING: return reduceUsersByNames(state, action.payload.name, { loading: true, error: null, @@ -425,7 +423,7 @@ export default function reducer(state: any = {}, action: any = {}) { } }; // Add single user cases - case ADD_USER: + case ADD_USER_PENDING: return { ...state, users: { @@ -453,7 +451,7 @@ export default function reducer(state: any = {}, action: any = {}) { } }; // Update single user cases - case UPDATE_USER: + case UPDATE_USER_PENDING: return { ...state, usersByNames: { diff --git a/scm-ui/src/users/modules/users.test.js b/scm-ui/src/users/modules/users.test.js index aef072da0e..72441d422f 100644 --- a/scm-ui/src/users/modules/users.test.js +++ b/scm-ui/src/users/modules/users.test.js @@ -4,22 +4,21 @@ import thunk from "redux-thunk"; import fetchMock from "fetch-mock"; import { - FETCH_USERS, + FETCH_USERS_PENDING, FETCH_USERS_SUCCESS, fetchUsers, FETCH_USERS_FAILURE, addUser, - ADD_USER, + ADD_USER_PENDING, ADD_USER_SUCCESS, ADD_USER_FAILURE, updateUser, - UPDATE_USER, + UPDATE_USER_PENDING, UPDATE_USER_FAILURE, UPDATE_USER_SUCCESS, - EDIT_USER, requestDeleteUser, deleteUserFailure, - DELETE_USER, + DELETE_USER_PENDING, DELETE_USER_SUCCESS, DELETE_USER_FAILURE, deleteUser, @@ -143,7 +142,7 @@ describe("users fetch()", () => { fetchMock.getOnce("/scm/api/rest/v2/users", response); const expectedActions = [ - { type: FETCH_USERS }, + { type: FETCH_USERS_PENDING }, { type: FETCH_USERS_SUCCESS, payload: response @@ -165,7 +164,7 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(fetchUsers()).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(FETCH_USERS); + expect(actions[0].type).toEqual(FETCH_USERS_PENDING); expect(actions[1].type).toEqual(FETCH_USERS_FAILURE); expect(actions[1].payload).toBeDefined(); }); @@ -183,9 +182,9 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(addUser(userZaphod)).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(ADD_USER); + expect(actions[0].type).toEqual(ADD_USER_PENDING); expect(actions[1].type).toEqual(ADD_USER_SUCCESS); - expect(actions[2].type).toEqual(FETCH_USERS); + expect(actions[2].type).toEqual(FETCH_USERS_PENDING); }); }); @@ -197,7 +196,7 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(addUser(userZaphod)).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(ADD_USER); + expect(actions[0].type).toEqual(ADD_USER_PENDING); expect(actions[1].type).toEqual(ADD_USER_FAILURE); expect(actions[1].payload).toBeDefined(); }); @@ -213,9 +212,9 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(updateUser(userZaphod)).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(UPDATE_USER); + expect(actions[0].type).toEqual(UPDATE_USER_PENDING); expect(actions[1].type).toEqual(UPDATE_USER_SUCCESS); - expect(actions[2].type).toEqual(FETCH_USERS); + expect(actions[2].type).toEqual(FETCH_USERS_PENDING); }); }); @@ -227,7 +226,7 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(updateUser(userZaphod)).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(UPDATE_USER); + expect(actions[0].type).toEqual(UPDATE_USER_PENDING); expect(actions[1].type).toEqual(UPDATE_USER_FAILURE); expect(actions[1].payload).toBeDefined(); }); @@ -243,10 +242,10 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(deleteUser(userZaphod)).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(DELETE_USER); + expect(actions[0].type).toEqual(DELETE_USER_PENDING); expect(actions[0].payload).toBe(userZaphod); expect(actions[1].type).toEqual(DELETE_USER_SUCCESS); - expect(actions[2].type).toEqual(FETCH_USERS); + expect(actions[2].type).toEqual(FETCH_USERS_PENDING); }); }); @@ -258,7 +257,7 @@ describe("users fetch()", () => { const store = mockStore({}); return store.dispatch(deleteUser(userZaphod)).then(() => { const actions = store.getActions(); - expect(actions[0].type).toEqual(DELETE_USER); + expect(actions[0].type).toEqual(DELETE_USER_PENDING); expect(actions[0].payload).toBe(userZaphod); expect(actions[1].type).toEqual(DELETE_USER_FAILURE); expect(actions[1].payload).toBeDefined(); @@ -412,10 +411,10 @@ describe("users reducer", () => { expect(newState.usersByNames["zaphod"].loading).toBeTruthy(); }); - it("should uppdate state according to FETCH_USER_FAILURE action", () => { + it("should update state according to FETCH_USER_FAILURE action", () => { const newState = reducer( {}, - fetchUserFailure(userFord, new Error("kaputt!")) + fetchUserFailure(userFord.name, new Error("kaputt!")) ); expect(newState.usersByNames["ford"].error).toBeTruthy; }); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java index 5646ca60cf..3f3b880bd7 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java @@ -1,34 +1,53 @@ package sonia.scm.api.v2.resources; -import lombok.AllArgsConstructor; -import lombok.Getter; -import lombok.NoArgsConstructor; -import lombok.Setter; +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; +import com.webcohesion.enunciate.metadata.rs.TypeHint; import org.apache.shiro.SecurityUtils; +import sonia.scm.user.User; +import sonia.scm.user.UserException; +import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; +import javax.inject.Inject; import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.Produces; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.Request; import javax.ws.rs.core.Response; +import javax.ws.rs.core.UriInfo; + @Path(MeResource.ME_PATH_V2) public class MeResource { static final String ME_PATH_V2 = "v2/me/"; + private final UserToUserDtoMapper userToDtoMapper; + + private final ResourceManagerAdapter adapter; + + @Inject + public MeResource(UserToUserDtoMapper userToDtoMapper, UserManager manager) { + this.userToDtoMapper = userToDtoMapper; + this.adapter = new ResourceManagerAdapter<>(manager); + } + + /** + * Returns the currently logged in user or a 401 if user is not logged in + */ @GET - @Produces(VndMediaType.ME) - public Response get() { - MeDto meDto = new MeDto((String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal()); - return Response.ok(meDto).build(); - } + @Path("") + @Produces(VndMediaType.USER) + @TypeHint(UserDto.class) + @StatusCodes({ + @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 500, condition = "internal server error") + }) + public Response get(@Context Request request, @Context UriInfo uriInfo) { - @NoArgsConstructor - @AllArgsConstructor - @Getter - @Setter - class MeDto { - String username; + String id = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); + return adapter.get(id, userToDtoMapper::map); } - } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java new file mode 100644 index 0000000000..8c08a433f5 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java @@ -0,0 +1,103 @@ +package sonia.scm.api.v2.resources; + +import com.github.sdorra.shiro.ShiroRule; +import com.github.sdorra.shiro.SubjectAware; +import com.google.common.io.Resources; +import org.apache.shiro.authc.credential.PasswordService; +import org.apache.shiro.subject.PrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.jboss.resteasy.core.Dispatcher; +import org.jboss.resteasy.mock.MockDispatcherFactory; +import org.jboss.resteasy.mock.MockHttpRequest; +import org.jboss.resteasy.mock.MockHttpResponse; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import sonia.scm.PageResult; +import sonia.scm.user.User; +import sonia.scm.user.UserException; +import sonia.scm.user.UserManager; +import sonia.scm.web.VndMediaType; + +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.core.UriInfo; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; + +import static java.util.Collections.singletonList; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.*; +import static org.mockito.MockitoAnnotations.initMocks; + +@SubjectAware( +// username = "trillian", +// password = "secret", + configuration = "classpath:sonia/scm/repository/shiro.ini" +) +public class MeResourceTest { + + @Rule + public ShiroRule shiro = new ShiroRule(); + + private Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); + + @Mock + private UriInfo uriInfo; + @Mock + private UriInfoStore uriInfoStore; + + @Mock + private UserManager userManager; + + @InjectMocks + private UserToUserDtoMapperImpl userToDtoMapper; + + private ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); + + @Before + public void prepareEnvironment() throws IOException, UserException { + initMocks(this); + createDummyUser("trillian"); + doNothing().when(userManager).create(userCaptor.capture()); + doNothing().when(userManager).modify(userCaptor.capture()); + doNothing().when(userManager).delete(userCaptor.capture()); + MeResource meResource = new MeResource(userToDtoMapper, userManager); + dispatcher.getRegistry().addSingletonResource(meResource); + when(uriInfo.getBaseUri()).thenReturn(URI.create("/")); + when(uriInfoStore.get()).thenReturn(uriInfo); + } + + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldReturnCurrentlyAuthenticatedUser() throws URISyntaxException { + MockHttpRequest request = MockHttpRequest.get("/" + MeResource.ME_PATH_V2); + request.accept(VndMediaType.USER); + MockHttpResponse response = new MockHttpResponse(); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertTrue(response.getContentAsString().contains("\"name\":\"trillian\"")); + assertTrue(response.getContentAsString().contains("\"password\":\"__dummypassword__\"")); + assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/trillian\"}")); + assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/trillian\"}")); + } + + private User createDummyUser(String name) { + User user = new User(); + user.setName(name); + user.setPassword("secret"); + user.setCreationDate(System.currentTimeMillis()); + when(userManager.get(name)).thenReturn(user); + return user; + } +}