Skip to content

BI-1855 - Create germplasm list not working #281

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

Merged
nickpalladino merged 15 commits into from
Aug 18, 2023
Merged

BI-1855 - Create germplasm list not working #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
fc23e97
Updated logic to allow empty pedigrees and only require gids for updates
nickpalladino Aug 2, 2023
153d204
Logic updates
nickpalladino Aug 3, 2023
bc885a6
refactoring some of the logic in GermplasmProcessor
timparsons Aug 4, 2023
ce4bee7
Fix missing gid message formatting
nickpalladino Aug 7, 2023
f22046a
Updated hasPedigree to check for unknown being true, not just keys
nickpalladino Aug 7, 2023
4f43283
Update to make sure existing brapi object is populated in all cases
nickpalladino Aug 8, 2023
4c74c6d
Workaround for breedbase pedigree strings
nickpalladino Aug 9, 2023
1f4291f
Update breedbase string logic
nickpalladino Aug 9, 2023
09ccfd5
Fixes for issues found testing with Breedbase
nickpalladino Aug 10, 2023
2329ca9
Cleanup some comments
nickpalladino Aug 10, 2023
4bd3006
Null fix
nickpalladino Aug 15, 2023
22bc163
Removed unused import
nickpalladino Aug 16, 2023
21eba3d
Removed unused method
nickpalladino Aug 16, 2023
33a38c9
Added comments
nickpalladino Aug 17, 2023
f0f7ec4
Update logic for filtering missing gids
nickpalladino Aug 18, 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
5 changes: 3 additions & 2 deletions src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.breedinginsight.api.model.v1.validators.SearchValid;
import org.breedinginsight.brapi.v1.controller.BrapiVersion;
import org.breedinginsight.brapi.v1.model.request.query.BrapiQuery;
import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields;
import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO;
import org.breedinginsight.brapi.v2.model.request.query.GermplasmQuery;
import org.breedinginsight.utilities.Utilities;
Expand Down Expand Up @@ -223,14 +224,14 @@ public HttpResponse<BrAPIGermplasmPedigreeResponse> getGermplasmPedigreeInfo(
response = pedigreeResponse.getBody();

//Add nodes for unknown parents if applicable
if (germplasm.getAdditionalInfo().has("femaleParentUnknown") && germplasm.getAdditionalInfo().get("femaleParentUnknown").getAsBoolean()) {
if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.FEMALE_PARENT_UNKNOWN) && germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.FEMALE_PARENT_UNKNOWN).getAsBoolean()) {
BrAPIPedigreeNodeParents unknownFemale = new BrAPIPedigreeNodeParents();
unknownFemale.setGermplasmDbId(germplasm.getGermplasmDbId()+"-F-Unknown");
unknownFemale.setGermplasmName("Unknown");
unknownFemale.setParentType(BrAPIParentType.FEMALE);
returnNode.addParentsItem(unknownFemale);
}
if (germplasm.getAdditionalInfo().has("maleParentUnknown") && germplasm.getAdditionalInfo().get("maleParentUnknown").getAsBoolean()) {
if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.MALE_PARENT_UNKNOWN) && germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.MALE_PARENT_UNKNOWN).getAsBoolean()) {
BrAPIPedigreeNodeParents unknownMale = new BrAPIPedigreeNodeParents();
unknownMale.setGermplasmDbId(germplasm.getGermplasmDbId()+"-M-Unknown");
unknownMale.setGermplasmName("Unknown");
Expand Down
43 changes: 38 additions & 5 deletions src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,10 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
additionalInfo = new JsonObject();
germplasm.setAdditionalInfo(additionalInfo);
}
additionalInfo.addProperty(BrAPIAdditionalInfoFields.GERMPLASM_RAW_PEDIGREE, germplasm.getPedigree());

// TODO: BI-1883 to cleanup this workaround for the pedigree string
String pedigree = processBreedbasePedigree(germplasm.getPedigree());
additionalInfo.addProperty(BrAPIAdditionalInfoFields.GERMPLASM_RAW_PEDIGREE, pedigree);
Comment on lines +192 to +193
Copy link
Contributor

