From 82039615dd9398e7613170c39cf7dd5e92f17962 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Mon, 17 Feb 2025 15:49:59 -0500 Subject: [PATCH 01/18] Remove flushalls, batch pedigree lookups from germplasm --- .../germ/GermplasmApiController.java | 2 + .../service/core/CropService.java | 2 +- .../service/germ/GermplasmService.java | 34 +++++- .../service/germ/PedigreeService.java | 104 ++++++++++++++---- 4 files changed, 115 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java b/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java index 9f1ed1ae..09d279cb 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java @@ -199,7 +199,9 @@ public ResponseEntity germplasmPost(@RequestBody List data = germplasmService.saveGermplasm(body); + // TODO: Add short-circuit if no ped connections are sent. pedigreeService.updateGermplasmPedigree(data); return responseOK(new GermplasmListResponse(), new GermplasmListResponseResult(), data); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java index a3d2d450..5419ea03 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java @@ -68,7 +68,7 @@ public CropEntity saveCropEntity(String commonCropName) throws BrAPIServerExcep if (commonCropName != null) { entity = new CropEntity(); entity.setCropName(commonCropName); - entity = cropRepository.saveAndFlush(entity); + entity = cropRepository.save(entity); } return entity; } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java index 6e4f0e29..c74f375d 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java @@ -473,7 +473,7 @@ public Germplasm updateGermplasm(String germplasmDbId, GermplasmNewRequest body) throws BrAPIServerException { GermplasmEntity entity = getGermplasmEntity(germplasmDbId, HttpStatus.NOT_FOUND); updateEntity(entity, body); - GermplasmEntity savedEntity = germplasmRepository.saveAndFlush(entity); + GermplasmEntity savedEntity = germplasmRepository.save(entity); return convertFromEntity(savedEntity); } @@ -509,7 +509,7 @@ public List saveGermplasm(@Valid List body) thro toSave.add(entity); } // Save batch. - return germplasmRepository.saveAllAndFlush(toSave) + return germplasmRepository.saveAll(toSave) .stream() .map(this::convertFromEntity) .collect(Collectors.toList()); @@ -677,14 +677,42 @@ private void updateSynonymEntities(List synonyms, G } } + public List findByNames(List germplasmNames) + throws BrAPIServerException { + GermplasmSearchRequest request = new GermplasmSearchRequest().germplasmNames(germplasmNames); + Metadata metadata = new Metadata().pagination(new IndexPagination()); + Page page = findGermplasmEntities(request, metadata); + + if (page.hasContent()) { + return page.getContent(); + } + + return null; + } + + // TODO: Add lookupType param to RQ Germplasm which can short-circuit all the lookup logic to only one query here. public GermplasmEntity findByUnknownIdentity(String germplasmStr) throws BrAPIServerException { + + // First, check to see if the str provided is a real UUID. + boolean tryDBIdLookup = true; + try { + UUID germUUID = UUID.fromString(germplasmStr); + } catch (IllegalArgumentException e) { + tryDBIdLookup = false; + } + List germplasmList = Arrays.asList(germplasmStr); Metadata metadata = new Metadata().pagination(new IndexPagination()); // germplasmDbId GermplasmSearchRequest request = new GermplasmSearchRequest().germplasmDbIds(germplasmList); - Page page = findGermplasmEntities(request, metadata); + Page page = Page.empty(); + + if (tryDBIdLookup) { + page = findGermplasmEntities(request, metadata); + } + if (page.hasContent()) { return page.getContent().get(0); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 0921c89d..37f0f827 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -188,7 +188,7 @@ public PedigreeNodeEntity findOrCreatePedigreeNode(String germplasmDbId) throws } else { PedigreeNodeEntity newNode = new PedigreeNodeEntity(); newNode.setGermplasm(germplasm); - node = pedigreeRepository.saveAndFlush(newNode); + node = pedigreeRepository.save(newNode); } } return node; @@ -255,7 +255,8 @@ public List savePedigreeNodes(List request) throws B newEntities.add(entity); } // save all the new nodes without edges - pedigreeRepository.saveAllAndFlush(newEntities); + //TODO: Fix to save nodes and edges at same time + pedigreeRepository.saveAll(newEntities); Map updateRequest = new HashMap<>(); for (PedigreeNode newNode : request) { @@ -281,7 +282,7 @@ public List updatePedigreeNodes(Map request) } } - List savedEntities = pedigreeRepository.saveAllAndFlush(newEntities); + List savedEntities = pedigreeRepository.saveAll(newEntities); List saved = convertFromEntities(savedEntities, new PedigreeSearchRequest().includeParents(true).includeProgeny(true).includeSiblings(true)); return saved; @@ -291,19 +292,24 @@ public void updateGermplasmPedigree(List data) throws BrAPIServerExce List createPedigreeNodes = new ArrayList<>(); Map updatePedigreeNodes = new HashMap<>(); + // TODO: This always returns empty for the germplasm post path, since this method is always passed a newly created germplasm record. Map nodesByGermplasm = getExistingPedigreeNodes( data.stream().map(p -> p.getGermplasmDbId()).collect(Collectors.toList())); - for (Germplasm germplasm : data) { - if (nodesByGermplasm.containsKey(germplasm.getGermplasmDbId())) { - updatePedigreeNodes.put(germplasm.getGermplasmDbId(), convertFromGermplasmToPedigree(germplasm)); - } else { - createPedigreeNodes.add(convertFromGermplasmToPedigree(germplasm)); + if (data.stream().noneMatch(g -> g.getPedigree() != null) || !nodesByGermplasm.isEmpty()) { + for (Germplasm germplasm : data) { + if (nodesByGermplasm.containsKey(germplasm.getGermplasmDbId())) { + updatePedigreeNodes.put(germplasm.getGermplasmDbId(), convertFromGermplasmToPedigree(germplasm)); + } else { + createPedigreeNodes.add(convertFromGermplasmToPedigree(germplasm)); + } } - } - savePedigreeNodes(createPedigreeNodes); - updatePedigreeNodes(updatePedigreeNodes); + savePedigreeNodes(createPedigreeNodes); + updatePedigreeNodes(updatePedigreeNodes); + } else { + savePedigreeNodes(convertFromGermplasmToPedigreeBatchUsingNames(data)); + } } @@ -507,7 +513,6 @@ private void updateEntityWithEdges(PedigreeNodeEntity entity, PedigreeNode node) if (!edgeIdsToDelete.isEmpty()) { pedigreeEdgeRepository.deleteAllByIdInBatch(edgeIdsToDelete); - pedigreeEdgeRepository.flush(); } for (PedigreeNodeParents parentNode : node.getParents()) { @@ -530,7 +535,6 @@ private void updateEntityWithEdges(PedigreeNodeEntity entity, PedigreeNode node) if (!edgeIdsToDelete.isEmpty()) { pedigreeEdgeRepository.deleteAllByIdInBatch(edgeIdsToDelete); - pedigreeEdgeRepository.flush(); } for (PedigreeNodeParents childNode : node.getProgeny()) { @@ -541,6 +545,48 @@ private void updateEntityWithEdges(PedigreeNodeEntity entity, PedigreeNode node) } } + public List convertFromGermplasmToPedigreeBatchUsingNames(List germplasms) + throws BrAPIServerException{ + List result = new ArrayList(); + + Map germsByPedigreeMother = new HashMap<>(); + Map germsByPedigreeFather = new HashMap<>(); + + for (Germplasm germplasm : germplasms) { + List pedigreeList = Arrays.asList(germplasm.getPedigree().split("/")); + + if (!pedigreeList.isEmpty()) { + germsByPedigreeMother.put(germplasm, pedigreeList.get(0)); + + if (pedigreeList.size() > 1) { + germsByPedigreeFather.put(germplasm, pedigreeList.get(1)); + } + } + } + + List motherGerms = germplasmService.findByNames(new ArrayList<>(germsByPedigreeMother.values())); + List fatherGerms = germplasmService.findByNames(new ArrayList<>(germsByPedigreeFather.values())); + + for (Germplasm germplasm : germplasms) { + GermplasmEntity motherGerm = null; + GermplasmEntity fatherGerm = null; + + if (germsByPedigreeMother.containsKey(germplasm)) { + String motherString = germsByPedigreeMother.get(germplasm); + motherGerm = motherGerms.stream().filter(g -> g.getGermplasmName().equals(motherString)).findFirst().orElse(null); + } + + if (germsByPedigreeFather.containsKey(germplasm)) { + String fatherString = germsByPedigreeFather.get(germplasm); + fatherGerm = fatherGerms.stream().filter(g -> g.getGermplasmName().equals(fatherString)).findFirst().orElse(null); + } + + result.add(createPedigreeFromGermplasm(germplasm, motherGerm, fatherGerm)); + } + + return result; + } + public PedigreeNode convertFromGermplasmToPedigree(Germplasm germplasm) throws BrAPIServerException { PedigreeNode node = new PedigreeNode(); @@ -549,15 +595,27 @@ public PedigreeNode convertFromGermplasmToPedigree(Germplasm germplasm) if (germplasm.getPedigree() != null) { pedigreeList = Arrays.asList(germplasm.getPedigree().split("/")); } - Optional motherOpt = Optional.empty(); - Optional fatherOpt = Optional.empty(); + + // TODO: Could split this up into one query in batch that returns a Map of Germplasm to its Nodes. + GermplasmEntity motherGerm = null; + GermplasmEntity fatherGerm = null; if (pedigreeList.size() > 0) { - motherOpt = Optional.ofNullable(germplasmService.findByUnknownIdentity(pedigreeList.get(0))); + motherGerm = germplasmService.findByUnknownIdentity(pedigreeList.get(0)); if (pedigreeList.size() > 1) { - fatherOpt = Optional.ofNullable(germplasmService.findByUnknownIdentity(pedigreeList.get(1))); + fatherGerm = germplasmService.findByUnknownIdentity(pedigreeList.get(1)); } } + return createPedigreeFromGermplasm(germplasm, + motherGerm, + fatherGerm); + } + + public PedigreeNode createPedigreeFromGermplasm(Germplasm germplasm, + GermplasmEntity motherGerm, + GermplasmEntity fatherGerm) { + PedigreeNode node = new PedigreeNode(); + node.setAdditionalInfo(germplasm.getAdditionalInfo()); node.setBreedingMethodDbId(germplasm.getBreedingMethodDbId()); node.setBreedingMethodName(germplasm.getBreedingMethodName()); @@ -568,17 +626,17 @@ public PedigreeNode convertFromGermplasmToPedigree(Germplasm germplasm) node.setGermplasmPUI(germplasm.getGermplasmPUI()); node.setPedigreeString(germplasm.getPedigree()); - if (motherOpt.isPresent()) { + if (motherGerm != null) { PedigreeNodeParents mother = new PedigreeNodeParents(); - mother.setGermplasmDbId(motherOpt.get().getId()); - mother.setGermplasmName(motherOpt.get().getGermplasmName()); + mother.setGermplasmDbId(motherGerm.getId()); + mother.setGermplasmName(motherGerm.getGermplasmName()); mother.setParentType(ParentType.FEMALE); node.addParentsItem(mother); } - if (fatherOpt.isPresent()) { + if (fatherGerm != null) { PedigreeNodeParents father = new PedigreeNodeParents(); - father.setGermplasmDbId(fatherOpt.get().getId()); - father.setGermplasmName(fatherOpt.get().getGermplasmName()); + father.setGermplasmDbId(fatherGerm.getId()); + father.setGermplasmName(fatherGerm.getGermplasmName()); father.setParentType(ParentType.MALE); node.addParentsItem(father); } From accc26f80466c6c0fc77915909e4004480b0e598 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Tue, 25 Feb 2025 12:09:58 -0500 Subject: [PATCH 02/18] Add createEntitiesInBatch method for PedigreeService to optimize batching --- .../service/germ/CrossingProjectService.java | 22 ++++++++ .../service/germ/GermplasmService.java | 13 +++++ .../service/germ/PedigreeService.java | 54 ++++++++++++++++--- 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java index 373dc65f..1e5fff4c 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Optional; +import io.swagger.model.IndexPagination; import jakarta.validation.Valid; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerDbIdNotFoundException; @@ -69,6 +70,27 @@ public List findCrossingProjects(String crossingProjectDbId, St return crossingProjects; } + public List findCrossingProjectsByIds(List crossingProjectIds) + throws BrAPIServerException { + Metadata metadata = new Metadata().pagination(new IndexPagination()); + Pageable pageReq = PagingUtility.getPageRequest(metadata); + + SearchQueryBuilder searchQuery = new SearchQueryBuilder( + CrossingProjectEntity.class); + + if (crossingProjectIds != null && !crossingProjectIds.isEmpty()) { + searchQuery = searchQuery.appendList(crossingProjectIds, "id"); + } + + Page page = crossingProjectRepository.findAllBySearchAndPaginate(searchQuery, pageReq); + + if (page.hasContent()) { + return page.getContent(); + } + + return null; + } + public CrossingProject getCrossingProject(String crossingProjectDbId) throws BrAPIServerException { return convertFromEntity(getCrossingProjectEntity(crossingProjectDbId, HttpStatus.NOT_FOUND)); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java index c74f375d..6f72feb7 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java @@ -690,6 +690,19 @@ public List findByNames(List germplasmNames) return null; } + public List findByIds(List germplasmDbIds) + throws BrAPIServerException { + GermplasmSearchRequest request = new GermplasmSearchRequest().germplasmDbIds(germplasmDbIds); + Metadata metadata = new Metadata().pagination(new IndexPagination()); + Page page = findGermplasmEntities(request, metadata); + + if (page.hasContent()) { + return page.getContent(); + } + + return null; + } + // TODO: Add lookupType param to RQ Germplasm which can short-circuit all the lookup logic to only one query here. public GermplasmEntity findByUnknownIdentity(String germplasmStr) throws BrAPIServerException { diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 37f0f827..cdb4d45b 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -12,6 +12,7 @@ import java.util.Set; import io.swagger.model.IndexPagination; +import org.apache.commons.lang3.tuple.Pair; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerDbIdNotFoundException; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException; import org.brapi.test.BrAPITestServer.model.entity.germ.CrossingProjectEntity; @@ -247,13 +248,8 @@ public List savePedigreeNodes(List request) throws B throw new BrAPIServerException(HttpStatus.BAD_REQUEST, errorMsg); } - List newEntities = new ArrayList<>(); - - for (PedigreeNode node : request) { - PedigreeNodeEntity entity = new PedigreeNodeEntity(); - updateEntity(entity, node); - newEntities.add(entity); - } + // TODO: Batch this + List newEntities = createEntitiesInBatch(request); // save all the new nodes without edges //TODO: Fix to save nodes and edges at same time pedigreeRepository.saveAll(newEntities); @@ -272,6 +268,7 @@ public List updatePedigreeNodes(Map request) Map nodesByGermplasm = getExistingPedigreeNodes(new ArrayList<>(request.keySet())); List newEntities = new ArrayList<>(); + // TODO: Batch this for (Entry entry : request.entrySet()) { PedigreeNodeEntity entity = nodesByGermplasm.get(entry.getKey()); if (entity != null) { @@ -329,6 +326,7 @@ private Map getExistingPedigreeNodes(List ge List nodeEntities = findPedigreeEntities(searchReq, null); for (PedigreeNodeEntity nodeEntity : nodeEntities) { + // TODO: nodeEntity.getGermplasm() causes a lazy load if (nodeEntity.getGermplasm() != null && nodeEntity.getGermplasm().getId() != null) { nodesByGermplasm.put(nodeEntity.getGermplasm().getId(), nodeEntity); } @@ -496,6 +494,48 @@ private void updateEntity(PedigreeNodeEntity entity, PedigreeNode node) throws B } } + + + // This method should be used in use cases where there are no existing node entities representing the list being passed through. + private List createEntitiesInBatch(List nodes) + throws BrAPIServerException { + List germIds = nodes.stream().map(PedigreeNode::getGermplasmDbId).collect(Collectors.toList()); + List crossingProjIds = nodes.stream().map(PedigreeNode::getCrossingProjectDbId).collect(Collectors.toList()); + + List germs = germplasmService.findByIds(germIds); + List crossingProjs = crossingProjectService.findCrossingProjectsByIds(crossingProjIds); + + List result = new ArrayList(); + for (PedigreeNode node : nodes) { + PedigreeNodeEntity entity = new PedigreeNodeEntity(); + + if (node.getGermplasmDbId() != null) { + String germId = node.getGermplasmDbId(); + Optional germEntity = germs.stream().filter(ge -> ge.getId().equals(germId)).findFirst(); + germEntity.ifPresent(entity::setGermplasm); + } + + UpdateUtility.updateEntity(node, entity); + + if (node.getCrossingYear() != null) + entity.setCrossingYear(node.getCrossingYear()); + if (node.getFamilyCode() != null) + entity.setFamilyCode(node.getFamilyCode()); + if (node.getPedigreeString() != null) + entity.setPedigreeString(node.getPedigreeString()); + + if (node.getCrossingProjectDbId() != null) { + String cpId = node.getCrossingProjectDbId(); + Optional crossingProjectEntity = crossingProjs.stream().filter(cp -> cp.getId().equals(cpId)).findFirst(); + crossingProjectEntity.ifPresent(entity::setCrossingProject); + } + + result.add(entity); + } + + return result; + } + private void updateEntityWithEdges(PedigreeNodeEntity entity, PedigreeNode node) throws BrAPIServerException { UpdateUtility.updateEntity(node, entity); updateEntity(entity, node); From 52dd04ceca1e5c21537dd95bd7d59fdbb42a0d64 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Wed, 5 Mar 2025 12:39:34 -0500 Subject: [PATCH 03/18] Batch pedigree edge functionality This may or may not improve things, needs testing --- .../repository/germ/PedigreeRepository.java | 2 + .../service/germ/GermplasmService.java | 18 +- .../service/germ/PedigreeService.java | 282 ++++++++++++++---- 3 files changed, 247 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/PedigreeRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/PedigreeRepository.java index 911707d4..2c11da57 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/PedigreeRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/PedigreeRepository.java @@ -7,4 +7,6 @@ public interface PedigreeRepository extends BrAPIRepository, PedigreeRepositoryCustom { public List findByGermplasm_Id(String germplasmDbId); + + public List findByGermplasm_IdIn(List germplasmDbIds); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java index 6f72feb7..f4b5becc 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java @@ -436,7 +436,7 @@ private void fetchRemainingGermCollectionsUsingQuery(SearchQueryBuilder pedigree = germplasmRepository.findAllBySearch(searchQuery); - Map pedigreeByGerm = new HashMap<>(); + Map pedigreeByGerm = new HashMap<>(); pedigree.forEach(germ -> pedigreeByGerm.put(germ.getId().toString(), germ.getPedigree())); germEntities.forEach(germ -> { @@ -696,11 +696,21 @@ public List findByIds(List germplasmDbIds) Metadata metadata = new Metadata().pagination(new IndexPagination()); Page page = findGermplasmEntities(request, metadata); - if (page.hasContent()) { - return page.getContent(); + if (!page.hasContent()) { + return null; } - return null; + List germsFoundInDb = page.getContent(); + + Set germIdsFoundInDB = germsFoundInDb.stream() + .map(BrAPIBaseEntity::getId) + .collect(Collectors.toSet()); + + if (!germIdsFoundInDB.containsAll(germplasmDbIds)) { + throw new BrAPIServerDbIdNotFoundException("Germplasm Ids passed to findByIds were not found in the DB", HttpStatus.BAD_REQUEST); + } + + return germsFoundInDb; } // TODO: Add lookupType param to RQ Germplasm which can short-circuit all the lookup logic to only one query here. diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index cdb4d45b..a6bf6fd2 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -1,20 +1,14 @@ package org.brapi.test.BrAPITestServer.service.germ; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; import java.util.stream.Collectors; -import java.util.Optional; -import java.util.Set; import io.swagger.model.IndexPagination; import org.apache.commons.lang3.tuple.Pair; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerDbIdNotFoundException; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException; +import org.brapi.test.BrAPITestServer.model.entity.BrAPIBaseEntity; import org.brapi.test.BrAPITestServer.model.entity.germ.CrossingProjectEntity; import org.brapi.test.BrAPITestServer.model.entity.germ.GermplasmEntity; import org.brapi.test.BrAPITestServer.model.entity.germ.PedigreeEdgeEntity; @@ -177,22 +171,74 @@ public Optional getPedigreeNode(String germplasmDbId) { return node; } - public PedigreeNodeEntity findOrCreatePedigreeNode(String germplasmDbId) throws BrAPIServerException { - Optional nodeOpt = getPedigreeNode(germplasmDbId); - PedigreeNodeEntity node; - if (nodeOpt.isPresent()) { - node = nodeOpt.get(); - } else { - GermplasmEntity germplasm = germplasmService.getGermplasmEntity(germplasmDbId); + public List getPedigreeNodes(List germplasmDbIds) { + List nodes = new ArrayList<>(); + + // TODO: Might have to make a custom query for this that fetches the germ eagerly, bc need to compare the germIds. Have to see if this is a significant performance hit. + List dbNodeList = pedigreeRepository.findByGermplasm_IdIn(germplasmDbIds); + + Map> nodesGroupedByGerm = dbNodeList.stream().collect(Collectors.groupingBy(pn -> pn.getGermplasm().getId())); + + nodesGroupedByGerm.forEach((germId, nodesByGerm) -> { + if (nodesByGerm.size() > 1) { + log.error("multiple pedigree nodes found for a single germplasm"); + } + + Optional node = nodesByGerm.stream().findFirst(); + + node.ifPresent(nodes::add); + }); + + return nodes; + } + + public List findOrCreatePedigreeNodesFromGermplasmIds(List germplasmDbIds) throws BrAPIServerException { + + List dbNodes = getPedigreeNodes(germplasmDbIds); + + List resultingNodes = new ArrayList<>(dbNodes); + + // Find out which germIds were not found in the DB. Use a set for improved performance on the contains check. + // TODO: Check if the germEntity is already populated by getPedigreeNodes, and if this block results in more DB transactions. + Set germIdsOfFoundNodes = dbNodes.stream() + .map(PedigreeNodeEntity::getGermplasm) + .map(BrAPIBaseEntity::getId) + .collect(Collectors.toSet()); + + List germIdsWithNoPedigreeRecord = germplasmDbIds.stream() + .filter(gId -> !germIdsOfFoundNodes.contains(gId)) + .collect(Collectors.toList()); + + // Now see if there are germplasm records that exist from the list created above that have a pedigree associated. + // If they do have a pedigree associated, add it to the result list. + // If not, create a Pedigree with that Germplasm record. + // TODO: Might need this to fetch the pedigree records on this query, otherwise a lazy load occurs + List germplasms = new ArrayList<>(); + + if (!germIdsWithNoPedigreeRecord.isEmpty()) { + germplasms = germplasmService.findByIds(germIdsWithNoPedigreeRecord); + } + + List nodesToCreate = new ArrayList<>(); + + for (GermplasmEntity germplasm : germplasms) { + // This is a lazy load if (germplasm.getPedigree() != null) { - node = germplasm.getPedigree(); + resultingNodes.add(germplasm.getPedigree()); } else { + // No pedigree exists for this germplasm. Create one and add the germplasm to it. PedigreeNodeEntity newNode = new PedigreeNodeEntity(); newNode.setGermplasm(germplasm); - node = pedigreeRepository.save(newNode); + nodesToCreate.add(newNode); } } - return node; + + // If any new nodes were made, save them and add them to the result list. + if (!nodesToCreate.isEmpty()) { + resultingNodes.addAll(pedigreeRepository.saveAll(nodesToCreate)); + } + + return resultingNodes; } public PedigreeNode getGermplasmPedigree(String germplasmDbId, Boolean includeSiblings) @@ -240,7 +286,7 @@ public ProgenyNode getGermplasmProgeny(String germplasmDbId) throws BrAPIServerE public List savePedigreeNodes(List request) throws BrAPIServerException { Map nodesByGermplasm = getExistingPedigreeNodes( - request.stream().map(p -> p.getGermplasmDbId()).collect(Collectors.toList())); + request.stream().map(PedigreeNode::getGermplasmDbId).collect(Collectors.toList())); if (!nodesByGermplasm.isEmpty()) { String errorMsg = "The following germplasmDbIds already have existing pedigree data. Please use PUT /pedigree to update these germplasm. \n" @@ -269,17 +315,24 @@ public List updatePedigreeNodes(Map request) List newEntities = new ArrayList<>(); // TODO: Batch this + + Map> entityDtoPairsByGermplasmId = new HashMap<>(); + for (Entry entry : request.entrySet()) { - PedigreeNodeEntity entity = nodesByGermplasm.get(entry.getKey()); + String germId = entry.getKey(); + PedigreeNodeEntity entity = nodesByGermplasm.get(germId); + if (entity != null) { - updateEntityWithEdges(entity, entry.getValue()); - newEntities.add(entity); + entityDtoPairsByGermplasmId.put(germId, Pair.of(entity, entry.getValue())); } else { throw new BrAPIServerDbIdNotFoundException("germplasm", entry.getKey(), HttpStatus.BAD_REQUEST); } } - List savedEntities = pedigreeRepository.saveAll(newEntities); + // First, update the basic properties of the nodes in batch. + updateEntitiesWithEdgesInBatch(entityDtoPairsByGermplasmId); + + List savedEntities = pedigreeRepository.saveAll(entityDtoPairsByGermplasmId.values().stream().map(Pair::getLeft).collect(Collectors.toList())); List saved = convertFromEntities(savedEntities, new PedigreeSearchRequest().includeParents(true).includeProgeny(true).includeSiblings(true)); return saved; @@ -302,8 +355,13 @@ public void updateGermplasmPedigree(List data) throws BrAPIServerExce } } - savePedigreeNodes(createPedigreeNodes); - updatePedigreeNodes(updatePedigreeNodes); + if (!createPedigreeNodes.isEmpty()) { + savePedigreeNodes(createPedigreeNodes); + } + + if (!updatePedigreeNodes.isEmpty()) { + updatePedigreeNodes(updatePedigreeNodes); + } } else { savePedigreeNodes(convertFromGermplasmToPedigreeBatchUsingNames(data)); } @@ -536,51 +594,173 @@ private List createEntitiesInBatch(List nodes) return result; } - private void updateEntityWithEdges(PedigreeNodeEntity entity, PedigreeNode node) throws BrAPIServerException { - UpdateUtility.updateEntity(node, entity); - updateEntity(entity, node); - if (node.getParents() != null) { + // This method should be used in use cases where there are existing node entities that may have edges. + private void updateEntitiesWithEdgesInBatch(Map> entityDtoPairsByGermId) throws BrAPIServerException { + List germIds = new ArrayList<>(entityDtoPairsByGermId.keySet()); + List crossingProjIds = entityDtoPairsByGermId.values() + .stream() + .map(nodePair -> nodePair.getRight().getCrossingProjectDbId()) + .filter(Objects::nonNull) + .collect(Collectors.toList()); - SearchQueryBuilder search = new SearchQueryBuilder(PedigreeEdgeEntity.class); - search.appendSingle(node.getGermplasmDbId(), "connectedNode.germplasm.id"); + List germIdsWithParentNodes = entityDtoPairsByGermId.entrySet() + .stream() + .filter(entry -> entry.getValue().getRight().getParents() != null) + .map(Entry::getKey) + .collect(Collectors.toList()); + + List germIdsWithProgenyNodes = entityDtoPairsByGermId.entrySet() + .stream() + .filter(entry -> entry.getValue().getRight().getProgeny() != null) + .map(Entry::getKey) + .collect(Collectors.toList()); + + List germs = new ArrayList<>(); + List crossingProjs = new ArrayList<>(); + + if (!germIds.isEmpty()) { + germs = germplasmService.findByIds(germIds); + } + if (!crossingProjIds.isEmpty()) { + crossingProjs = crossingProjectService.findCrossingProjectsByIds(crossingProjIds); + } + + if (!germIdsWithParentNodes.isEmpty()) { + List parentEdgesToDelete = new ArrayList<>(); + + SearchQueryBuilder search = new SearchQueryBuilder<>(PedigreeEdgeEntity.class); + search.appendList(germIdsWithParentNodes, "connectedNode.germplasm.id"); search.appendEnum(PedigreeEdgeEntity.EdgeType.child, "edgeType"); + // TODO: Does not need to be a paginated search. Pageable defaultPageSize = PagingUtility.getPageRequest(new Metadata().pagination(new IndexPagination().pageSize(10000000))); Page existingParentEdges = pedigreeEdgeRepository.findAllBySearchAndPaginate(search, defaultPageSize); - List edgeIdsToDelete = new ArrayList<>(); - edgeIdsToDelete.addAll(entity.getParentEdges().stream().map(e -> e.getId()).collect(Collectors.toList())); - edgeIdsToDelete.addAll(existingParentEdges.getContent().stream().map(e -> e.getId()).collect(Collectors.toList())); + List existingParentEdgesFromPassedEntities = entityDtoPairsByGermId.entrySet() + .stream() + .flatMap(entry -> entry.getValue().getLeft().getParentEdges().stream()) + .map(BrAPIBaseEntity::getId) + .collect(Collectors.toList()); + + parentEdgesToDelete.addAll(existingParentEdgesFromPassedEntities); + parentEdgesToDelete.addAll(existingParentEdges.getContent().stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList())); - if (!edgeIdsToDelete.isEmpty()) { - pedigreeEdgeRepository.deleteAllByIdInBatch(edgeIdsToDelete); + if (!parentEdgesToDelete.isEmpty()) { + pedigreeEdgeRepository.deleteAllByIdInBatch(parentEdgesToDelete); } - for (PedigreeNodeParents parentNode : node.getParents()) { - PedigreeNodeEntity parentEntity = findOrCreatePedigreeNode(parentNode.getGermplasmDbId()); - entity.addParent(parentEntity, parentNode.getParentType()); - parentEntity.addProgeny(entity, parentNode.getParentType()); + List> nodesWithParents = entityDtoPairsByGermId.values() + .stream() + .filter(p -> !p.getRight().getParents().isEmpty()) + .collect(Collectors.toList()); + + List germIdsOfAllParents = nodesWithParents.stream() + .flatMap(p -> p.getRight().getParents().stream()) + .map(PedigreeNodeParents::getGermplasmDbId) + .collect(Collectors.toList()); + + + List createdOrFoundParentNodes = findOrCreatePedigreeNodesFromGermplasmIds(germIdsOfAllParents); + + for (Pair nodeWithParent : nodesWithParents) { + PedigreeNodeEntity nodeEntity = nodeWithParent.getLeft(); + PedigreeNode nodeDto = nodeWithParent.getRight(); + for (PedigreeNodeParents parentNode : nodeDto.getParents()) { + // Is it possible that more that any of these parents share the same germplasm ID? If so, we need to figure out how to handle that use case. + PedigreeNodeEntity parentEntity = createdOrFoundParentNodes.stream() + .filter(pne -> pne.getGermplasm().getId().equals(parentNode.getGermplasmDbId())) + .findFirst() + .orElse(null); + + // Impossible to be null because of exception thrown in findOrCreatePedigreeNodesFromGermplasmIds(). + // As long as the germIds of all the parents nodes in the rq were supplied, they will be found or created or this code is never executed. + if (parentEntity != null) { + nodeEntity.addParent(parentEntity, parentNode.getParentType()); + parentEntity.addProgeny(nodeEntity, parentNode.getParentType()); + } + } } } - if (node.getProgeny() != null) { + + if (!germIdsWithProgenyNodes.isEmpty()) { + List progenyEdgesToDelete = new ArrayList<>(); SearchQueryBuilder search = new SearchQueryBuilder(PedigreeEdgeEntity.class); - search.appendSingle(node.getGermplasmDbId(), "connectedNode.germplasm.id"); + + search.appendList(germIdsWithProgenyNodes, "conncetedNode.germplasm.id"); search.appendEnum(PedigreeEdgeEntity.EdgeType.parent, "edgeType"); Pageable defaultPageSize = PagingUtility.getPageRequest(new Metadata().pagination(new IndexPagination().pageSize(10000000))); Page existingProgenyEdges = pedigreeEdgeRepository.findAllBySearchAndPaginate(search, defaultPageSize); - List edgeIdsToDelete = new ArrayList<>(); - edgeIdsToDelete.addAll(entity.getProgenyEdges().stream().map(e -> e.getId()).collect(Collectors.toList())); - edgeIdsToDelete.addAll(existingProgenyEdges.getContent().stream().map(e -> e.getId()).collect(Collectors.toList())); + List existingProgenyEdgeFromPassedEntities = entityDtoPairsByGermId.entrySet() + .stream() + .flatMap(entry -> entry.getValue().getLeft().getProgenyEdges().stream()) + .map(BrAPIBaseEntity::getId) + .collect(Collectors.toList()); - if (!edgeIdsToDelete.isEmpty()) { - pedigreeEdgeRepository.deleteAllByIdInBatch(edgeIdsToDelete); + progenyEdgesToDelete.addAll(existingProgenyEdgeFromPassedEntities); + progenyEdgesToDelete.addAll(existingProgenyEdges.getContent().stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList())); + + if (!progenyEdgesToDelete.isEmpty()) { + pedigreeEdgeRepository.deleteAllByIdInBatch(progenyEdgesToDelete); } - for (PedigreeNodeParents childNode : node.getProgeny()) { - PedigreeNodeEntity childEntity = findOrCreatePedigreeNode(childNode.getGermplasmDbId()); - entity.addProgeny(childEntity, childNode.getParentType()); - childEntity.addParent(entity, childNode.getParentType()); + List> nodesWithProgeny = entityDtoPairsByGermId.values() + .stream() + .filter(p -> !p.getRight().getProgeny().isEmpty()) + .collect(Collectors.toList()); + + List germIdsOfAllProgeny = nodesWithProgeny.stream() + .flatMap(p -> p.getRight().getParents().stream()) + .map(PedigreeNodeParents::getGermplasmDbId) + .collect(Collectors.toList()); + + List createdOrFoundProgenyNodes = findOrCreatePedigreeNodesFromGermplasmIds(germIdsOfAllProgeny); + + for (Pair nodeWithProgeny : nodesWithProgeny) { + PedigreeNodeEntity nodeEntity = nodeWithProgeny.getLeft(); + PedigreeNode nodeDto = nodeWithProgeny.getRight(); + // Create a map of Nodes with progeny to its + + for (PedigreeNodeParents childNode : nodeDto.getProgeny()) { + PedigreeNodeEntity childEntity = createdOrFoundProgenyNodes.stream() + .filter(pne -> pne.getGermplasm().getId().equals(childNode.getGermplasmDbId())) + .findFirst() + .orElse(null); + + // Impossible to be null because of exception thrown in findOrCreatePedigreeNodesFromGermplasmIds(). + // As long as the germIds of all the parents nodes in the rq were supplied, they will be found or created or this code is never executed. + if (childEntity != null) { + nodeEntity.addParent(childEntity, childNode.getParentType()); + childEntity.addProgeny(nodeEntity, childNode.getParentType()); + } + } + } + } + + for (Entry> entityDtoPairByGermId: entityDtoPairsByGermId.entrySet()) { + PedigreeNodeEntity entity = entityDtoPairByGermId.getValue().getLeft(); + PedigreeNode node = entityDtoPairByGermId.getValue().getRight(); + + if (node.getGermplasmDbId() != null && entity.getGermplasm() == null) { + String germId = node.getGermplasmDbId(); + Optional germEntity = germs.stream().filter(ge -> ge.getId().equals(germId)).findFirst(); + germEntity.ifPresent(entity::setGermplasm); + } + + UpdateUtility.updateEntity(node, entity); + + if (node.getCrossingYear() != null) + entity.setCrossingYear(node.getCrossingYear()); + if (node.getFamilyCode() != null) + entity.setFamilyCode(node.getFamilyCode()); + if (node.getPedigreeString() != null) + entity.setPedigreeString(node.getPedigreeString()); + + if (node.getCrossingProjectDbId() != null) { + String cpId = node.getCrossingProjectDbId(); + Optional crossingProjectEntity = crossingProjs.stream().filter(cp -> cp.getId().equals(cpId)).findFirst(); + crossingProjectEntity.ifPresent(entity::setCrossingProject); } } } From afeded89ca273e43642fe539f148fc113143a770 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Wed, 19 Mar 2025 16:19:54 -0400 Subject: [PATCH 04/18] Clean up updateEntitiesWithEdgesInBatch Replace pagination calls with normal queries --- .../service/germ/PedigreeService.java | 226 +++++++++--------- 1 file changed, 118 insertions(+), 108 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index a6bf6fd2..49f74d0a 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -595,8 +595,8 @@ private List createEntitiesInBatch(List nodes) } // This method should be used in use cases where there are existing node entities that may have edges. - private void updateEntitiesWithEdgesInBatch(Map> entityDtoPairsByGermId) throws BrAPIServerException { + private void updateEntitiesWithEdgesInBatch(Map> entityDtoPairsByGermId) + throws BrAPIServerException { List germIds = new ArrayList<>(entityDtoPairsByGermId.keySet()); List crossingProjIds = entityDtoPairsByGermId.values() .stream() @@ -627,115 +627,11 @@ private void updateEntitiesWithEdgesInBatch(Map parentEdgesToDelete = new ArrayList<>(); - - SearchQueryBuilder search = new SearchQueryBuilder<>(PedigreeEdgeEntity.class); - search.appendList(germIdsWithParentNodes, "connectedNode.germplasm.id"); - search.appendEnum(PedigreeEdgeEntity.EdgeType.child, "edgeType"); - // TODO: Does not need to be a paginated search. - Pageable defaultPageSize = PagingUtility.getPageRequest(new Metadata().pagination(new IndexPagination().pageSize(10000000))); - Page existingParentEdges = pedigreeEdgeRepository.findAllBySearchAndPaginate(search, defaultPageSize); - - List existingParentEdgesFromPassedEntities = entityDtoPairsByGermId.entrySet() - .stream() - .flatMap(entry -> entry.getValue().getLeft().getParentEdges().stream()) - .map(BrAPIBaseEntity::getId) - .collect(Collectors.toList()); - - parentEdgesToDelete.addAll(existingParentEdgesFromPassedEntities); - parentEdgesToDelete.addAll(existingParentEdges.getContent().stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList())); - - if (!parentEdgesToDelete.isEmpty()) { - pedigreeEdgeRepository.deleteAllByIdInBatch(parentEdgesToDelete); - } - - List> nodesWithParents = entityDtoPairsByGermId.values() - .stream() - .filter(p -> !p.getRight().getParents().isEmpty()) - .collect(Collectors.toList()); - - List germIdsOfAllParents = nodesWithParents.stream() - .flatMap(p -> p.getRight().getParents().stream()) - .map(PedigreeNodeParents::getGermplasmDbId) - .collect(Collectors.toList()); - - - List createdOrFoundParentNodes = findOrCreatePedigreeNodesFromGermplasmIds(germIdsOfAllParents); - - for (Pair nodeWithParent : nodesWithParents) { - PedigreeNodeEntity nodeEntity = nodeWithParent.getLeft(); - PedigreeNode nodeDto = nodeWithParent.getRight(); - for (PedigreeNodeParents parentNode : nodeDto.getParents()) { - // Is it possible that more that any of these parents share the same germplasm ID? If so, we need to figure out how to handle that use case. - PedigreeNodeEntity parentEntity = createdOrFoundParentNodes.stream() - .filter(pne -> pne.getGermplasm().getId().equals(parentNode.getGermplasmDbId())) - .findFirst() - .orElse(null); - - // Impossible to be null because of exception thrown in findOrCreatePedigreeNodesFromGermplasmIds(). - // As long as the germIds of all the parents nodes in the rq were supplied, they will be found or created or this code is never executed. - if (parentEntity != null) { - nodeEntity.addParent(parentEntity, parentNode.getParentType()); - parentEntity.addProgeny(nodeEntity, parentNode.getParentType()); - } - } - } + updateParentEdges(germIdsWithParentNodes, entityDtoPairsByGermId); } if (!germIdsWithProgenyNodes.isEmpty()) { - List progenyEdgesToDelete = new ArrayList<>(); - - SearchQueryBuilder search = new SearchQueryBuilder(PedigreeEdgeEntity.class); - - search.appendList(germIdsWithProgenyNodes, "conncetedNode.germplasm.id"); - search.appendEnum(PedigreeEdgeEntity.EdgeType.parent, "edgeType"); - Pageable defaultPageSize = PagingUtility.getPageRequest(new Metadata().pagination(new IndexPagination().pageSize(10000000))); - Page existingProgenyEdges = pedigreeEdgeRepository.findAllBySearchAndPaginate(search, defaultPageSize); - - List existingProgenyEdgeFromPassedEntities = entityDtoPairsByGermId.entrySet() - .stream() - .flatMap(entry -> entry.getValue().getLeft().getProgenyEdges().stream()) - .map(BrAPIBaseEntity::getId) - .collect(Collectors.toList()); - - progenyEdgesToDelete.addAll(existingProgenyEdgeFromPassedEntities); - progenyEdgesToDelete.addAll(existingProgenyEdges.getContent().stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList())); - - if (!progenyEdgesToDelete.isEmpty()) { - pedigreeEdgeRepository.deleteAllByIdInBatch(progenyEdgesToDelete); - } - - List> nodesWithProgeny = entityDtoPairsByGermId.values() - .stream() - .filter(p -> !p.getRight().getProgeny().isEmpty()) - .collect(Collectors.toList()); - - List germIdsOfAllProgeny = nodesWithProgeny.stream() - .flatMap(p -> p.getRight().getParents().stream()) - .map(PedigreeNodeParents::getGermplasmDbId) - .collect(Collectors.toList()); - - List createdOrFoundProgenyNodes = findOrCreatePedigreeNodesFromGermplasmIds(germIdsOfAllProgeny); - - for (Pair nodeWithProgeny : nodesWithProgeny) { - PedigreeNodeEntity nodeEntity = nodeWithProgeny.getLeft(); - PedigreeNode nodeDto = nodeWithProgeny.getRight(); - // Create a map of Nodes with progeny to its - - for (PedigreeNodeParents childNode : nodeDto.getProgeny()) { - PedigreeNodeEntity childEntity = createdOrFoundProgenyNodes.stream() - .filter(pne -> pne.getGermplasm().getId().equals(childNode.getGermplasmDbId())) - .findFirst() - .orElse(null); - - // Impossible to be null because of exception thrown in findOrCreatePedigreeNodesFromGermplasmIds(). - // As long as the germIds of all the parents nodes in the rq were supplied, they will be found or created or this code is never executed. - if (childEntity != null) { - nodeEntity.addParent(childEntity, childNode.getParentType()); - childEntity.addProgeny(nodeEntity, childNode.getParentType()); - } - } - } + updateChildEdges(germIdsWithProgenyNodes, entityDtoPairsByGermId); } for (Entry> entityDtoPairByGermId: entityDtoPairsByGermId.entrySet()) { @@ -765,6 +661,120 @@ private void updateEntitiesWithEdgesInBatch(Map germIdsWithParentNodes, + Map> entityDtoPairsByGermId) + throws BrAPIServerException { + + List parentEdgesToDelete = new ArrayList<>(); + + SearchQueryBuilder search = new SearchQueryBuilder<>(PedigreeEdgeEntity.class); + search.appendList(germIdsWithParentNodes, "connectedNode.germplasm.id"); + search.appendEnum(PedigreeEdgeEntity.EdgeType.child, "edgeType"); + List existingParentEdges = pedigreeEdgeRepository.findAllBySearch(search); + + List existingParentEdgesFromPassedEntities = entityDtoPairsByGermId.entrySet() + .stream() + .flatMap(entry -> entry.getValue().getLeft().getParentEdges().stream()) + .map(BrAPIBaseEntity::getId) + .collect(Collectors.toList()); + + parentEdgesToDelete.addAll(existingParentEdgesFromPassedEntities); + parentEdgesToDelete.addAll(existingParentEdges.stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList())); + + if (!parentEdgesToDelete.isEmpty()) { + pedigreeEdgeRepository.deleteAllByIdInBatch(parentEdgesToDelete); + } + + List> nodesWithParents = entityDtoPairsByGermId.values() + .stream() + .filter(p -> !p.getRight().getParents().isEmpty()) + .collect(Collectors.toList()); + + List germIdsOfAllParents = nodesWithParents.stream() + .flatMap(p -> p.getRight().getParents().stream()) + .map(PedigreeNodeParents::getGermplasmDbId) + .collect(Collectors.toList()); + + + List createdOrFoundParentNodes = findOrCreatePedigreeNodesFromGermplasmIds(germIdsOfAllParents); + + for (Pair nodeWithParent : nodesWithParents) { + PedigreeNodeEntity nodeEntity = nodeWithParent.getLeft(); + PedigreeNode nodeDto = nodeWithParent.getRight(); + for (PedigreeNodeParents parentNode : nodeDto.getParents()) { + // Is it possible that more that any of these parents share the same germplasm ID? If so, we need to figure out how to handle that use case. + PedigreeNodeEntity parentEntity = createdOrFoundParentNodes.stream() + .filter(pne -> pne.getGermplasm().getId().equals(parentNode.getGermplasmDbId())) + .findFirst() + .orElse(null); + + // Impossible to be null because of exception thrown in findOrCreatePedigreeNodesFromGermplasmIds(). + // As long as the germIds of all the parents nodes in the rq were supplied, they will be found or created or this code is never executed. + if (parentEntity != null) { + nodeEntity.addParent(parentEntity, parentNode.getParentType()); + parentEntity.addProgeny(nodeEntity, parentNode.getParentType()); + } + } + } + } + + private void updateChildEdges(List germIdsWithProgenyNodes, + Map> entityDtoPairsByGermId) + throws BrAPIServerException { + List progenyEdgesToDelete = new ArrayList<>(); + + SearchQueryBuilder search = new SearchQueryBuilder(PedigreeEdgeEntity.class); + + search.appendList(germIdsWithProgenyNodes, "conncetedNode.germplasm.id"); + search.appendEnum(PedigreeEdgeEntity.EdgeType.parent, "edgeType"); + List existingProgenyEdges = pedigreeEdgeRepository.findAllBySearch(search); + + List existingProgenyEdgeFromPassedEntities = entityDtoPairsByGermId.entrySet() + .stream() + .flatMap(entry -> entry.getValue().getLeft().getProgenyEdges().stream()) + .map(BrAPIBaseEntity::getId) + .collect(Collectors.toList()); + + progenyEdgesToDelete.addAll(existingProgenyEdgeFromPassedEntities); + progenyEdgesToDelete.addAll(existingProgenyEdges.stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList())); + + if (!progenyEdgesToDelete.isEmpty()) { + pedigreeEdgeRepository.deleteAllByIdInBatch(progenyEdgesToDelete); + } + + List> nodesWithProgeny = entityDtoPairsByGermId.values() + .stream() + .filter(p -> !p.getRight().getProgeny().isEmpty()) + .collect(Collectors.toList()); + + List germIdsOfAllProgeny = nodesWithProgeny.stream() + .flatMap(p -> p.getRight().getParents().stream()) + .map(PedigreeNodeParents::getGermplasmDbId) + .collect(Collectors.toList()); + + List createdOrFoundProgenyNodes = findOrCreatePedigreeNodesFromGermplasmIds(germIdsOfAllProgeny); + + for (Pair nodeWithProgeny : nodesWithProgeny) { + PedigreeNodeEntity nodeEntity = nodeWithProgeny.getLeft(); + PedigreeNode nodeDto = nodeWithProgeny.getRight(); + // Create a map of Nodes with progeny to its + + for (PedigreeNodeParents childNode : nodeDto.getProgeny()) { + PedigreeNodeEntity childEntity = createdOrFoundProgenyNodes.stream() + .filter(pne -> pne.getGermplasm().getId().equals(childNode.getGermplasmDbId())) + .findFirst() + .orElse(null); + + // Impossible to be null because of exception thrown in findOrCreatePedigreeNodesFromGermplasmIds(). + // As long as the germIds of all the parents nodes in the rq were supplied, they will be found or created or this code is never executed. + if (childEntity != null) { + nodeEntity.addParent(childEntity, childNode.getParentType()); + childEntity.addProgeny(nodeEntity, childNode.getParentType()); + } + } + } + } + public List convertFromGermplasmToPedigreeBatchUsingNames(List germplasms) throws BrAPIServerException{ List result = new ArrayList(); From 53f5919a273e2eb305895d317a5217392092ddcf Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Mon, 10 Mar 2025 19:53:03 -0400 Subject: [PATCH 05/18] Prevent DTO converting on pedigree for Germplasm POST Path, as data is unused in response --- .../germ/GermplasmApiController.java | 1 - .../service/germ/PedigreeService.java | 48 +++++++++++++------ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java b/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java index 09d279cb..ccd3eccf 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java @@ -201,7 +201,6 @@ public ResponseEntity germplasmPost(@RequestBody List data = germplasmService.saveGermplasm(body); - // TODO: Add short-circuit if no ped connections are sent. pedigreeService.updateGermplasmPedigree(data); return responseOK(new GermplasmListResponse(), new GermplasmListResponseResult(), data); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 49f74d0a..bc726134 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -284,13 +284,19 @@ public ProgenyNode getGermplasmProgeny(String germplasmDbId) throws BrAPIServerE return result; } - public List savePedigreeNodes(List request) throws BrAPIServerException { + public List savePedigreeNodes(List request) + throws BrAPIServerException { + return savePedigreeNodes(request, true); + } + + public List savePedigreeNodes(List request, + boolean returnValues) throws BrAPIServerException { Map nodesByGermplasm = getExistingPedigreeNodes( request.stream().map(PedigreeNode::getGermplasmDbId).collect(Collectors.toList())); if (!nodesByGermplasm.isEmpty()) { String errorMsg = "The following germplasmDbIds already have existing pedigree data. Please use PUT /pedigree to update these germplasm. \n" - + nodesByGermplasm.keySet().toString(); + + nodesByGermplasm.keySet(); throw new BrAPIServerException(HttpStatus.BAD_REQUEST, errorMsg); } @@ -305,16 +311,22 @@ public List savePedigreeNodes(List request) throws B updateRequest.put(newNode.getGermplasmDbId(), newNode); } // update the new nodes with requested edges - List saved = updatePedigreeNodes(updateRequest); + if (returnValues) { + return updatePedigreeNodes(updateRequest); + } else { + updatePedigreeNodes(updateRequest, false); + return Collections.emptyList(); + } + } - return saved; + public List updatePedigreeNodes(Map request) + throws BrAPIServerException { + return updatePedigreeNodes(request, true); } - public List updatePedigreeNodes(Map request) throws BrAPIServerException { + public List updatePedigreeNodes(Map request, + boolean returnValues) throws BrAPIServerException { Map nodesByGermplasm = getExistingPedigreeNodes(new ArrayList<>(request.keySet())); - List newEntities = new ArrayList<>(); - - // TODO: Batch this Map> entityDtoPairsByGermplasmId = new HashMap<>(); @@ -332,10 +344,16 @@ public List updatePedigreeNodes(Map request) // First, update the basic properties of the nodes in batch. updateEntitiesWithEdgesInBatch(entityDtoPairsByGermplasmId); - List savedEntities = pedigreeRepository.saveAll(entityDtoPairsByGermplasmId.values().stream().map(Pair::getLeft).collect(Collectors.toList())); - List saved = convertFromEntities(savedEntities, - new PedigreeSearchRequest().includeParents(true).includeProgeny(true).includeSiblings(true)); - return saved; + if (returnValues) { + List savedEntities = pedigreeRepository.saveAll(entityDtoPairsByGermplasmId.values().stream().map(Pair::getLeft).collect(Collectors.toList())); + List saved = convertFromEntities(savedEntities, + new PedigreeSearchRequest().includeParents(true).includeProgeny(true).includeSiblings(true)); + return saved; + } else { + // If no values are required, simply return an empty list. + pedigreeRepository.saveAll(entityDtoPairsByGermplasmId.values().stream().map(Pair::getLeft).collect(Collectors.toList())); + return Collections.emptyList(); + } } public void updateGermplasmPedigree(List data) throws BrAPIServerException { @@ -356,14 +374,14 @@ public void updateGermplasmPedigree(List data) throws BrAPIServerExce } if (!createPedigreeNodes.isEmpty()) { - savePedigreeNodes(createPedigreeNodes); + savePedigreeNodes(createPedigreeNodes, false); } if (!updatePedigreeNodes.isEmpty()) { - updatePedigreeNodes(updatePedigreeNodes); + updatePedigreeNodes(updatePedigreeNodes, false); } } else { - savePedigreeNodes(convertFromGermplasmToPedigreeBatchUsingNames(data)); + savePedigreeNodes(convertFromGermplasmToPedigreeBatchUsingNames(data), false); } } From f020388f67a40444dca5c2b3f2ff5c95546a8b23 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Mon, 10 Mar 2025 19:46:37 -0400 Subject: [PATCH 06/18] Add exref checking method to UpdateUtility This method will help to alleviate use cases during batch updates where external references were getting updated when they didn't need to. This created a large amount of deletions and insertions slowing batch updates down --- .../service/UpdateUtility.java | 58 ++++++++++++++++++- .../service/germ/PedigreeService.java | 2 +- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/UpdateUtility.java b/src/main/java/org/brapi/test/BrAPITestServer/service/UpdateUtility.java index f198c2ab..7052732d 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/UpdateUtility.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/UpdateUtility.java @@ -1,9 +1,13 @@ package org.brapi.test.BrAPITestServer.service; import io.swagger.model.BrAPIDataModel; +import io.swagger.model.ExternalReferences; +import io.swagger.model.ExternalReferencesInner; import org.brapi.test.BrAPITestServer.model.entity.BrAPIPrimaryEntity; +import org.brapi.test.BrAPITestServer.model.entity.ExternalReferenceEntity; -import java.util.Optional; +import java.util.*; +import java.util.stream.Collectors; public class UpdateUtility { @@ -34,12 +38,60 @@ public static T convertFromEntity(BrAPIPrimaryEntity } public static T updateEntity(BrAPIDataModel model, T entity) { + updateAdditionalInfo(model, entity); + if (model.getExternalReferences() != null) { + entity.setExternalReferences(model.getExternalReferences()); + } + return entity; + } + + private static void updateAdditionalInfo(BrAPIDataModel model, T entity) { if (model.getAdditionalInfo() != null) { entity.setAdditionalInfo(model.getAdditionalInfo()); } + } + + /* + Call this method when external references are eagerly loaded for bulk updates to entities to ensure + unnecessary deletions and insertions don't occur. This will improve performance in these use cases. + + WARN: If refs aren't eagerly loaded, hibernate will generate a query on the entity.getReferences() call. This could slow performance. + + This method will check if the external references in the model already exist in the entity, and will prevent setting + the entity with the model's refs. + + TODO: See if migrating all callers of updateEntity to this method can be done. + */ + public static T updateEntityCheckExRefs(BrAPIDataModel model, T entity) { + updateAdditionalInfo(model, entity); if (model.getExternalReferences() != null) { - entity.setExternalReferences(model.getExternalReferences()); + + ExternalReferences exRefs = model.getExternalReferences(); + + Map> existingRefsById = + entity.getExternalReferences() != null ? + entity.getExternalReferences().stream().collect(Collectors.groupingBy(ExternalReferenceEntity::getExternalReferenceId)) + : Collections.emptyMap(); + + + List newExRefs = exRefs.stream().filter(exRef -> { + List existingEntityRefList = existingRefsById.get(exRef.getReferenceID()); + + if (existingEntityRefList == null || existingEntityRefList.isEmpty()) { + return true; + } + + ExternalReferenceEntity existingEntityRef = existingEntityRefList.get(0); + + return !existingEntityRef.getExternalReferenceSource().equals(exRef.getReferenceSource()); + }).collect(Collectors.toList()); + + if (!newExRefs.isEmpty()) { + // Detected different ex refs than what is in the original entity. Updating entity exRefs. + entity.setExternalReferences(model.getExternalReferences()); + } + // If there are no new exRefs no update is made. } return entity; } -} +} \ No newline at end of file diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index bc726134..3ea1eb3c 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -662,7 +662,7 @@ private void updateEntitiesWithEdgesInBatch(Map Date: Thu, 20 Mar 2025 18:18:11 -0400 Subject: [PATCH 07/18] Simplify updateGermplasmPedigree method Made a new method putting relevant logic to the POST call. Not sure if PUT could use this one, would need to test. --- .../germ/GermplasmApiController.java | 2 +- .../service/germ/PedigreeService.java | 49 +++++++++++-------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java b/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java index ccd3eccf..407dc9b6 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java @@ -201,7 +201,7 @@ public ResponseEntity germplasmPost(@RequestBody List data = germplasmService.saveGermplasm(body); - pedigreeService.updateGermplasmPedigree(data); + pedigreeService.updateGermplasmPedigreeForPost(data); return responseOK(new GermplasmListResponse(), new GermplasmListResponseResult(), data); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 3ea1eb3c..24e38961 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -356,34 +356,45 @@ public List updatePedigreeNodes(Map request, } } + // TODO: See if we can get the PUT from the GermplasmApiController to use this code as well instead of updateGermplasmPedigree() + public void updateGermplasmPedigreeForPost(List data) throws BrAPIServerException { + // T = With pedigree, F = Without pedigree + Map> germsWithAndWithoutPedigree + = data.stream().collect(Collectors.partitioningBy(g -> g.getPedigree() != null)); + + if (!germsWithAndWithoutPedigree.get(true).isEmpty()) { + savePedigreeNodes(convertFromGermplasmToPedigreeBatchUsingNames(germsWithAndWithoutPedigree.get(true)), false); + } + + if (!germsWithAndWithoutPedigree.get(false).isEmpty()) { + List createPedigreeNodes = new ArrayList<>(); + + for (Germplasm germplasm : germsWithAndWithoutPedigree.get(false)) { + createPedigreeNodes.add(convertFromGermplasmToPedigree(germplasm)); + } + + savePedigreeNodes(createPedigreeNodes, false); + } + } + public void updateGermplasmPedigree(List data) throws BrAPIServerException { List createPedigreeNodes = new ArrayList<>(); Map updatePedigreeNodes = new HashMap<>(); - // TODO: This always returns empty for the germplasm post path, since this method is always passed a newly created germplasm record. Map nodesByGermplasm = getExistingPedigreeNodes( data.stream().map(p -> p.getGermplasmDbId()).collect(Collectors.toList())); - if (data.stream().noneMatch(g -> g.getPedigree() != null) || !nodesByGermplasm.isEmpty()) { - for (Germplasm germplasm : data) { - if (nodesByGermplasm.containsKey(germplasm.getGermplasmDbId())) { - updatePedigreeNodes.put(germplasm.getGermplasmDbId(), convertFromGermplasmToPedigree(germplasm)); - } else { - createPedigreeNodes.add(convertFromGermplasmToPedigree(germplasm)); - } - } - - if (!createPedigreeNodes.isEmpty()) { - savePedigreeNodes(createPedigreeNodes, false); - } - - if (!updatePedigreeNodes.isEmpty()) { - updatePedigreeNodes(updatePedigreeNodes, false); + for (Germplasm germplasm : data) { + if (nodesByGermplasm.containsKey(germplasm.getGermplasmDbId())) { + updatePedigreeNodes.put(germplasm.getGermplasmDbId(), convertFromGermplasmToPedigree(germplasm)); + } else { + createPedigreeNodes.add(convertFromGermplasmToPedigree(germplasm)); } - } else { - savePedigreeNodes(convertFromGermplasmToPedigreeBatchUsingNames(data), false); } + savePedigreeNodes(createPedigreeNodes); + updatePedigreeNodes(updatePedigreeNodes); + } private Map getExistingPedigreeNodes(List germplasmDbIds) @@ -837,14 +848,12 @@ public List convertFromGermplasmToPedigreeBatchUsingNames(List pedigreeList = new ArrayList<>(); if (germplasm.getPedigree() != null) { pedigreeList = Arrays.asList(germplasm.getPedigree().split("/")); } - // TODO: Could split this up into one query in batch that returns a Map of Germplasm to its Nodes. GermplasmEntity motherGerm = null; GermplasmEntity fatherGerm = null; if (pedigreeList.size() > 0) { From 76292ab9b7e2bfbddc51190e6a832e4ca8a88800 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 21 Mar 2025 11:34:49 -0400 Subject: [PATCH 08/18] Batch crop and breeding method lookups on germ creation --- .../repository/core/CropRepository.java | 4 + .../germ/BreedingMethodRepository.java | 5 +- .../service/core/CropService.java | 4 + .../service/germ/BreedingMethodService.java | 6 + .../service/germ/GermplasmService.java | 108 ++++++++++++++++-- 5 files changed, 119 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/core/CropRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/core/CropRepository.java index c009b911..9808025f 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/core/CropRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/core/CropRepository.java @@ -5,6 +5,10 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import java.util.List; + public interface CropRepository extends BrAPIRepository{ public Page findByCropName(String cropName, Pageable pageRequest); + + public List findByCropNameIn(List names); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java index a77d30aa..06f4c4d4 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java @@ -3,6 +3,9 @@ import org.brapi.test.BrAPITestServer.model.entity.germ.BreedingMethodEntity; import org.brapi.test.BrAPITestServer.repository.BrAPIRepository; -public interface BreedingMethodRepository extends BrAPIRepository{ +import java.util.List; +import java.util.Set; +public interface BreedingMethodRepository extends BrAPIRepository{ + public List findByIdIn(List id); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java index 5419ea03..3932e698 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/core/CropService.java @@ -63,6 +63,10 @@ public CropEntity getCropEntity(String commonCropName) throws BrAPIServerExcepti return entity; } + public List findCropsByNames(List names) { + return cropRepository.findByCropNameIn(names); + } + public CropEntity saveCropEntity(String commonCropName) throws BrAPIServerException { CropEntity entity = null; if (commonCropName != null) { diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/BreedingMethodService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/BreedingMethodService.java index fbfb64a5..a38a8d3c 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/BreedingMethodService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/BreedingMethodService.java @@ -1,7 +1,9 @@ package org.brapi.test.BrAPITestServer.service.germ; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerDbIdNotFoundException; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException; @@ -52,6 +54,10 @@ public BreedingMethodEntity getBreedingMethodEntity(String breedingMethodDbId, H return breedingMethodEntity; } + public List findBreedingMethodsByIds(List ids) { + return breedingMethodRepository.findByIdIn(ids); + } + private BreedingMethod convertFromEntity(BreedingMethodEntity entity) { BreedingMethod method = new BreedingMethod(); method.setAbbreviation(entity.getAbbreviation()); diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java index f4b5becc..319dc9b6 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java @@ -502,14 +502,8 @@ public void softDeleteGermplasm(String germplasmDbId) throws BrAPIServerExceptio } public List saveGermplasm(@Valid List body) throws BrAPIServerException { - List toSave = new ArrayList<>(); - for (GermplasmNewRequest germplasm : body) { - GermplasmEntity entity = new GermplasmEntity(); - updateEntity(entity, germplasm); - toSave.add(entity); - } // Save batch. - return germplasmRepository.saveAll(toSave) + return germplasmRepository.saveAll(createEntitiesInBatch(body)) .stream() .map(this::convertFromEntity) .collect(Collectors.toList()); @@ -572,6 +566,106 @@ private Germplasm convertFromEntity(GermplasmEntity entity) { return germ; } + private List createEntitiesInBatch(List body) + throws BrAPIServerException { + List toSave = new ArrayList<>(); + + List breedingMethodIds = body.stream() + .map(GermplasmNewRequest::getBreedingMethodDbId) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + // Use a set since all crop names are likely all or mostly the same. + Set cropNames = body.stream() + .map(GermplasmNewRequest::getCommonCropName) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + Map foundBreedingMethodsById + = breedingMethodService.findBreedingMethodsByIds(breedingMethodIds) + .stream() + .collect(Collectors.toMap(BrAPIBaseEntity::getId, e -> e)); + Map foundCropsByCropName + = cropService.findCropsByNames(new ArrayList<>(cropNames)) + .stream() + .collect(Collectors.toMap(CropEntity::getCropName, e -> e)); + + for (GermplasmNewRequest request : body) { + GermplasmEntity entity = new GermplasmEntity(); + + UpdateUtility.updateEntity(request, entity); + + if (request.getAccessionNumber() != null) + entity.setAccessionNumber(request.getAccessionNumber()); + if (request.getAcquisitionDate() != null) { + entity.setAcquisitionDate(DateUtility.toDate(request.getAcquisitionDate())); + entity.setAcquisitionSourceCode(AcquisitionSourceCodeEnum._99); + } + if (request.getBiologicalStatusOfAccessionCode() != null) + entity.setBiologicalStatusOfAccessionCode(request.getBiologicalStatusOfAccessionCode()); + if (request.getBreedingMethodDbId() != null) { + entity.setBreedingMethod(foundBreedingMethodsById.get(request.getBreedingMethodDbId())); + } + if (request.getCollection() != null) + entity.setCollection(request.getCollection()); + if (request.getCommonCropName() != null) { + CropEntity crop = foundCropsByCropName.get(request.getCommonCropName()); + if (crop == null) { + crop = cropService.saveCropEntity(request.getCommonCropName()); + } + entity.setCrop(crop); + } + if (request.getCountryOfOriginCode() != null) + entity.setCountryOfOriginCode(request.getCountryOfOriginCode()); + if (request.getDefaultDisplayName() != null) + entity.setDefaultDisplayName(request.getDefaultDisplayName()); + if (request.getDocumentationURL() != null) + entity.setDocumentationURL(request.getDocumentationURL()); + if (request.getDonors() != null) + updateDonorEntities(request.getDonors(), entity); + if (request.getGenus() != null) + entity.setGenus(request.getGenus()); + if (request.getGermplasmName() != null) + entity.setGermplasmName(request.getGermplasmName()); + if (request.getGermplasmOrigin() != null) + updateOriginEntities(request.getGermplasmOrigin(), entity); + if (request.getGermplasmPreprocessing() != null) + entity.setGermplasmPreprocessing(request.getGermplasmPreprocessing()); + if (request.getGermplasmPUI() != null) + entity.setGermplasmPUI(request.getGermplasmPUI()); + if (request.getInstituteCode() != null || request.getInstituteName() != null) + entity.setHostInstitute(request.getInstituteCode(), request.getInstituteName()); + entity.setMlsStatus(MlsStatusEnum.EMPTY); + if (request.getPedigree() != null) { + if(entity.getPedigree() == null) { + entity.setPedigree(new PedigreeNodeEntity()); + } + entity.getPedigree().setPedigreeString(request.getPedigree()); + } + if (request.getSeedSource() != null) + entity.setSeedSource(request.getSeedSource()); + if (request.getSeedSourceDescription() != null) + entity.setSeedSourceDescription(request.getSeedSourceDescription()); + if (request.getSpecies() != null) + entity.setSpecies(request.getSpecies()); + if (request.getSpeciesAuthority() != null) + entity.setSpeciesAuthority(request.getSpeciesAuthority()); + if (request.getStorageTypes() != null) + entity.setTypeOfGermplasmStorageCode(request.getStorageTypes().stream().filter(Objects::nonNull) + .map(GermplasmStorageTypes::getCode).collect(Collectors.toList())); + if (request.getSubtaxa() != null) + entity.setSubtaxa(request.getSubtaxa()); + if (request.getSubtaxaAuthority() != null) + entity.setSubtaxaAuthority(request.getSubtaxaAuthority()); + if (request.getSynonyms() != null) + updateSynonymEntities(request.getSynonyms(), entity); + if (request.getTaxonIds() != null) + updateTaxonEntities(request.getTaxonIds(), entity); + + toSave.add(entity); + } + return toSave; + } + private void updateEntity(GermplasmEntity entity, GermplasmNewRequest request) throws BrAPIServerException { UpdateUtility.updateEntity(request, entity); From c430ebfd9e7affc06d8298d34e66f89e30e0e80b Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 21 Mar 2025 13:28:16 -0400 Subject: [PATCH 09/18] Apply Map optimizations on PedigreeService --- .../service/germ/PedigreeService.java | 56 +++++++++---------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 24e38961..7c4fd011 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -4,7 +4,6 @@ import java.util.Map.Entry; import java.util.stream.Collectors; -import io.swagger.model.IndexPagination; import org.apache.commons.lang3.tuple.Pair; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerDbIdNotFoundException; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException; @@ -300,10 +299,8 @@ public List savePedigreeNodes(List request, throw new BrAPIServerException(HttpStatus.BAD_REQUEST, errorMsg); } - // TODO: Batch this List newEntities = createEntitiesInBatch(request); // save all the new nodes without edges - //TODO: Fix to save nodes and edges at same time pedigreeRepository.saveAll(newEntities); Map updateRequest = new HashMap<>(); @@ -341,7 +338,6 @@ public List updatePedigreeNodes(Map request, } } - // First, update the basic properties of the nodes in batch. updateEntitiesWithEdgesInBatch(entityDtoPairsByGermplasmId); if (returnValues) { @@ -586,20 +582,28 @@ private void updateEntity(PedigreeNodeEntity entity, PedigreeNode node) throws B // This method should be used in use cases where there are no existing node entities representing the list being passed through. private List createEntitiesInBatch(List nodes) throws BrAPIServerException { - List germIds = nodes.stream().map(PedigreeNode::getGermplasmDbId).collect(Collectors.toList()); - List crossingProjIds = nodes.stream().map(PedigreeNode::getCrossingProjectDbId).collect(Collectors.toList()); + List germIds = nodes.stream() + .map(PedigreeNode::getGermplasmDbId) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + List crossingProjIds = nodes.stream() + .map(PedigreeNode::getCrossingProjectDbId) + .filter(Objects::nonNull) + .collect(Collectors.toList()); - List germs = germplasmService.findByIds(germIds); - List crossingProjs = crossingProjectService.findCrossingProjectsByIds(crossingProjIds); + Map foundGermsById = germplasmService.findByIds(germIds) + .stream() + .collect(Collectors.toMap(BrAPIBaseEntity::getId, e -> e)); + Map foundCrossingProjsById = crossingProjectService.findCrossingProjectsByIds(crossingProjIds) + .stream() + .collect(Collectors.toMap(BrAPIBaseEntity::getId, e -> e)); - List result = new ArrayList(); + List result = new ArrayList<>(); for (PedigreeNode node : nodes) { PedigreeNodeEntity entity = new PedigreeNodeEntity(); if (node.getGermplasmDbId() != null) { - String germId = node.getGermplasmDbId(); - Optional germEntity = germs.stream().filter(ge -> ge.getId().equals(germId)).findFirst(); - germEntity.ifPresent(entity::setGermplasm); + entity.setGermplasm(foundGermsById.get(node.getGermplasmDbId())); } UpdateUtility.updateEntity(node, entity); @@ -612,9 +616,7 @@ private List createEntitiesInBatch(List nodes) entity.setPedigreeString(node.getPedigreeString()); if (node.getCrossingProjectDbId() != null) { - String cpId = node.getCrossingProjectDbId(); - Optional crossingProjectEntity = crossingProjs.stream().filter(cp -> cp.getId().equals(cpId)).findFirst(); - crossingProjectEntity.ifPresent(entity::setCrossingProject); + entity.setCrossingProject(foundCrossingProjsById.get(node.getCrossingProjectDbId())); } result.add(entity); @@ -645,15 +647,15 @@ private void updateEntitiesWithEdgesInBatch(Map germs = new ArrayList<>(); - List crossingProjs = new ArrayList<>(); + new HashMap<>(); - if (!germIds.isEmpty()) { - germs = germplasmService.findByIds(germIds); - } - if (!crossingProjIds.isEmpty()) { - crossingProjs = crossingProjectService.findCrossingProjectsByIds(crossingProjIds); - } + Map foundGermsById = germplasmService.findByIds(germIds) + .stream() + .collect(Collectors.toMap(BrAPIBaseEntity::getId, e -> e)); + + Map foundCrossingProjsById = crossingProjectService.findCrossingProjectsByIds(crossingProjIds) + .stream() + .collect(Collectors.toMap(BrAPIBaseEntity::getId, e -> e)); if (!germIdsWithParentNodes.isEmpty()) { updateParentEdges(germIdsWithParentNodes, entityDtoPairsByGermId); @@ -668,9 +670,7 @@ private void updateEntitiesWithEdgesInBatch(Map germEntity = germs.stream().filter(ge -> ge.getId().equals(germId)).findFirst(); - germEntity.ifPresent(entity::setGermplasm); + entity.setGermplasm(foundGermsById.get(node.getGermplasmDbId())); } UpdateUtility.updateEntityCheckExRefs(node, entity); @@ -683,9 +683,7 @@ private void updateEntitiesWithEdgesInBatch(Map crossingProjectEntity = crossingProjs.stream().filter(cp -> cp.getId().equals(cpId)).findFirst(); - crossingProjectEntity.ifPresent(entity::setCrossingProject); + entity.setCrossingProject(foundCrossingProjsById.get(node.getCrossingProjectDbId())); } } } From 48bd5073a389b8455acba05078b25ea4189a7489 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 21 Mar 2025 15:25:01 -0400 Subject: [PATCH 10/18] Fix bugs related to default pagination --- .../repository/germ/GermplasmRepository.java | 4 ++++ .../service/germ/GermplasmService.java | 23 +++---------------- .../service/germ/PedigreeService.java | 10 ++++++-- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java index 7c5d9df6..fa731dd5 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java @@ -19,4 +19,8 @@ public interface GermplasmRepository extends BrAPIRepository germplasmIds, @Param("softDeleted") boolean softDeleted); + + List findByIdIn(List ids); + + List findByGermplasmNameIn(List germplasmNames); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java index 319dc9b6..d6ad45a2 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java @@ -771,30 +771,13 @@ private void updateSynonymEntities(List synonyms, G } } - public List findByNames(List germplasmNames) - throws BrAPIServerException { - GermplasmSearchRequest request = new GermplasmSearchRequest().germplasmNames(germplasmNames); - Metadata metadata = new Metadata().pagination(new IndexPagination()); - Page page = findGermplasmEntities(request, metadata); - - if (page.hasContent()) { - return page.getContent(); - } - - return null; + public List findByNames(List germplasmNames) { + return germplasmRepository.findByGermplasmNameIn(germplasmNames); } public List findByIds(List germplasmDbIds) throws BrAPIServerException { - GermplasmSearchRequest request = new GermplasmSearchRequest().germplasmDbIds(germplasmDbIds); - Metadata metadata = new Metadata().pagination(new IndexPagination()); - Page page = findGermplasmEntities(request, metadata); - - if (!page.hasContent()) { - return null; - } - - List germsFoundInDb = page.getContent(); + List germsFoundInDb = germplasmRepository.findByIdIn(germplasmDbIds); Set germIdsFoundInDB = germsFoundInDb.stream() .map(BrAPIBaseEntity::getId) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 7c4fd011..c0b9c8aa 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -4,6 +4,8 @@ import java.util.Map.Entry; import java.util.stream.Collectors; +import io.swagger.model.IndexPagination; +import io.swagger.model.Pagination; import org.apache.commons.lang3.tuple.Pair; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerDbIdNotFoundException; import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException; @@ -406,10 +408,14 @@ private Map getExistingPedigreeNodes(List ge searchReq.setPedigreeDepth(0); searchReq.setProgenyDepth(0); - List nodeEntities = findPedigreeEntities(searchReq, null); + Pagination pagination = new IndexPagination(); + pagination.setPageSize(germplasmDbIds.size()); + Metadata metadata = new Metadata(); + metadata.setPagination(pagination); + + List nodeEntities = findPedigreeEntities(searchReq, metadata); for (PedigreeNodeEntity nodeEntity : nodeEntities) { - // TODO: nodeEntity.getGermplasm() causes a lazy load if (nodeEntity.getGermplasm() != null && nodeEntity.getGermplasm().getId() != null) { nodesByGermplasm.put(nodeEntity.getGermplasm().getId(), nodeEntity); } From 6af2e8f8a3f811d1e68271dd5b49923d5605f0f0 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 21 Mar 2025 15:26:11 -0400 Subject: [PATCH 11/18] Add batching props to application.properties --- src/main/resources/application.properties.template | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/resources/application.properties.template b/src/main/resources/application.properties.template index 8baf2928..be2b1dc8 100644 --- a/src/main/resources/application.properties.template +++ b/src/main/resources/application.properties.template @@ -12,6 +12,10 @@ spring.flyway.baselineOnMigrate=true spring.datasource.driver-class-name=org.postgresql.Driver +spring.jpa.properties.hibernate.jdbc.batch_size=50 +spring.jpa.properties.hibernate.order_inserts=true +spring.jpa.properties.hibernate.order_updates=true +spring.jpa.properties.hibernate.order_deletes=true spring.jpa.hibernate.ddl-auto=validate spring.jpa.show-sql=false From 9b54e8d79743b75ab9cb6604c868a8a55bbc616b Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 21 Mar 2025 15:55:48 -0400 Subject: [PATCH 12/18] Remove erroneous property --- src/main/resources/application.properties.template | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/resources/application.properties.template b/src/main/resources/application.properties.template index be2b1dc8..989bd8f6 100644 --- a/src/main/resources/application.properties.template +++ b/src/main/resources/application.properties.template @@ -15,7 +15,6 @@ spring.datasource.driver-class-name=org.postgresql.Driver spring.jpa.properties.hibernate.jdbc.batch_size=50 spring.jpa.properties.hibernate.order_inserts=true spring.jpa.properties.hibernate.order_updates=true -spring.jpa.properties.hibernate.order_deletes=true spring.jpa.hibernate.ddl-auto=validate spring.jpa.show-sql=false From d12ce0cbae95bac3c7c4ae4ba06a5f7227498c88 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 21 Mar 2025 16:05:25 -0400 Subject: [PATCH 13/18] Add additional properties for debugging, with comments --- src/main/resources/application.properties.template | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/resources/application.properties.template b/src/main/resources/application.properties.template index 989bd8f6..2e0166a2 100644 --- a/src/main/resources/application.properties.template +++ b/src/main/resources/application.properties.template @@ -12,11 +12,20 @@ spring.flyway.baselineOnMigrate=true spring.datasource.driver-class-name=org.postgresql.Driver +# This property when set to true makes it so that a DB transaction is open through the body of a request, nullifying the use of @Transactional. +# It is generally recommended that this be set to false, and methods are properly annotated and release the transactions when complete. +# However, many of the endpoints already rely on this kind of connection infrastructure and changing is more trouble than it's worth. +spring.jpa.open-in-view=true + spring.jpa.properties.hibernate.jdbc.batch_size=50 spring.jpa.properties.hibernate.order_inserts=true spring.jpa.properties.hibernate.order_updates=true spring.jpa.hibernate.ddl-auto=validate + +# Use these to help debug queries. +# The stats will tell you how long hibernate transactions are taking, how many queries occur, how many entities are being flushed/accessed, etc. spring.jpa.show-sql=false +spring.jpa.properties.hibernate.generate_statistics=false spring.mvc.dispatch-options-request=true From b0a5d4b9c54689c288158b01dae40da533fc0e31 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Mon, 24 Mar 2025 11:50:10 -0400 Subject: [PATCH 14/18] Add general findByIdIn to BrAPIRepository to be used by importer --- .../repository/BrAPIRepository.java | 3 +++ .../repository/BrAPIRepositoryImpl.java | 4 ++++ .../germ/BreedingMethodRepository.java | 6 +----- .../germ/CrossingProjectRepository.java | 1 - .../repository/germ/GermplasmRepository.java | 2 -- .../service/germ/CrossingProjectService.java | 21 ++----------------- 6 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepository.java index e038360c..3a796a07 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepository.java @@ -10,6 +10,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.NoRepositoryBean; @NoRepositoryBean @@ -30,4 +31,6 @@ public interface BrAPIRepository void refresh(S entity); public void fetchXrefs(Page page, Class searchClass) throws InvalidPagingException; + + List findByIdIn(List ids); } \ No newline at end of file diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepositoryImpl.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepositoryImpl.java index 9bfa577c..3de292d0 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepositoryImpl.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepositoryImpl.java @@ -119,6 +119,10 @@ public Optional findById(ID id) { return response; } + public List findByIdIn(List ids) { + return super.findAllById(ids); + } + public S save(S entity) { entity.setAuthUserId(SecurityUtils.getCurrentUserId()); return super.save(entity); diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java index 06f4c4d4..e090aeb6 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/BreedingMethodRepository.java @@ -3,9 +3,5 @@ import org.brapi.test.BrAPITestServer.model.entity.germ.BreedingMethodEntity; import org.brapi.test.BrAPITestServer.repository.BrAPIRepository; -import java.util.List; -import java.util.Set; - -public interface BreedingMethodRepository extends BrAPIRepository{ - public List findByIdIn(List id); +public interface BreedingMethodRepository extends BrAPIRepository { } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/CrossingProjectRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/CrossingProjectRepository.java index 998ee194..0ceb9d8f 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/CrossingProjectRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/CrossingProjectRepository.java @@ -4,5 +4,4 @@ import org.brapi.test.BrAPITestServer.repository.BrAPIRepository; public interface CrossingProjectRepository extends BrAPIRepository { - } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java index fa731dd5..62901285 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/repository/germ/GermplasmRepository.java @@ -20,7 +20,5 @@ public interface GermplasmRepository extends BrAPIRepository germplasmIds, @Param("softDeleted") boolean softDeleted); - List findByIdIn(List ids); - List findByGermplasmNameIn(List germplasmNames); } diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java index 1e5fff4c..318b0c5b 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/CrossingProjectService.java @@ -70,25 +70,8 @@ public List findCrossingProjects(String crossingProjectDbId, St return crossingProjects; } - public List findCrossingProjectsByIds(List crossingProjectIds) - throws BrAPIServerException { - Metadata metadata = new Metadata().pagination(new IndexPagination()); - Pageable pageReq = PagingUtility.getPageRequest(metadata); - - SearchQueryBuilder searchQuery = new SearchQueryBuilder( - CrossingProjectEntity.class); - - if (crossingProjectIds != null && !crossingProjectIds.isEmpty()) { - searchQuery = searchQuery.appendList(crossingProjectIds, "id"); - } - - Page page = crossingProjectRepository.findAllBySearchAndPaginate(searchQuery, pageReq); - - if (page.hasContent()) { - return page.getContent(); - } - - return null; + public List findCrossingProjectsByIds(List crossingProjectIds) { + return crossingProjectRepository.findByIdIn(crossingProjectIds); } public CrossingProject getCrossingProject(String crossingProjectDbId) throws BrAPIServerException { From 4836f3340fbf99185df883384cd0b54517d52699 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Thu, 3 Apr 2025 17:17:43 -0400 Subject: [PATCH 15/18] Simplify resultingNodes logic, fix connectedNode typo --- .../BrAPITestServer/service/germ/PedigreeService.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index c0b9c8aa..13521b29 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -195,13 +195,12 @@ public List getPedigreeNodes(List germplasmDbIds) { public List findOrCreatePedigreeNodesFromGermplasmIds(List germplasmDbIds) throws BrAPIServerException { - List dbNodes = getPedigreeNodes(germplasmDbIds); + // First, grab nodes from the DB that match the germplasmDbIds passed through. + List resultingNodes = getPedigreeNodes(germplasmDbIds); - List resultingNodes = new ArrayList<>(dbNodes); - - // Find out which germIds were not found in the DB. Use a set for improved performance on the contains check. + // Next, find out which germIds were not found in the DB. Use a set for improved performance on the contains check. // TODO: Check if the germEntity is already populated by getPedigreeNodes, and if this block results in more DB transactions. - Set germIdsOfFoundNodes = dbNodes.stream() + Set germIdsOfFoundNodes = resultingNodes.stream() .map(PedigreeNodeEntity::getGermplasm) .map(BrAPIBaseEntity::getId) .collect(Collectors.toSet()); @@ -758,7 +757,7 @@ private void updateChildEdges(List germIdsWithProgenyNodes, SearchQueryBuilder search = new SearchQueryBuilder(PedigreeEdgeEntity.class); - search.appendList(germIdsWithProgenyNodes, "conncetedNode.germplasm.id"); + search.appendList(germIdsWithProgenyNodes, "connected.germplasm.id"); search.appendEnum(PedigreeEdgeEntity.EdgeType.parent, "edgeType"); List existingProgenyEdges = pedigreeEdgeRepository.findAllBySearch(search); From fb1a01f9fd0526667a79946d67bb75487e245e6f Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Thu, 3 Apr 2025 19:02:09 -0400 Subject: [PATCH 16/18] Handle empty name search use case in GermService, add mother/father warn logs --- .../BrAPITestServer/service/germ/GermplasmService.java | 8 +++++++- .../BrAPITestServer/service/germ/PedigreeService.java | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java index 2df62f1e..db46885a 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/GermplasmService.java @@ -772,7 +772,13 @@ private void updateSynonymEntities(List synonyms, G } public List findByNames(List germplasmNames) { - return germplasmRepository.findByGermplasmNameIn(germplasmNames); + List foundGerms = new ArrayList<>(); + + if (!germplasmNames.isEmpty()) { + foundGerms = germplasmRepository.findByGermplasmNameIn(germplasmNames); + } + + return foundGerms; } public List findByIds(List germplasmDbIds) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 13521b29..5ba87ece 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -827,8 +827,17 @@ public List convertFromGermplasmToPedigreeBatchUsingNames(List motherGerms = germplasmService.findByNames(new ArrayList<>(germsByPedigreeMother.values())); + + if (motherGerms.isEmpty() && !germsByPedigreeMother.isEmpty()) { + log.warn("Could not find any germplasms looking up with mother names {}", germsByPedigreeMother.values()); + } + List fatherGerms = germplasmService.findByNames(new ArrayList<>(germsByPedigreeFather.values())); + if (fatherGerms.isEmpty() && !germsByPedigreeFather.isEmpty()) { + log.warn("Could not find any germplasms looking up with father names {}", germsByPedigreeFather.values()); + } + for (Germplasm germplasm : germplasms) { GermplasmEntity motherGerm = null; GermplasmEntity fatherGerm = null; From 449263f8e2c269a5befeefcdd61d8ff895091710 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 4 Apr 2025 11:47:26 -0400 Subject: [PATCH 17/18] Fix mispell for updateChildEdges method --- .../test/BrAPITestServer/service/germ/PedigreeService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 5ba87ece..0d1c5e7c 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -757,7 +757,7 @@ private void updateChildEdges(List germIdsWithProgenyNodes, SearchQueryBuilder search = new SearchQueryBuilder(PedigreeEdgeEntity.class); - search.appendList(germIdsWithProgenyNodes, "connected.germplasm.id"); + search.appendList(germIdsWithProgenyNodes, "connectedNode.germplasm.id"); search.appendEnum(PedigreeEdgeEntity.EdgeType.parent, "edgeType"); List existingProgenyEdges = pedigreeEdgeRepository.findAllBySearch(search); From e1051b1c830171869a20367eb6c38af7be596bb5 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Wed, 23 Apr 2025 12:33:20 -0400 Subject: [PATCH 18/18] Rename gId to dbId in PedigreeService --- .../test/BrAPITestServer/service/germ/PedigreeService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java index 0d1c5e7c..613ba5dd 100644 --- a/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java +++ b/src/main/java/org/brapi/test/BrAPITestServer/service/germ/PedigreeService.java @@ -206,7 +206,7 @@ public List findOrCreatePedigreeNodesFromGermplasmIds(List germIdsWithNoPedigreeRecord = germplasmDbIds.stream() - .filter(gId -> !germIdsOfFoundNodes.contains(gId)) + .filter(dbId -> !germIdsOfFoundNodes.contains(dbId)) .collect(Collectors.toList()); // Now see if there are germplasm records that exist from the list created above that have a pedigree associated.