Skip to content

Commit

Permalink
YD-687 Correct goal update scenarios
Browse files Browse the repository at this point in the history
The previous change broke scenarios where the goal with activities was updated. The issue is that originally, we initialized the GoalIdMapping using the DTOs but then we started using the entities. The DTOs have a collection goals which actually contains goalsIncludingHistoryItems. Because of that, the tests that use activities on updated goals now fail, as the history items are not available in the mapping. This is corrected now.

* The goal ID mapping is now initialized including the history items
* Several more goal collections that actually contain "goals including history items" are now renamed
* The user entity is locked prior to making goal changes
* The behavior for calculating the set of goals including history items (which was already based on entities) is now moved to the UserAnonymized entity
* The entity loading statistics were recalculated: the situation worsened considerably. Before this PR is merged, I want to see whether that can be recovered.
  • Loading branch information
Bert-R committed Sep 30, 2020
1 parent 3797f22 commit e7ea610
Show file tree
Hide file tree
Showing 12 changed files with 148 additions and 56 deletions.
Expand Up @@ -409,7 +409,7 @@ static Set<GoalDto> determineRelevantGoals(ActivityPayload payload)
&& payload.deviceAnonymized.getOperatingSystem() == OperatingSystem.ANDROID;
Set<UUID> matchingActivityCategoryIds = payload.activityCategories.stream().map(ActivityCategoryDto::getId)
.collect(Collectors.toSet());
Set<GoalDto> goalsOfUser = payload.userAnonymized.getGoals();
Set<GoalDto> goalsOfUser = payload.userAnonymized.getGoalsIncludingHistoryItems();
return goalsOfUser.stream().filter(g -> !g.isHistoryItem())
.filter(g -> matchingActivityCategoryIds.contains(g.getActivityCategoryId()))
.filter(g -> g.isNoGoGoal() || !onlyNoGoGoals)
Expand Down
104 changes: 90 additions & 14 deletions appservice/src/intTest/HibernateStatsTest.md

Large diffs are not rendered by default.