@mlm483 mlm483 Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we want to clean this up eventually once BreedBase is handling pedigree correctly?
Just want to make sure we're documenting future work with Jira stories and TODOs.

Copy link
Member Author

@nickpalladino nickpalladino Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment


String newPedigreeString = "";
String namePedigreeString = "";
Expand All @@ -205,7 +208,7 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
stream().filter(ref -> ref.getReferenceSource().equals(referenceSource)).
map(ref -> ref.getReferenceID()).findFirst().orElse("");
additionalInfo.addProperty(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID, femaleParentAccessionNumber);
} else if (additionalInfo.has("femaleParentUnknown") && additionalInfo.get("femaleParentUnknown").getAsBoolean()) {
} else if (additionalInfo.has(BrAPIAdditionalInfoFields.FEMALE_PARENT_UNKNOWN) && additionalInfo.get(BrAPIAdditionalInfoFields.FEMALE_PARENT_UNKNOWN).getAsBoolean()) {
namePedigreeString = "Unknown";
}
}
Expand All @@ -221,7 +224,7 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
}
}
//Add Unknown germplasm for display
if (additionalInfo.has("maleParentUnknown") && additionalInfo.get("maleParentUnknown").getAsBoolean()) {
if (additionalInfo.has(BrAPIAdditionalInfoFields.MALE_PARENT_UNKNOWN) && additionalInfo.get(BrAPIAdditionalInfoFields.MALE_PARENT_UNKNOWN).getAsBoolean()) {
namePedigreeString += "/Unknown";
}
//For use in individual germplasm display
Expand All @@ -238,6 +241,34 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
return programGermplasmMap;
}

// TODO: hack for now, probably should update breedbase
// Made a JIRA card BI-1883 for this
// Breedbase will return NA/NA for no pedigree or NA/father, mother/NA
// strip NAs before saving RAW_PEDIGREE, if there was a germplasm with name NA it would be in format NA [program key]
// so that case should be ok if we just strip NA/NA, NA/, or /NA<\0>
private String processBreedbasePedigree(String pedigree) {

if (pedigree != null) {
if (pedigree.equals("NA/NA")) {
return "";
}

// Technically processGermplasmForDisplay should handle ok without stripping these NAs but will strip anyways
// for consistency.
// We only allow the /NA case for single parent as we require a female parent in the pedigree
// keep the leading slash, will be handled by processGermplasmForDisplay
if (pedigree.endsWith("/NA")) {
return pedigree.substring(0, pedigree.length()-2);
}

// shouldn't have this case in our data but just in case
if (pedigree.startsWith("NA/")) {
return pedigree.substring(2);
}
}
return pedigree;
}

public List<BrAPIGermplasm> createBrAPIGermplasm(List<BrAPIGermplasm> postBrAPIGermplasmList, UUID programId, ImportUpload upload) {
GermplasmApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), GermplasmApi.class);
var program = programDAO.fetchOneById(programId);
Expand All @@ -262,8 +293,10 @@ public List<BrAPIGermplasm> updateBrAPIGermplasm(List<BrAPIGermplasm> putBrAPIGe
try {
if (!putBrAPIGermplasmList.isEmpty()) {
postFunction = () -> {
List<BrAPIGermplasm> postResponse = putGermplasm(putBrAPIGermplasmList, api);
return processGermplasmForDisplay(postResponse, program.getKey());
putGermplasm(putBrAPIGermplasmList, api);
// Need all program germplasm for processGermplasmForDisplay parents pedigree
List<BrAPIGermplasm> germplasm = getRawGermplasm(programId);
return processGermplasmForDisplay(germplasm, program.getKey());
};
}
return programGermplasmCache.post(programId, postFunction);
Expand Down
46 changes: 26 additions & 20 deletions src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.apache.commons.lang3.StringUtils;
import org.brapi.v2.model.BrAPIExternalReference;
import org.brapi.v2.model.core.BrAPIListTypes;
import org.brapi.v2.model.core.request.BrAPIListNewRequest;
Expand All @@ -30,7 +31,6 @@
import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields;
import org.breedinginsight.brapps.importer.model.config.*;
import org.breedinginsight.brapps.importer.services.ExternalReferenceSource;
import org.breedinginsight.dao.db.tables.pojos.BreedingMethodEntity;
import org.breedinginsight.dao.db.tables.pojos.ProgramBreedingMethodEntity;
import org.breedinginsight.model.Program;
import org.breedinginsight.model.User;
Expand Down Expand Up @@ -156,12 +156,22 @@ public static String constructGermplasmListName(String listName, Program program
return String.format("%s [%s-germplasm]", listName, program.getKey());
}

public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID listId, boolean commit) {
public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID listId, boolean commit, boolean updatePedigree) {

germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID, getFemaleParentDBID());
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_GID, getMaleParentDBID());
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_ENTRY_NO, getFemaleParentEntryNo());
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_ENTRY_NO, getMaleParentEntryNo());
if (updatePedigree) {
if (!StringUtils.isBlank(getFemaleParentDBID())) {
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID, getFemaleParentDBID());
}
if (!StringUtils.isBlank(getMaleParentDBID())) {
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_GID, getMaleParentDBID());
}
if (!StringUtils.isBlank(getFemaleParentEntryNo())) {
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_ENTRY_NO, getFemaleParentEntryNo());
}
if (!StringUtils.isBlank(getMaleParentEntryNo())) {
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_ENTRY_NO, getMaleParentEntryNo());
}
}

