Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix grant loader comparison logic #62

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Fix grant loader comparison logic #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ccf05e
Don't print ID differences
rpoet-jh Oct 7, 2023
dc01137
Compare by association ID instead of equals
rpoet-jh Oct 7, 2023
c975eb5
Add tests
rpoet-jh Oct 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions ...nt-data/src/main/java/org/eclipse/pass/support/grant/data/AbstractDefaultPassUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import org.apache.commons.lang3.StringUtils;
import org.eclipse.pass.support.client.PassClient;
Expand Down Expand Up @@ -313,6 +317,24 @@ private void updateGrants(Collection<Map<String, String>> results) {
}
}

/**
* Returns the Pass entity ID of passEntity. This method is null-safe.
* @param passEntity the passEntity
* @return the ID of passEntity or null if passEntity is null or the ID is null
*/
protected String getPassEntityId(PassEntity passEntity) {
return Optional.ofNullable(passEntity).map(PassEntity::getId).orElse(null);
}

/**
* Returns a Set of Pass entity ID of passEntities.
* @param passEntities the list of passEntities
* @return the Set of passEntity IDs
*/
protected Set<String> getPassEntityIds(List<? extends PassEntity> passEntities) {
return passEntities.stream().map(PassEntity::getId).collect(Collectors.toSet());
}

private void updateUsers(Collection<Map<String, String>> results) {

boolean modeChecked = false;
Expand Down
3 changes: 2 additions & 1 deletion ...r/pass-grant-data/src/main/java/org/eclipse/pass/support/grant/data/DifferenceLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ private List<String> getDifference(Object s1, Object s2, String grantId) {
getPassEntityDiffs((PassEntity) value1, (PassEntity) value2, values, field);
} else if (value1 instanceof List || value2 instanceof List) {
getListDiffs((List<PassEntity>) value1, (List<PassEntity>) value2, values, field);
} else if (!Objects.equals(value1, value2)) {
} else if (!field.getName().equals("id")
&& !Objects.equals(value1, value2)) {
values.add(field.getName() + ": " + value1 + " -> " + value2);
}
}
Expand Down
61 changes: 34 additions & 27 deletions ...pass-grant-data/src/main/java/org/eclipse/pass/support/grant/data/JhuPassInitUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package org.eclipse.pass.support.grant.data;

import java.util.HashSet;

import org.eclipse.pass.support.client.model.Grant;
import org.eclipse.pass.support.client.model.User;

