Skip to content

Commit

Permalink
Allow unlaunch and Fix Potential Data Issue (#148)
Browse files Browse the repository at this point in the history 
* moved immutable field checks to service layer

* updates to tests

* reverted to bson update instead of panache

* remove commented code
  • Loading branch information
@dwasinge
dwasinge authored Jun 9, 2021
1 parent f2ad21a commit 9bb491e
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 59 deletions.
23 changes: 4 additions & 19 deletions src/main/java/com/redhat/labs/lodestar/repository/EngagementRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
import static com.mongodb.client.model.Filters.regex;
import static com.mongodb.client.model.Projections.exclude;
import static com.mongodb.client.model.Projections.include;
import static com.mongodb.client.model.Updates.combine;
import static com.mongodb.client.model.Updates.set;
import static com.mongodb.client.model.Updates.combine;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -61,10 +60,6 @@ public class EngagementRepository implements PanacheMongoRepository<Engagement>
private static final String ARTIFACTS = "artifacts";
private static final String ARTIFACTS_TYPE = new StringBuilder(ARTIFACTS).append(".").append(TYPE).toString();
private static final String COUNT = "count";
private static final String LAUNCH = "launch";

private static final List<String> IMMUTABLE_FIELDS = new ArrayList<>(
Arrays.asList("uuid", "mongoId", "projectId", "creationDetails", "status", "commits", LAUNCH));

private ObjectMapper objectMapper = new ObjectMapper();

Expand Down Expand Up @@ -135,15 +130,13 @@ public Optional<Engagement> setCommits(String uuid, List<Commit> commits) {
*
* @param replacement
* @param lastUpdate
* @param skipLaunch
* @return
*/
public Optional<Engagement> updateEngagementIfLastUpdateMatched(Engagement toUpdate, String lastUpdate,
Boolean skipLaunch) {
public Optional<Engagement> updateEngagement(Engagement toUpdate, String lastUpdate) {

// create the bson for filter and update
Bson filter = createFilterForEngagement(toUpdate, lastUpdate);
Bson update = createUpdateDocument(toUpdate, skipLaunch);
Bson update = createUpdateDocument(toUpdate);

FindOneAndUpdateOptions optionAfter = new FindOneAndUpdateOptions().returnDocument(ReturnDocument.AFTER);

Expand Down Expand Up @@ -513,10 +506,9 @@ private Bson createFilterForEngagement(Engagement engagement, String lastUpdate)
* {@link Engagement}.
*
* @param engagement
* @param skipLaunch
* @return
*/
private Bson createUpdateDocument(Engagement engagement, boolean skipLaunch) {
private Bson createUpdateDocument(Engagement engagement) {

Bson updates = null;

Expand All @@ -525,13 +517,6 @@ private Bson createUpdateDocument(Engagement engagement, boolean skipLaunch) {
};
Map<String, Object> fieldMap = objectMapper.convertValue(engagement, typeRef);

// remove values that should not be updated
IMMUTABLE_FIELDS.forEach(f -> {
if (!f.equals(LAUNCH) || skipLaunch) {
fieldMap.remove(f);
}
});

// add a set for each field in the update
for (Entry<String, Object> entry : fieldMap.entrySet()) {

Expand Down
71 changes: 41 additions & 30 deletions src/main/java/com/redhat/labs/lodestar/service/EngagementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.redhat.labs.lodestar.model.Artifact;
import com.redhat.labs.lodestar.model.Category;
Expand Down Expand Up @@ -202,13 +201,12 @@ public Engagement update(Engagement engagement) {
() -> new WebApplicationException("no engagement found, use POST to create", HttpStatus.SC_NOT_FOUND));

String currentLastUpdated = engagement.getLastUpdate();

validateHostingEnvironments(engagement.getHostingEnvironments());
validateSubdomainOnUpdate(engagement);
validateCustomerAndProjectNames(engagement, existing);
setBeforeUpdate(engagement, existing);

boolean skipLaunch = skipLaunch(existing);

// create copy to send to git api
Engagement copy = clone(engagement);

Expand All @@ -218,7 +216,7 @@ public Engagement update(Engagement engagement) {
engagement.getEngagementUsers().stream().forEach(u -> u.setReset(false));
}

Engagement updated = repository.updateEngagementIfLastUpdateMatched(engagement, currentLastUpdated, skipLaunch)
Engagement updated = repository.updateEngagement(engagement, currentLastUpdated)
.orElseThrow(() -> new WebApplicationException(
"Failed to modify engagement because request contained stale data. Please refresh and try again.",
HttpStatus.SC_CONFLICT));
Expand Down Expand Up @@ -389,6 +387,8 @@ public Optional<Engagement> getBySubdomain(String subdomain) {
*/
void setBeforeUpdate(Engagement engagement, Engagement existing) {

setImmutableFieldsOnUpdate(engagement, existing);

setLastUpdate(engagement);

// create new or use existing uuids for users
Expand All @@ -415,6 +415,35 @@ void setBeforeUpdate(Engagement engagement, Engagement existing) {

}

/**
* Uses values from the existing {@link Engagement} to set fields that should be
* immutable.
*
* @param engagement
* @param existing
*/
void setImmutableFieldsOnUpdate(Engagement engagement, Engagement existing) {

// uuid
engagement.setUuid(existing.getUuid());

// mongo id
engagement.setMongoId(existing.getMongoId());

// project id
engagement.setProjectId(existing.getProjectId());

// creation details
engagement.setCreationDetails(existing.getCreationDetails());

// status
engagement.setStatus(existing.getStatus());

// commits
engagement.setCommits(existing.getCommits());

}

/**
* Updates the UUID and/or timestamps if the {@link Artifact}, {@link Category},
* {@link UseCase}, {@link Score}, or {@link HostingEnvironment} is new or has
Expand Down Expand Up @@ -535,17 +564,6 @@ void setLastUpdate(Engagement engagement) {
engagement.setLastUpdate(getZuluTimeAsString());
}

/**
* Returns true if the {@link Launch} data is present on the {@link Engagement}.
* Otherwise, false.
*
* @param engagement
* @return
*/
boolean skipLaunch(Engagement engagement) {
return (null != engagement.getLaunch());
}

/**
* Sets the project ID for the {@link Engagement} with the matching UUID.
*
Expand Down Expand Up @@ -640,19 +658,19 @@ public Engagement getByCustomerAndProjectName(String customerName, String projec
"no engagement found with customer:project " + customerName + ":" + projectName,
HttpStatus.SC_NOT_FOUND));
}

public Map<EngagementState, Integer> getEngagementCountByStatus(LocalDateTime currentTime) {

List<Engagement> engagementList = repository.listAll();
Map<EngagementState, Integer> statusCounts = new EnumMap<>(EngagementState.class);

for (Engagement engagement : engagementList) {
EngagementState state = engagement.getEngagementCurrentState(currentTime);
EngagementState state = engagement.getEngagementCurrentState(currentTime);

int count = statusCounts.containsKey(state) ? statusCounts.get(state) + 1 : 1;
statusCounts.put(state, count);
}

statusCounts.put(EngagementState.ANY, engagementList.size());

return statusCounts;
Expand Down Expand Up @@ -1021,7 +1039,7 @@ public boolean persistEngagementIfNotFound(Engagement engagement) {

engagement.setLastUpdate(getZuluTimeAsString());
repository.persist(engagement);

return true;

}
Expand Down Expand Up @@ -1090,14 +1108,7 @@ String getZuluTimeAsString() {
* @return
*/
Engagement clone(Engagement toClone) {

try {
return objectMapper.readValue(objectMapper.writeValueAsString(toClone), Engagement.class);
} catch (JsonProcessingException e) {
throw new WebApplicationException("failed to create engagement for event. " + toClone,
HttpStatus.SC_INTERNAL_SERVER_ERROR);
}

return jsonb.fromJson(jsonb.toJson(toClone), Engagement.class);
}

}
8 changes: 4 additions & 4 deletions src/test/java/com/redhat/labs/lodestar/resource/EngagementResourceUpdateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void testPutEngagementWithAuthAndRoleSuccess() throws Exception {
toUpdate.setDescription("testing");

Mockito.when(eRepository.findByUuid("1234")).thenReturn(Optional.of(persisted));
Mockito.when(eRepository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()), Mockito.any())).thenReturn(Optional.of(toUpdate));
Mockito.when(eRepository.updateEngagement(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()))).thenReturn(Optional.of(toUpdate));

String body = quarkusJsonb.toJson(toUpdate);

Expand Down Expand Up @@ -145,7 +145,7 @@ void testPutEngagementWithAuthAndRoleDuplicateUsers() throws Exception {
toUpdate.setEngagementUsers(Sets.newHashSet(user1, user2, user3));

Mockito.when(eRepository.findByUuid("1234")).thenReturn(Optional.of(persisted));
Mockito.when(eRepository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()), Mockito.any())).thenReturn(Optional.of(toUpdate));
Mockito.when(eRepository.updateEngagement(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()))).thenReturn(Optional.of(toUpdate));

String body = quarkusJsonb.toJson(toUpdate);

Expand Down Expand Up @@ -208,7 +208,7 @@ void testLaunchEngagementWithAuthAndRoleSuccess() throws Exception {
toUpdate.setDescription("testing");

Mockito.when(eRepository.findByUuid("1234")).thenReturn(Optional.of(persisted));
Mockito.when(eRepository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()), Mockito.any())).thenReturn(Optional.of(toUpdate));
Mockito.when(eRepository.updateEngagement(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()))).thenReturn(Optional.of(toUpdate));

String body = quarkusJsonb.toJson(toUpdate);

Expand Down Expand Up @@ -358,7 +358,7 @@ void testPutEngagementByNamesWithAuthAndRoleSuccess() throws Exception {
toUpdate.setDescription("testing");

Mockito.when(eRepository.findByCustomerNameAndProjectName("c1", "e2")).thenReturn(Optional.of(persisted));
Mockito.when(eRepository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()), Mockito.any())).thenReturn(Optional.of(toUpdate));
Mockito.when(eRepository.updateEngagement(Mockito.any(), Mockito.eq(toUpdate.getLastUpdate()))).thenReturn(Optional.of(toUpdate));

String body = quarkusJsonb.toJson(toUpdate);

Expand Down
8 changes: 4 additions & 4 deletions src/test/java/com/redhat/labs/lodestar/service/EngagementServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ void testUpdateSuccessNameChange() {
Mockito.when(repository.findByUuid("1234")).thenReturn(Optional.of(persisted));
Mockito.when(repository.findByCustomerNameAndProjectName("c3", "p3", new FilterOptions()))
.thenReturn(Optional.empty());
Mockito.when(repository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.any(), Mockito.any()))
Mockito.when(repository.updateEngagement(Mockito.any(), Mockito.any()))
.thenReturn(Optional.of(toUpdate));

Engagement updated = service.update(toUpdate);
Expand Down Expand Up @@ -325,7 +325,7 @@ void testUpdateSuccessNoNameChange() throws Exception {
Mockito.when(repository.findByUuid("1234")).thenReturn(Optional.of(persisted));
Mockito.when(repository.findByCustomerNameAndProjectName("c3", "p3", new FilterOptions()))
.thenReturn(Optional.empty());
Mockito.when(repository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.any(), Mockito.any()))
Mockito.when(repository.updateEngagement(Mockito.any(), Mockito.any()))
.thenReturn(Optional.of(toUpdate));

Engagement updated = service.update(toUpdate);
Expand Down Expand Up @@ -836,14 +836,14 @@ void testLaunch() {
persisted.setProjectId(1111);

Mockito.when(repository.findByUuid("1234")).thenReturn(Optional.of(persisted));
Mockito.when(repository.updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.any(), Mockito.any()))
Mockito.when(repository.updateEngagement(Mockito.any(), Mockito.any()))
.thenReturn(Optional.of(toUpdate));

Engagement updated = service.launch(toUpdate);
assertNotNull(updated);
assertNotNull(updated.getLaunch());

Mockito.verify(repository).updateEngagementIfLastUpdateMatched(Mockito.any(), Mockito.any(), Mockito.any());
Mockito.verify(repository).updateEngagement(Mockito.any(), Mockito.any());
Mockito.verify(eventBus).sendAndForget(Mockito.eq(EventType.UPDATE_ENGAGEMENT_EVENT_ADDRESS), Mockito.any());

}
Expand Down
20 changes: 18 additions & 2 deletions src/test/java/com/redhat/labs/lodestar/zrepository/EngagementRepositoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,22 @@ void testSetProjectIdNotFound() {
// set commits
// update engagement if last update matched

@Test
void testUpdateEngagementIfLastUpdateMatchedMultipleFieldUpdate() throws Exception {

Engagement e1 = MockUtils.mockMinimumEngagement("c1", "c2", "1234");
e1.setLastUpdate("value");
repository.persist(e1);

Engagement e2 = MockUtils.cloneEngagement(e1);
e2.setLastUpdate("updated");
e2.setDescription("testing");

Optional<Engagement> optional = repository.updateEngagement(e2, "value");
assertTrue(optional.isPresent());

}

@Test
void testUpdateEngagementIfLastUpdateMatched() throws Exception {

Expand All @@ -140,7 +156,7 @@ void testUpdateEngagementIfLastUpdateMatched() throws Exception {
Engagement e2 = MockUtils.cloneEngagement(e1);
e2.setDescription("testing");

Optional<Engagement> optional = repository.updateEngagementIfLastUpdateMatched(e2, "value", false);
Optional<Engagement> optional = repository.updateEngagement(e2, "value");
assertTrue(optional.isPresent());

}
Expand All @@ -155,7 +171,7 @@ void testUpdateEngagementIfLastUpdateMatchedStale() throws Exception {
Engagement e2 = MockUtils.cloneEngagement(e1);
e2.setDescription("testing");

Optional<Engagement> optional = repository.updateEngagementIfLastUpdateMatched(e2, "value2", false);
Optional<Engagement> optional = repository.updateEngagement(e2, "value2");
assertTrue(optional.isEmpty());

}
Expand Down

0 comments on commit 9bb491e

Please sign in to comment.