// Append synonyms to germplasm that don't already exist
// Synonym comparison is based on name and type
Expand All @@ -178,7 +188,12 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID

// Add germplasm to the new list
JsonObject listEntryNumbers = germplasm.getAdditionalInfo().getAsJsonObject(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS);
if(listEntryNumbers == null) {
listEntryNumbers = new JsonObject();
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS, listEntryNumbers);
}
listEntryNumbers.addProperty(listId.toString(), entryNo);
germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER, entryNo); //so the preview UI shows correctly

// TODO: figure out why clear this out: brapi-server
germplasm.setBreedingMethodDbId(null);
Expand All @@ -203,20 +218,11 @@ public void setUpdateCommitFields(BrAPIGermplasm germplasm, String programKey) {
}
}

public boolean pedigreesEqual(BrAPIGermplasm brAPIGermplasm) {
JsonElement femaleGid = brAPIGermplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID);
String brapiFemaleGid = femaleGid != null ? femaleGid.getAsString() : null;
JsonElement maleGid = brAPIGermplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_GID);
String brapiMaleGid = maleGid != null ? maleGid.getAsString() : null;
JsonElement femaleEntryNo = brAPIGermplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_ENTRY_NO);
String brapiFemaleEntryNo = femaleEntryNo != null ? femaleEntryNo.getAsString() : null;
JsonElement maleEntryNo = brAPIGermplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_ENTRY_NO);
String brapiMaleEntryNo = maleEntryNo != null ? maleEntryNo.getAsString() : null;

return ((getFemaleParentDBID() == null && brapiFemaleGid == null) || (getFemaleParentDBID() != null && getFemaleParentDBID().equals(brapiFemaleGid))) &&
((getMaleParentDBID() == null && brapiMaleGid == null) || (getMaleParentDBID() != null && getMaleParentDBID().equals(brapiMaleGid))) &&
((getFemaleParentEntryNo() == null && brapiFemaleEntryNo == null) || (getFemaleParentEntryNo() != null && getFemaleParentEntryNo().equals(brapiFemaleEntryNo))) &&
((getMaleParentEntryNo() == null && brapiMaleEntryNo == null) || (getMaleParentEntryNo() != null && getMaleParentEntryNo().equals(brapiMaleEntryNo)));
public boolean pedigreeExists() {
return StringUtils.isNotBlank(getFemaleParentDBID()) ||
StringUtils.isNotBlank(getMaleParentDBID()) ||
StringUtils.isNotBlank(getFemaleParentEntryNo()) ||
StringUtils.isNotBlank(getMaleParentEntryNo());
}

public BrAPIGermplasm constructBrAPIGermplasm(ProgramBreedingMethodEntity breedingMethod, User user, UUID listId) {
Expand Down
Loading