Expand Down Expand Up @@ -75,50 +73,59 @@ public Grant updateGrantIfNeeded(Grant system, Grant stored) {
*/

private boolean grantNeedsUpdate(Grant system, Grant stored) {
if (system.getAwardNumber() != null ? !system.getAwardNumber()
.equals(
stored.getAwardNumber()) : stored.getAwardNumber() != null) {
if (system.getAwardNumber() != null
? !system.getAwardNumber().equals(stored.getAwardNumber())
: stored.getAwardNumber() != null) {
return true;
}
if (system.getAwardStatus() != null ? !system.getAwardStatus()
.equals(
stored.getAwardStatus()) : stored.getAwardStatus() != null) {
if (system.getAwardStatus() != null
? !system.getAwardStatus().equals(stored.getAwardStatus())
: stored.getAwardStatus() != null) {
return true;
}
if (system.getLocalKey() != null ? !system.getLocalKey()
.equals(stored.getLocalKey()) : stored.getLocalKey() != null) {
if (system.getLocalKey() != null
? !system.getLocalKey().equals(stored.getLocalKey())
: stored.getLocalKey() != null) {
return true;
}
if (system.getProjectName() != null ? !system.getProjectName()
.equals(
stored.getProjectName()) : stored.getProjectName() != null) {
if (system.getProjectName() != null
? !system.getProjectName().equals(stored.getProjectName())
: stored.getProjectName() != null) {
return true;
}
if (system.getPrimaryFunder() != null ? !system.getPrimaryFunder().equals(
stored.getPrimaryFunder()) : stored.getPrimaryFunder() != null) {
if (system.getPrimaryFunder() != null
? !system.getPrimaryFunder().getId().equals(getPassEntityId(stored.getPrimaryFunder()))
: stored.getPrimaryFunder() != null) {
return true;
}
if (system.getDirectFunder() != null ? !system.getDirectFunder().equals(
stored.getDirectFunder()) : stored.getDirectFunder() != null) {
if (system.getDirectFunder() != null
? !system.getDirectFunder().getId().equals(getPassEntityId(stored.getDirectFunder()))
: stored.getDirectFunder() != null) {
return true;
}
if (system.getPi() != null ? !system.getPi().equals(stored.getPi()) : stored.getPi() != null) {
if (system.getPi() != null
? !system.getPi().getId().equals(getPassEntityId(stored.getPi()))
: stored.getPi() != null) {
return true;
}
if (system.getCoPis() != null ? !new HashSet(system.getCoPis()).equals(
new HashSet(stored.getCoPis())) : stored.getCoPis() != null) {
if (system.getCoPis() != null
? !getPassEntityIds(system.getCoPis()).equals(getPassEntityIds(stored.getCoPis()))
: stored.getCoPis() != null) {
return true;
}
if (system.getAwardDate() != null ? system.getAwardDate()
.isBefore(stored.getAwardDate()) : stored.getAwardDate() != null) {
if (system.getAwardDate() != null
? system.getAwardDate().isBefore(stored.getAwardDate())
: stored.getAwardDate() != null) {
return true;
}
if (system.getStartDate() != null ? system.getStartDate()
.isBefore(stored.getStartDate()) : stored.getStartDate() != null) {
if (system.getStartDate() != null
? system.getStartDate().isBefore(stored.getStartDate())
: stored.getStartDate() != null) {
return true;
}
if (system.getEndDate() != null ? system.getEndDate()
.isAfter(stored.getEndDate()) : stored.getEndDate() != null) {
if (system.getEndDate() != null
? system.getEndDate().isAfter(stored.getEndDate())
: stored.getEndDate() != null) {
return true;
}
return false;
Expand All @@ -132,7 +139,7 @@ private boolean grantNeedsUpdate(Grant system, Grant stored) {
* @return the Grant object which represents the Pass object, with any new information from COEUS merged in
*/
private Grant updateGrant(Grant system, Grant stored) {
differenceLogger.log(system, stored);
differenceLogger.log(stored, system);
stored.setAwardNumber(system.getAwardNumber());
stored.setAwardStatus(system.getAwardStatus());
stored.setLocalKey(system.getLocalKey());
Expand Down
29 changes: 18 additions & 11 deletions ...der/pass-grant-data/src/main/java/org/eclipse/pass/support/grant/data/JhuPassUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,14 @@ private boolean funderNeedsUpdate(Funder system, Funder stored) {
if (system.getName() != null && !system.getName().equals(stored.getName())) {
return true;
}
if (system.getLocalKey() != null ? !system.getLocalKey()
.equals(stored.getLocalKey()) : stored.getLocalKey() != null) {
if (system.getLocalKey() != null
? !system.getLocalKey().equals(stored.getLocalKey())
: stored.getLocalKey() != null) {
return true;
}
if (system.getPolicy() != null ? !system.getPolicy().equals(stored.getPolicy()) : stored.getPolicy() != null) {
if (system.getPolicy() != null
? !system.getPolicy().getId().equals(getPassEntityId(stored.getPolicy()))
: stored.getPolicy() != null) {
return true;
}
return false;
Expand Down Expand Up @@ -244,20 +247,24 @@ private User updateUser(User system, User stored) {
* @return a boolean which asserts whether the two supplied Grants are "COEUS equal"
*/
private boolean grantNeedsUpdate(Grant system, Grant stored) {
if (system.getAwardStatus() != null ? !system.getAwardStatus()
.equals(
stored.getAwardStatus()) : stored.getAwardStatus() != null) {
if (system.getAwardStatus() != null
? !system.getAwardStatus().equals(stored.getAwardStatus())
: stored.getAwardStatus() != null) {
return true;
}
if (system.getPi() != null ? !system.getPi().equals(stored.getPi()) : stored.getPi() != null) {
if (system.getPi() != null
? !system.getPi().getId().equals(getPassEntityId(stored.getPi()))
: stored.getPi() != null) {
return true;
}
if (system.getCoPis() != null ? !new HashSet(system.getCoPis()).equals(
new HashSet(stored.getCoPis())) : stored.getCoPis() != null) {
if (system.getCoPis() != null
? !getPassEntityIds(system.getCoPis()).equals(getPassEntityIds(stored.getCoPis()))
: stored.getCoPis() != null) {
return true;
}
if (system.getEndDate() != null ? system.getEndDate()
.isAfter(stored.getEndDate()) : stored.getEndDate() != null) {
if (system.getEndDate() != null
? system.getEndDate().isAfter(stored.getEndDate())
: stored.getEndDate() != null) {
return true;
}
return false;
Expand Down
120 changes: 96 additions & 24 deletions ...ss-grant-data/src/test/java/org/eclipse/pass/support/grant/data/JhuPassInitUpdaterIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,18 @@
import static org.eclipse.pass.support.grant.data.JhuPassUpdater.JHED_LOCATOR_ID;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.apache.commons.lang3.reflect.FieldUtils;
import org.eclipse.pass.support.client.PassClient;
import org.eclipse.pass.support.client.PassClientResult;
import org.eclipse.pass.support.client.PassClientSelector;
Expand All @@ -56,9 +61,12 @@
import org.eclipse.pass.support.client.model.Grant;
import org.eclipse.pass.support.client.model.Policy;
import org.eclipse.pass.support.client.model.User;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;

@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class JhuPassInitUpdaterIT {

private final String[] grantAwardNumber = {"A10000000", "A10000001", "A10000002"};
Expand All @@ -77,29 +85,11 @@ public class JhuPassInitUpdaterIT {
private final String[] userLastName = {"Melon", "Einstein", "Jones"};
private final String[] userEmail = {"[email protected]", "[email protected]", "[email protected]"};

private String primaryFunderPolicyUriString;
private String directFunderPolicyUriString;

private final String grantIdPrefix = "johnshopkins.edu:grant:";
//private final String funderIdPrefix = "johnshopkins.edu:funder:";

private final PassClient passClient = PassClient.newInstance();

@BeforeEach
public void setup() throws IOException {
Policy policy1 = new Policy();
policy1.setTitle("Primary Policy");
policy1.setDescription("MOO");
passClient.createObject(policy1);
primaryFunderPolicyUriString = policy1.getId();

Policy policy2 = new Policy();
policy2.setTitle("Direct Policy");
policy2.setDescription("MOO");
passClient.createObject(policy2);
directFunderPolicyUriString = policy2.getId();
}

/**
* we put an initial award for a grant into PASS, then simulate a pull of all records related
* to this grant from the Beginning of Time (including records which created the initial object)
Expand All @@ -110,6 +100,7 @@ public void setup() throws IOException {
*
*/
@Test
@Order(1)
public void processInitGrantIT() throws IOException {
// GIVEN
List<Map<String, String>> resultSet = new ArrayList<>();
Expand Down Expand Up @@ -187,6 +178,59 @@ public void processInitGrantIT() throws IOException {
assertTrue(updatePassGrant.getCoPis().contains(user2));//Jones
}

@Test
@Order(2)
public void processInitGrantIT_DoesNotUpdateWithNoChange() throws IOException, IllegalAccessException {
// GIVEN
Map<String, String> piRecord0 = makeRowMap(0, 0, "P");
Map<String, String> coPiRecord0 = makeRowMap(0, 1, "C");
Map<String, String> piRecord1 = makeRowMap(1, 0, "P");
Map<String, String> coPiRecord1 = makeRowMap(1, 1, "C");
Map<String, String> newCoPiRecord1 = makeRowMap(1, 2, "C");
Map<String, String> piRecord2 = makeRowMap(2, 1, "P");

//in the initial pull, we will find all of the records (check?)
List<Map<String, String>> resultSet = new ArrayList<>();
resultSet.add(piRecord0);
resultSet.add(coPiRecord0);
resultSet.add(piRecord1);
resultSet.add(coPiRecord1);
resultSet.add(newCoPiRecord1);
resultSet.add(piRecord2);

PassClient spyPassClient = spy(passClient);
JhuPassInitUpdater passUpdater = new JhuPassInitUpdater();
FieldUtils.writeField(passUpdater, "passClient", spyPassClient, true);

// WHEN
passUpdater.updatePass(resultSet, "grant");

// THEN
PassClientSelector<Grant> grantSelector = new PassClientSelector<>(Grant.class);
grantSelector.setFilter(RSQL.equals("localKey", grantIdPrefix + grantLocalKey[2]));
grantSelector.setInclude("primaryFunder", "directFunder", "pi", "coPis");
PassClientResult<Grant> resultGrant = passClient.selectObjects(grantSelector);
assertEquals(1, resultGrant.getTotal());
Grant passGrant = resultGrant.getObjects().get(0);

User user0 = getVerifiedUser(0);
User user1 = getVerifiedUser(1);
User user2 = getVerifiedUser(2);

verify(spyPassClient, times(0)).updateObject(any());
assertEquals(grantAwardNumber[0], passGrant.getAwardNumber());//initial
assertEquals(AwardStatus.ACTIVE, passGrant.getAwardStatus());
assertEquals(grantIdPrefix + grantLocalKey[0], passGrant.getLocalKey());
assertEquals(grantProjectName[0], passGrant.getProjectName());//initial
assertEquals(createZonedDateTime(grantAwardDate[0]), passGrant.getAwardDate());//initial
assertEquals(createZonedDateTime(grantStartDate[0]), passGrant.getStartDate());//initial
assertEquals(createZonedDateTime(grantEndDate[2]), passGrant.getEndDate());//latest
assertEquals(user1, passGrant.getPi());//Einstein
assertEquals(2, passGrant.getCoPis().size());
assertTrue(passGrant.getCoPis().contains(user0));//Melon
assertTrue(passGrant.getCoPis().contains(user2));//Jones
}

private User getVerifiedUser(int userIndex) throws IOException {
PassClientSelector<User> userSelector = new PassClientSelector<>(User.class);
userSelector.setFilter(RSQL.hasMember("locatorIds", EMPLOYEE_LOCATOR_ID + userEmployeeId[userIndex]));
Expand All @@ -207,7 +251,7 @@ private User getVerifiedUser(int userIndex) throws IOException {
* @param abbrRole the role: Pi ("P") or co-pi (C" or "K")
* @return the row map for the record
*/
private Map<String, String> makeRowMap(int iteration, int user, String abbrRole) {
private Map<String, String> makeRowMap(int iteration, int user, String abbrRole) throws IOException {
Map<String, String> rowMap = new HashMap<>();
rowMap.put(C_GRANT_AWARD_NUMBER, grantAwardNumber[iteration]);
rowMap.put(C_GRANT_AWARD_STATUS, "Active");
Expand All @@ -217,9 +261,9 @@ private Map<String, String> makeRowMap(int iteration, int user, String abbrRole)
rowMap.put(C_GRANT_START_DATE, grantStartDate[iteration]);
rowMap.put(C_GRANT_END_DATE, grantEndDate[iteration]);

rowMap.put(C_DIRECT_FUNDER_LOCAL_KEY, "20000000");
rowMap.put(C_DIRECT_FUNDER_LOCAL_KEY, "30000000");
rowMap.put(C_DIRECT_FUNDER_NAME, "Enormous State University");
rowMap.put(C_PRIMARY_FUNDER_LOCAL_KEY, "20000001");
rowMap.put(C_PRIMARY_FUNDER_LOCAL_KEY, "30000001");
rowMap.put(C_PRIMARY_FUNDER_NAME, "J L Gotrocks Foundation");

rowMap.put(C_USER_FIRST_NAME, userFirstName[user]);
Expand All @@ -232,10 +276,38 @@ private Map<String, String> makeRowMap(int iteration, int user, String abbrRole)
rowMap.put(C_UPDATE_TIMESTAMP, grantUpdateTimestamp[iteration]);
rowMap.put(C_ABBREVIATED_ROLE, abbrRole);

rowMap.put(C_DIRECT_FUNDER_POLICY, directFunderPolicyUriString);
rowMap.put(C_PRIMARY_FUNDER_POLICY, primaryFunderPolicyUriString);
rowMap.put(C_DIRECT_FUNDER_POLICY, getDirectFunderPolicyId());
rowMap.put(C_PRIMARY_FUNDER_POLICY, getPrimaryFunderPolicyId());

return rowMap;
}

private String getPrimaryFunderPolicyId() throws IOException {
PassClientSelector<Policy> policySelector = new PassClientSelector<>(Policy.class);
policySelector.setFilter(RSQL.equals("title", "Init Primary Funder Policy"));
PassClientResult<Policy> resultPolicy = passClient.selectObjects(policySelector);
if (resultPolicy.getObjects().isEmpty()) {
Policy policy1 = new Policy();
policy1.setTitle("Init Primary Funder Policy");
policy1.setDescription("BAA");
passClient.createObject(policy1);
return policy1.getId();
}
return resultPolicy.getObjects().get(0).getId();
}

private String getDirectFunderPolicyId() throws IOException {
PassClientSelector<Policy> policySelector = new PassClientSelector<>(Policy.class);
policySelector.setFilter(RSQL.equals("title", "Init Direct Funder Policy"));
PassClientResult<Policy> resultPolicy = passClient.selectObjects(policySelector);
if (resultPolicy.getObjects().isEmpty()) {
Policy policy1 = new Policy();
policy1.setTitle("Init Direct Funder Policy");
policy1.setDescription("BAA");
passClient.createObject(policy1);
return policy1.getId();
}
return resultPolicy.getObjects().get(0).getId();
}

}
Loading
Loading