Skip to content

Commit

Permalink
fix(users): introduce generic ids for users instead of using emails a…
Browse files Browse the repository at this point in the history
…s ids

closes #242
  • Loading branch information
alexbrdn committed May 2, 2018
1 parent 503fe9f commit 3da49c1
Show file tree
Hide file tree
Showing 23 changed files with 157 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
map = "function(doc) { if (doc.type == 'user') emit(null, doc._id) }"),
@View(name = "byExternalId",
map = "function(doc) { if (doc.type == 'user') emit(doc.externalid, doc._id) }"),
@View(name = "byEmail",
map = "function(doc) { if (doc.type == 'user') emit(doc.email, doc._id) }"),
@View(name = "byFormerEmails",
map = "function(doc) {" +
" if (doc.type == 'user' && doc.formerEmailAddresses && Array.isArray(doc.formerEmailAddresses)) {" +
Expand Down Expand Up @@ -68,4 +70,11 @@ public User getByFormerEmail(String email) {
return get(CommonUtils.getFirst(userIds));
return null;
}

public User getByEmail(String email) {
final Set<String> userIds = queryForIdsAsValue("byEmail", email);
if (userIds != null && !userIds.isEmpty())
return get(CommonUtils.getFirst(userIds));
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ public class ComponentDatabaseHandlerTest {
private static final String email1 = "[email protected]";
private static final String email2 = "[email protected]";

private static final User user1 = new User().setEmail(email1).setDepartment("AB CD EF").setId(email1);
private static final User user2 = new User().setEmail(email2).setDepartment("AB CD EF").setId(email2);
private static final User user1 = new User().setEmail(email1).setDepartment("AB CD EF").setId("481489458");
private static final User user2 = new User().setEmail(email2).setDepartment("AB CD EF").setId("4786487647680");

@Rule
public final ExpectedException exception = ExpectedException.none();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static void main(String[] args) throws TException, IOException {
TProtocol protocol = new TCompactProtocol(thriftClient);
ModerationService.Iface client = new ModerationService.Client(protocol);

List<ModerationRequest> requestsByModerator = client.getRequestsByModerator(new User().setId("").setEmail("[email protected]").setDepartment("BB"));
List<ModerationRequest> requestsByModerator = client.getRequestsByModerator(new User().setId("58245y9845").setEmail("[email protected]").setDepartment("BB"));


System.out.println("Fetched " + requestsByModerator.size() + " moderation requests from moderation service");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ public UserHandler() throws IOException {
db = new UserDatabaseHandler(DatabaseSettings.getConfiguredHttpClient(), DatabaseSettings.COUCH_DB_USERS);
}

@Override
public User getUser(String id) throws TException {
return db.getUser(id);
}

@Override
public User getByEmail(String email) throws TException {
StackTraceElement stackTraceElement = Thread.currentThread().getStackTrace()[2];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ public UserDatabaseHandler(Supplier<HttpClient> httpClient, String dbName) throw
}

public User getByEmail(String email) {
return db.get(User.class, email);
return repository.getByEmail(email);
}

public User getUser(String id) {
return db.get(User.class, id);
}

public User getByFormerEmail(String email) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,12 @@ public static <T> org.eclipse.sw360.datahandler.thrift.users.User synchronizeUse
synchronizer.accept(resultUser, source);
client.addUser(resultUser);
} else {
if (existingThriftUser.getEmail().equals(email)){
resultUser = existingThriftUser;
synchronizer.accept(resultUser, source);
client.updateUser(resultUser);
} else { // email has changed
org.eclipse.sw360.datahandler.thrift.users.User newThriftUser = new org.eclipse.sw360.datahandler.thrift.users.User();
synchronizer.accept(newThriftUser, source);
newThriftUser.setFormerEmailAddresses(prepareFormerEmailAddresses(existingThriftUser, email));

client.addUser(newThriftUser);
client.deleteUser(existingThriftUser, existingThriftUser);
resultUser = newThriftUser;
resultUser = existingThriftUser;
if (!existingThriftUser.getEmail().equals(email)) { // email has changed
resultUser.setFormerEmailAddresses(prepareFormerEmailAddresses(existingThriftUser, email));
}
synchronizer.accept(resultUser, source);
client.updateUser(resultUser);
}
} catch (TException e) {
log.error("Thrift exception when saving the user", e);
Expand Down Expand Up @@ -194,7 +187,6 @@ private boolean equivalent(org.eclipse.sw360.datahandler.thrift.users.User refre

public static void fillThriftUserFromUserCSV(final org.eclipse.sw360.datahandler.thrift.users.User thriftUser, final UserCSV userCsv) {
thriftUser.setEmail(userCsv.getEmail());
thriftUser.setId(userCsv.getEmail());
thriftUser.setType(TYPE_USER);
thriftUser.setUserGroup(UserGroup.valueOf(userCsv.getGroup()));
thriftUser.setExternalid(userCsv.getGid());
Expand All @@ -207,7 +199,6 @@ public static void fillThriftUserFromUserCSV(final org.eclipse.sw360.datahandler

public static void fillThriftUserFromLiferayUser(final org.eclipse.sw360.datahandler.thrift.users.User thriftUser, final User user) {
thriftUser.setEmail(user.getEmailAddress());
thriftUser.setId(user.getEmailAddress());
thriftUser.setType(TYPE_USER);
thriftUser.setUserGroup(getUserGroupFromLiferayUser(user));
thriftUser.setExternalid(user.getOpenId());
Expand All @@ -219,7 +210,7 @@ public static void fillThriftUserFromLiferayUser(final org.eclipse.sw360.datahan

public static void fillThriftUserFromThriftUser(final org.eclipse.sw360.datahandler.thrift.users.User thriftUser, final org.eclipse.sw360.datahandler.thrift.users.User user) {
thriftUser.setEmail(user.getEmail());
thriftUser.setId(user.getEmail());
thriftUser.setId(user.getId());
thriftUser.setType(TYPE_USER);
thriftUser.setUserGroup(user.getUserGroup());
thriftUser.setExternalid(user.getExternalid());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ public static void prepareLicense(License license) throws SW360Exception {
public static void prepareUser(User user) throws SW360Exception {
// Check required fields
assertNotEmpty(user.getEmail());
// Set id to email, in order to have human readable database
user.setId(user.getEmail());
// Set type
user.setType(TYPE_USER);
// guarantee that `CommentMadeDuringModerationRequest` is never stored in the database as this is intended to be a transient field
Expand Down
7 changes: 6 additions & 1 deletion libraries/lib-datahandler/src/main/thrift/users.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ struct User {
service UserService {

/**
* returns SW360-user with id equal to email
* returns SW360-user with given id
**/
User getUser(1:string id);

/**
* returns SW360-user with given email
**/
User getByEmail(1:string email);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static org.eclipse.sw360.datahandler.thrift.ThriftValidate.prepareUser;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;

public class ThriftValidateTest {
final String DUMMY_EMAIL_ADDRESS = "[email protected]";
Expand All @@ -36,8 +37,8 @@ public void testPrepareUser() throws Exception {
user.setCommentMadeDuringModerationRequest(DUMMY_MODERATION_COMMENT);
prepareUser(user);

assertEquals(user.getEmail(), user.getId());
assertEquals(TYPE_USER,user.getType());
assertNull(user.getId());
assertEquals(TYPE_USER, user.getType());
assertFalse(user.isSetCommentMadeDuringModerationRequest());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ static abstract class ProjectMixin extends Project {

@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonIgnoreProperties({
"id",
"revision",
"externalid",
"wantsMailNotification",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.springframework.security.oauth2.provider.OAuth2Authentication;
import org.springframework.stereotype.Service;

import java.util.Base64;
import java.util.List;
import java.util.Set;

Expand Down Expand Up @@ -97,8 +96,7 @@ public void addEmbeddedUser(HalResource halResource, User user, String relation)
User embeddedUser = convertToEmbeddedUser(user);
Resource<User> embeddedUserResource = new Resource<>(embeddedUser);
try {
String userUUID = Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8"));
Link userLink = linkTo(UserController.class).slash("api/users/" + userUUID).withSelfRel();
Link userLink = linkTo(UserController.class).slash("api/users/" + user.getId()).withSelfRel();
embeddedUserResource.add(userLink);
} catch (Exception e) {
LOGGER.error("cannot create embedded user with email: " + user.getEmail());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ public User getUserByEmail(String email) {
}
}

public User getUser(String id) {
try {
UserService.Iface sw360UserClient = getThriftUserClient();
return sw360UserClient.getUser(id);
} catch (TException e) {
throw new RuntimeException(e);
}
}

private UserService.Iface getThriftUserClient() throws TTransportException {
THttpClient thriftClient = new THttpClient(thriftServerUrl + "/users/thrift");
TProtocol protocol = new TCompactProtocol(thriftClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,18 @@ public ResponseEntity<Resources<Resource<User>>> getUsers() {
return new ResponseEntity<>(resources, HttpStatus.OK);
}

@RequestMapping(value = USERS_URL + "/{id:.+}", method = RequestMethod.GET)
@RequestMapping(value = USERS_URL + "/{email:.+}", method = RequestMethod.GET)
public ResponseEntity<Resource<User>> getUserByEmail(
@PathVariable("email") String email) {
User sw360User = userService.getUserByEmail(email);
HalResource<User> halResource = createHalUser(sw360User);
return new ResponseEntity<>(halResource, HttpStatus.OK);
}

@RequestMapping(value = USERS_URL + "/byid/{id:.+}", method = RequestMethod.GET)
public ResponseEntity<Resource<User>> getUser(
@PathVariable("id") String id) {
byte[] base64decodedBytes = Base64.getDecoder().decode(id);
String decodedId;
try {
decodedId = new String(base64decodedBytes, "utf-8");
} catch (UnsupportedEncodingException e) {
throw (new RuntimeException(e));
}

User sw360User = userService.getUserByEmail(decodedId);
User sw360User = userService.getUser(id);
HalResource<User> halResource = createHalUser(sw360User);
return new ResponseEntity<>(halResource, HttpStatus.OK);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void before() throws TException {
given(this.componentServiceMock.getComponentsForUser(anyObject())).willReturn(componentList);

User user = new User();
user.setId("[email protected]");
// user.setId("[email protected]");
user.setEmail("[email protected]");
user.setFullname("John Doe");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void before() throws TException {
given(this.projectServiceMock.getProjectsForUser(anyObject())).willReturn(projectList);

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ public void before() {
List<User> userList = new ArrayList<>();

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");
userList.add(user);

user = new User();
user.setId("[email protected]");
user.setId("987654321");
user.setEmail("[email protected]");
user.setFullname("Jane Doe");
userList.add(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void before() throws TException {
given(this.attachmentServiceMock.getAttachmentByIdForUser(eq(attachment.getAttachmentContentId()), anyObject())).willReturn(attachmentInfo);

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public void before() throws TException {
given(this.componentServiceMock.searchComponentByName(eq(angularComponent.getName()))).willReturn(componentListByName);

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");
user.setDepartment("sw360");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public void before() throws TException {
given(this.releaseServiceMock.getReleaseForUserById(eq(release2.getId()), anyObject())).willReturn(release2);

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");
user.setDepartment("sw360");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void before() throws TException {
given(this.releaseServiceMock.getReleaseForUserById(eq(release.getId()), anyObject())).willReturn(release);

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ public class UserSpecTest extends TestRestDocsSpecBase {
private User user;

@Before
public void before() throws UnsupportedEncodingException {
public void before() {
List<User> userList = new ArrayList<>();

user = new User();
user.setEmail("[email protected]");
user.setId(Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8")));
user.setId("4784587578e87989");
user.setUserGroup(UserGroup.ADMIN);
user.setFullname("John Doe");
user.setGivenname("John");
Expand All @@ -66,10 +66,11 @@ public void before() throws UnsupportedEncodingException {
userList.add(user);

given(this.userServiceMock.getUserByEmail("[email protected]")).willReturn(user);
given(this.userServiceMock.getUser("4784587578e87989")).willReturn(user);

User user2 = new User();
user2.setEmail("[email protected]");
user2.setId(Base64.getEncoder().encodeToString(user.getEmail().getBytes("utf-8")));
user2.setId("frwey45786rwe");
user2.setUserGroup(UserGroup.USER);
user2.setFullname("Jane Doe");
user2.setGivenname("Jane");
Expand Down Expand Up @@ -102,7 +103,7 @@ public void should_document_get_users() throws Exception {
@Test
public void should_document_get_user() throws Exception {
String accessToken = TestHelper.getAccessToken(mockMvc, testUserId, testUserPassword);
mockMvc.perform(get("/api/users/" + user.getId())
mockMvc.perform(get("/api/users/byid/" + user.getId())
.header("Authorization", "Bearer " + accessToken)
.accept(MediaTypes.HAL_JSON))
.andExpect(status().isOk())
Expand All @@ -111,6 +112,31 @@ public void should_document_get_user() throws Exception {
linkWithRel("self").description("The <<resources-users,User resource>>")
),
responseFields(
fieldWithPath("id").description("The user's id"),
fieldWithPath("email").description("The user's email"),
fieldWithPath("userGroup").description("The user group, possible values are: " + Arrays.asList(UserGroup.values())),
fieldWithPath("fullName").description("The users's full name"),
fieldWithPath("givenName").description("The user's given name"),
fieldWithPath("lastName").description("The user's last name"),
fieldWithPath("department").description("The user's company department"),
fieldWithPath("formerEmailAddresses").description("The user's former email addresses"),
fieldWithPath("_links").description("<<resources-index-links,Links>> to other resources")
)));
}

@Test
public void should_document_get_user_by_email() throws Exception {
String accessToken = TestHelper.getAccessToken(mockMvc, testUserId, testUserPassword);
mockMvc.perform(get("/api/users/" + user.getEmail())
.header("Authorization", "Bearer " + accessToken)
.accept(MediaTypes.HAL_JSON))
.andExpect(status().isOk())
.andDo(this.documentationHandler.document(
links(
linkWithRel("self").description("The <<resources-users,User resource>>")
),
responseFields(
fieldWithPath("id").description("The user's id"),
fieldWithPath("email").description("The user's email"),
fieldWithPath("userGroup").description("The user group, possible values are: " + Arrays.asList(UserGroup.values())),
fieldWithPath("fullName").description("The users's full name"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void before() {
vulnerabilityList.add(vulnerability2);

User user = new User();
user.setId("[email protected]");
user.setId("123456789");
user.setEmail("[email protected]");
user.setFullname("John Doe");

Expand Down
Loading

0 comments on commit 3da49c1

Please sign in to comment.