Expand Up @@ -165,7 +165,7 @@ private List<WeekActivityOverviewDto> getWeekActivityOverviews(UUID userAnonymiz
weekActivityEntitiesByLocalDate);
Map<ZonedDateTime, Set<WeekActivityDto>> weekActivityDtosByZonedDate = mapWeekActivitiesToDtos(earliestPossibleDate,
weekActivityEntitiesByZonedDate, userAnonymized);
addMissingInactivity(userAnonymized.getGoals(), weekActivityDtosByZonedDate, interval, ChronoUnit.WEEKS, userAnonymized,
addMissingInactivity(userAnonymized.getGoalsIncludingHistoryItems(), weekActivityDtosByZonedDate, interval, ChronoUnit.WEEKS, userAnonymized,
(goal, startOfWeek) -> createAndSaveWeekInactivity(userAnonymized, earliestPossibleDate, goal, startOfWeek,
LevelOfDetail.WEEK_OVERVIEW, missingInactivities),
Optional.of((g, wa) -> createAndSaveInactivityDays(userAnonymized, earliestPossibleDate,
Expand Down Expand Up @@ -352,7 +352,7 @@ private Map<ZonedDateTime, Set<DayActivityDto>> getDayActivitiesForCategories(Us
LocalDate earliestPossibleDate, Set<UUID> relevantActivityCategoryIds, Interval interval,
Set<IntervalInactivityDto> mia)
{
Set<GoalDto> relevantGoals = userAnonymizedDto.getGoals().stream()
Set<GoalDto> relevantGoals = userAnonymizedDto.getGoalsIncludingHistoryItems().stream()
.filter(g -> relevantActivityCategoryIds.contains(g.getActivityCategoryId())).collect(Collectors.toSet());
return getDayActivities(userAnonymizedDto, earliestPossibleDate, relevantGoals, interval, mia);
}
Expand Down Expand Up @@ -414,7 +414,7 @@ private List<DayActivityOverviewDto<DayActivityDto>> getDayActivityOverviews(Loc
private Map<ZonedDateTime, Set<DayActivityDto>> getDayActivities(UserAnonymizedDto userAnonymized,
LocalDate earliestPossibleDate, Interval interval, Set<IntervalInactivityDto> missingInactivities)
{
return getDayActivities(userAnonymized, earliestPossibleDate, userAnonymized.getGoals(), interval, missingInactivities);
return getDayActivities(userAnonymized, earliestPossibleDate, userAnonymized.getGoalsIncludingHistoryItems(), interval, missingInactivities);
}

private Map<ZonedDateTime, Set<DayActivityDto>> getDayActivities(UserAnonymizedDto userAnonymized,
Expand Down
Expand Up @@ -71,7 +71,7 @@ public GoalDto getGoalForUserId(UUID userId, UUID goalId)

public GoalDto getGoalForUserAnonymizedId(UUID userAnonymizedId, UUID goalId)
{
return userAnonymizedService.getUserAnonymized(userAnonymizedId).getGoals().stream()
return userAnonymizedService.getUserAnonymized(userAnonymizedId).getGoalsIncludingHistoryItems().stream()
.filter(g -> g.getGoalId().equals(goalId)).findFirst()
.orElseThrow(() -> GoalServiceException.goalNotFoundByIdForUserAnonymized(userAnonymizedId, goalId));
}
Expand Down Expand Up @@ -101,7 +101,7 @@ private Goal getGoalEntity(UserAnonymized userAnonymized, UUID goalId)
public GoalDto addGoal(UUID userId, GoalDto goal, Optional<String> message)
{
assertIsValidNewGoal(goal);
User userEntity = userService.getUserEntityById(userId);
User userEntity = userService.lockUserForUpdate(userId);
UserAnonymized userAnonymizedEntity = userEntity.getAnonymized();
Optional<Goal> conflictingExistingGoal = userAnonymizedEntity.getGoals().stream()
.filter(existingGoal -> existingGoal.getActivityCategory().getId().equals(goal.getActivityCategoryId()))
Expand Down Expand Up @@ -131,7 +131,7 @@ private void assertIsValidNewGoal(GoalDto goal)
@Transactional
public GoalDto updateGoal(UUID userId, UUID goalId, GoalDto newGoalDto, Optional<String> message)
{
User userEntity = userService.getUserEntityById(userId);
User userEntity = userService.lockUserForUpdate(userId);
Goal existingGoal = getGoalEntity(userEntity, goalId);

assertValidGoalUpdate(existingGoal, newGoalDto);
Expand Down Expand Up @@ -240,7 +240,7 @@ private void assertNoUpdateToThePast(GoalDto newGoalDto, Goal existingGoal)
@Transactional
public void deleteGoalAndInformBuddies(UUID userId, UUID goalId, Optional<String> message)
{
User userEntity = userService.getUserEntityById(userId);
User userEntity = userService.lockUserForUpdate(userId);
UserAnonymized userAnonymizedEntity = userEntity.getAnonymized();
Goal goalEntity = userAnonymizedEntity.getGoals().stream().filter(goal -> goal.getId().equals(goalId)).findFirst()
.orElseThrow(() -> GoalServiceException.goalNotFoundByIdForUser(userId, goalId));
Expand Down
Expand Up @@ -252,6 +252,11 @@ public Set<Goal> getGoals()
return getUserPrivate().getUserAnonymized().getGoals();
}

public Set<Goal> getGoalsIncludingHistoryItems()
{
return getUserPrivate().getUserAnonymized().getGoalsIncludingHistoryItems();
}

public UUID getUserAnonymizedId()
{
return getUserPrivate().getUserAnonymizedId();
Expand Down
Expand Up @@ -171,4 +171,28 @@ public void preloadGoals()
isGoalPreloadDone = true;
}
}

public Set<Goal> getGoalsIncludingHistoryItems()
{
preloadGoals();
Set<Goal> activeGoals = getGoals();
Set<Goal> historyItems = getGoalHistoryItems(activeGoals);
Set<Goal> allGoals = new HashSet<>(activeGoals);
allGoals.addAll(historyItems);
return allGoals;
}

private static Set<Goal> getGoalHistoryItems(Set<Goal> activeGoals)
{
Set<Goal> historyItems = new HashSet<>();
activeGoals.stream().forEach(g -> {
Optional<Goal> historyItem = g.getPreviousVersionOfThisGoal();
while (historyItem.isPresent())
{
historyItems.add(historyItem.get());
historyItem = historyItem.get().getPreviousVersionOfThisGoal();
}
});
return historyItems;
}
}
Expand Up @@ -53,4 +53,9 @@ public Status getReceivingStatus()
{
return receivingStatus;
}

public boolean isAccepted()
{
return (getReceivingStatus() == Status.ACCEPTED) || (getSendingStatus() == Status.ACCEPTED);
}
}
Expand Up @@ -52,7 +52,7 @@ static BuddyUserPrivateDataDto createInstance(Buddy buddyEntity, BuddyAnonymized
{
UserAnonymizedDto buddyUserAnonymizedDto = buddyUserAnonymizedDtoOptional.orElseThrow(
() -> new IllegalStateException("Should have user anonymized when buddy relationship is established"));
Set<GoalDto> goals = buddyUserAnonymizedDto.getGoals();
Set<GoalDto> goals = buddyUserAnonymizedDto.getGoalsIncludingHistoryItems();
Set<DeviceBaseDto> devices = buddyEntity.getDevices().stream()
.map(bd -> BuddyDeviceDto.createInstance(bd,
getDeviceAnonymizedIfExisting(buddyUserAnonymizedDto, bd.getDeviceAnonymizedId())))
Expand Down
Expand Up @@ -62,9 +62,9 @@ public UUID getBuddyId(UUID goalId)

public static GoalIdMapping createInstance(User user)
{
Set<UUID> userGoalIds = user.getGoals().stream().map(Goal::getId).collect(Collectors.toSet());
Set<UUID> userGoalIds = user.getGoalsIncludingHistoryItems().stream().map(Goal::getId).collect(Collectors.toSet());
Map<UUID, UUID> goalIdToBuddyIdmapping = new HashMap<>();
user.getBuddies().stream().filter(Buddy::isAccepted).forEach(b -> b.getBuddyAnonymized().getUserAnonymized().getGoals()
user.getBuddies().stream().filter(Buddy::isAccepted).forEach(b -> b.getBuddyAnonymized().getUserAnonymized().getGoalsIncludingHistoryItems()
.forEach(g -> goalIdToBuddyIdmapping.put(g.getId(), b.getId())));
Map<UUID, UUID> buddyIdToUserIdmapping = new HashMap<>();
user.getBuddies().forEach(b -> buddyIdToUserIdmapping.put(b.getId(), b.getUser().getId()));
Expand Down
Expand Up @@ -39,9 +39,9 @@ public class OwnUserPrivateDataDto extends UserPrivateDataBaseDto

OwnUserPrivateDataDto(Optional<LocalDate> lastMonitoredActivityDate, String yonaPassword, String firstName, String lastName,
String nickname, Optional<UUID> userPhotoId, UUID namedMessageSourceId, UUID anonymousMessageSourceId,
Set<GoalDto> goals, Set<BuddyDto> buddies, UUID userAnonymizedId, Set<UserDeviceDto> devices)
Set<GoalDto> goalsIncludingHistoryItems, Set<BuddyDto> buddies, UUID userAnonymizedId, Set<UserDeviceDto> devices)
{
super(firstName, lastName, nickname, userPhotoId, Optional.of(goals), Optional.of(new HashSet<>(devices)));
super(firstName, lastName, nickname, userPhotoId, Optional.of(goalsIncludingHistoryItems), Optional.of(new HashSet<>(devices)));

this.lastMonitoredActivityDate = lastMonitoredActivityDate;
this.yonaPassword = yonaPassword;
Expand Down
Expand Up @@ -33,19 +33,19 @@ public class UserAnonymizedDto implements Serializable

private final LocalDate lastMonitoredActivityDate;
private final ZoneId timeZone;
private final Set<GoalDto> goals;
private final Set<GoalDto> goalsIncludingHistoryItems;
private final MessageDestinationDto anonymousMessageDestination;
private final Set<BuddyAnonymizedDto> buddiesAnonymized;
private final Set<DeviceAnonymizedDto> devicesAnonymized;

public UserAnonymizedDto(UUID id, Optional<LocalDate> lastMonitoredActivityDate, ZoneId timeZone, Set<GoalDto> goals,
public UserAnonymizedDto(UUID id, Optional<LocalDate> lastMonitoredActivityDate, ZoneId timeZone, Set<GoalDto> goalsIncludingHistoryItems,
MessageDestinationDto anonymousMessageDestination, Set<BuddyAnonymizedDto> buddiesAnonymized,
Set<DeviceAnonymizedDto> devicesAnonymized)
{
this.id = id;
this.lastMonitoredActivityDate = lastMonitoredActivityDate.orElse(null);
this.timeZone = timeZone;
this.goals = new HashSet<>(goals);
this.goalsIncludingHistoryItems = new HashSet<>(goalsIncludingHistoryItems);
this.anonymousMessageDestination = anonymousMessageDestination;
this.buddiesAnonymized = buddiesAnonymized;
this.devicesAnonymized = devicesAnonymized;
Expand All @@ -71,14 +71,14 @@ public Optional<LocalDate> getLastMonitoredActivityDate()
return Optional.ofNullable(lastMonitoredActivityDate);
}

public Set<GoalDto> getGoals()
public Set<GoalDto> getGoalsIncludingHistoryItems()
{
return goals;
return goalsIncludingHistoryItems;
}

public Set<GoalDto> getGoalsForActivityCategory(ActivityCategory activityCategory)
{
return getGoals().stream().filter(g -> g.getActivityCategoryId().equals(activityCategory.getId()))
return getGoalsIncludingHistoryItems().stream().filter(g -> g.getActivityCategoryId().equals(activityCategory.getId()))
.collect(Collectors.toSet());
}

Expand Down Expand Up @@ -121,28 +121,10 @@ public Set<DeviceAnonymizedDto> getDevicesAnonymized()

static Set<GoalDto> getGoalsIncludingHistoryItems(UserAnonymized userAnonymizedEntity)
{
userAnonymizedEntity.preloadGoals();
Set<Goal> activeGoals = userAnonymizedEntity.getGoals();
Set<Goal> historyItems = getGoalHistoryItems(activeGoals);
Set<Goal> allGoals = new HashSet<>(activeGoals);
allGoals.addAll(historyItems);
Set<Goal> allGoals = userAnonymizedEntity.getGoalsIncludingHistoryItems();
return allGoals.stream().map(GoalDto::createInstance).collect(Collectors.toSet());
}

private static Set<Goal> getGoalHistoryItems(Set<Goal> activeGoals)
{
Set<Goal> historyItems = new HashSet<>();
activeGoals.stream().forEach(g -> {
Optional<Goal> historyItem = g.getPreviousVersionOfThisGoal();
while (historyItem.isPresent())
{
historyItems.add(historyItem.get());
historyItem = historyItem.get().getPreviousVersionOfThisGoal();
}
});
return historyItems;
}

public DeviceAnonymizedDto getDeviceAnonymized(UUID deviceAnonymizedId)
{
return getDeviceAnonymizedIfExisting(deviceAnonymizedId)
Expand All @@ -156,7 +138,7 @@ public Optional<DeviceAnonymizedDto> getDeviceAnonymizedIfExisting(UUID deviceAn

public GoalDto getGoal(UUID goalId)
{
return goals.stream().filter(g -> g.getGoalId().equals(goalId)).findFirst()
return goalsIncludingHistoryItems.stream().filter(g -> g.getGoalId().equals(goalId)).findFirst()
.orElseThrow(() -> GoalServiceException.goalNotFoundByIdForUserAnonymized(id, goalId));
}
}
Expand Up @@ -53,12 +53,12 @@ public class UserDto
private UserDto(UUID id, LocalDateTime creationTime, Optional<LocalDate> appLastOpenedDate,
Optional<LocalDate> lastMonitoredActivityDate, String firstName, String lastName, String yonaPassword,
String nickname, Optional<UUID> userPhotoId, String mobileNumber, boolean isConfirmed,
boolean isCreatedOnBuddyRequest, UUID namedMessageSourceId, UUID anonymousMessageSourceId, Set<GoalDto> goals,
boolean isCreatedOnBuddyRequest, UUID namedMessageSourceId, UUID anonymousMessageSourceId, Set<GoalDto> goalsIncludingHistoryItems,
Set<BuddyDto> buddies, UUID userAnonymizedId, Set<UserDeviceDto> devices)
{
this(id, Optional.of(creationTime), appLastOpenedDate, null, mobileNumber, isConfirmed, isCreatedOnBuddyRequest,
new OwnUserPrivateDataDto(lastMonitoredActivityDate, yonaPassword, firstName, lastName, nickname, userPhotoId,
namedMessageSourceId, anonymousMessageSourceId, goals, buddies, userAnonymizedId, devices));
namedMessageSourceId, anonymousMessageSourceId, goalsIncludingHistoryItems, buddies, userAnonymizedId, devices));
}

private UserDto(UUID id, Optional<LocalDate> appLastOpenedDate, String mobileNumber, boolean isConfirmed,
Expand Down Expand Up @@ -197,7 +197,7 @@ public static UserDto createInstance(User userEntity, UserAnonymizedDto userAnon
userAnonymizedDto.getLastMonitoredActivityDate(), userEntity.getFirstName(), userEntity.getLastName(),
CryptoSession.getCurrent().getKeyString(), userEntity.getNickname(), userEntity.getUserPhotoId(),
userEntity.getMobileNumber(), userEntity.isMobileNumberConfirmed(), userEntity.isCreatedOnBuddyRequest(),
userEntity.getNamedMessageSourceId(), userEntity.getAnonymousMessageSourceId(), userAnonymizedDto.getGoals(),
userEntity.getNamedMessageSourceId(), userEntity.getAnonymousMessageSourceId(), userAnonymizedDto.getGoalsIncludingHistoryItems(),
buddies, userEntity.getUserAnonymizedId(),
userEntity.getDevices().stream().map(d -> UserDeviceDto.createInstance(userAnonymizedDto, d,
userAnonymizedDto.getDeviceAnonymized(d.getDeviceAnonymizedId()))).collect(Collectors.toSet()));
Expand Down

0 comments on commit e7ea610

Please sign in to comment.