From 73a876cc142f296ffebe2fe2a8ea10a9bbd65027 Mon Sep 17 00:00:00 2001 From: Jay Allen <107942890+jallentxbiomed@users.noreply.github.com> Date: Tue, 24 Sep 2024 11:02:40 -0500 Subject: [PATCH] Handle getEvent processing where multiple top-level EventData entries have the same SuperPkgId (#233) * added logic for finding duplicate superPkgIds * updated some comments * updated comments for better understanding of code * added check for existing eventDataId in map --- src/org/labkey/snd/SNDManager.java | 330 +++++++++++++++++------------ 1 file changed, 196 insertions(+), 134 deletions(-) diff --git a/src/org/labkey/snd/SNDManager.java b/src/org/labkey/snd/SNDManager.java index 93338a9..2a84eae 100644 --- a/src/org/labkey/snd/SNDManager.java +++ b/src/org/labkey/snd/SNDManager.java @@ -22,6 +22,7 @@ import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.collections4.ListUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -99,6 +100,7 @@ import java.util.Comparator; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -3743,21 +3745,21 @@ public List> getProjectItemsList(Container c, User u, int pr /** - * Query the EventData table and create a Map of all top level SuperPackages for a set of eventIds + * Retrieves all top-level SuperPackages for a given set of event IDs, maps them by EventDataId, and groups them by EventId. * - * @param c - * @param u - * @param eventIds - * @param superPackages - * @return + * @param c The container context. + * @param u The user performing the query. + * @param eventIds The list of event IDs to query. + * @param superPackages Map of SuperPackage objects, keyed by their SuperPkgId. + * @return A map where the key is EventId and the value is a map of EventDataId to its corresponding SuperPackage. */ private Map> getBulkTopLevelEventDataSuperPkgs(Container c, User u, List eventIds, Map superPackages) { - // EventData from query - SELECT * FROM EventData WHERE EventId IN {eventIds} AND ParentEventId IS NULL ORDER BY EventDataId + // Query the EventData table for the eventIds where ParentEventDataId is null (top-level) TableSelector eventDataSelector = getTableSelector(c, u, eventIds, SNDSchema.EVENTDATA_TABLE_NAME, Event.EVENT_ID, EventData.EVENT_DATA_ID, EventData.EVENT_DATA_PARENT_EVENTDATAID); List allEventData = eventDataSelector.getArrayList(EventData.class); - // Get SuperPackages for eventData and group by EventId + // Group SuperPackages by EventId and EventDataId, retrieving from the superPackages map. EventDataId and SuperPackage are 1:1 Map> topLevelEventDataSuperPkgs = allEventData.stream().collect( Collectors.groupingBy( EventData::getEventId, @@ -3773,14 +3775,15 @@ private Map> getBulkTopLevelEventDataSuperPk } /** - * getEvent method that uses cache-optimized methods for bulk loading to more quickly build a single event - * @param c - * @param u - * @param eventId - * @param narrativeOptions - * @param skipPermissionCheck - * @param errors - * @return + * Retrieves a single Event object, including narratives and other related data using the getBulkEvents method + * + * @param c The container context. + * @param u The user requesting the event. + * @param eventId The ID of the event to retrieve. + * @param narrativeOptions Set of narrative options for event generation. + * @param skipPermissionCheck If true, skip permission validation. + * @param errors A collection of validation errors. + * @return The requested Event object, or null if not found. */ @Nullable public Event getEvent(Container c, User u, int eventId, Set narrativeOptions, boolean skipPermissionCheck, BatchValidationException errors) { @@ -3792,12 +3795,15 @@ public Event getEvent(Container c, User u, int eventId, Set eventExtraFields = getExtraFields(c, u, SNDSchema.EVENTS_TABLE_NAME); List eventDataExtraFields = getExtraFields(c, u, SNDSchema.EVENTDATA_TABLE_NAME); List packageExtraFields = getExtraFields(c, u, SNDSchema.PKGS_TABLE_NAME); + // Retrieve all SuperPackages associated with a single event Map superPackages = getBulkSuperPkgs(c, u, packageExtraFields, pkgQus, eventId, lookups, errors); + // Map top-level EventDataIds to associated SuperPackage objects and group by EventId Map> topLevelEventDataSuperPkgs = getBulkTopLevelEventDataSuperPkgs(c, u, Collections.singletonList(eventId), superPackages); Event event = getBulkEvents(c, u, Collections.singletonList(eventId), narrativeOptions, topLevelEventDataSuperPkgs, skipPermissionCheck, errors, eventExtraFields, eventDataExtraFields, true).get(eventId); @@ -3807,18 +3813,19 @@ public Event getEvent(Container c, User u, int eventId, Set getBulkEvents(Container c, User u, List eventIds, Set narrativeOptions, @@ -3826,7 +3833,7 @@ public Map getBulkEvents(Container c, User u, List even List eventExtraFields, List eventDataExtraFields, boolean includeEmptySubPackages) { - // Events from query - SELECT * FROM Events WHERE EventId IN {eventIds} + // Query Events and EventData from the database TableSelector eventSelector = getTableSelector(c, u, eventIds, SNDSchema.EVENTS_TABLE_NAME, Event.EVENT_ID, null, null); List events = eventSelector.getArrayList(Event.class); Collection> eventsExtensibleFields = eventSelector.getMapCollection(); @@ -3835,16 +3842,28 @@ public Map getBulkEvents(Container c, User u, List even List rawEventData = eventDataSelector.getArrayList(EventData.class); Collection> eventDataExtensibleFields = eventDataSelector.getMapCollection(); - //EventNotes grouped by EventId + // Retrieve Event Notes and Project information Map eventNotes = getBulkEventNotes(c, u, eventIds); - - //ProjectIdRev strings grouped by Event ObjectId Map projectIdRevs = getBulkProjectIdRevs(c, u, events.stream().map(Event::getParentObjectId).collect(Collectors.toList())); - //EventData grouped by EventId - Map> eventData = getBulkEventData(c, topLevelSuperPkgs, rawEventData, eventDataExtensibleFields, eventDataExtraFields, includeEmptySubPackages); + /* + Transform top-level SuperPackages map to have the object structure for processing repeated top-level SuperPkgIds, + The value of the HashMap will now be stored as a Pair where Integer is the associated ParentEventDataId + */ + Map>> transformedTopLevelSuperPkgs = topLevelSuperPkgs.entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, // Keep outer key (eventId) + entry -> entry.getValue().entrySet().stream() // Stream over inner map entries + .collect(Collectors.toMap( + Map.Entry::getKey, // Keep inner key (eventDataId) + innerEntry -> Pair.of(innerEntry.getValue(), null) // ParentEventDataId will always be null at the top level + )) + )); - // Build events from eventData, eventNotes, and project data and group by EventId + // Process EventData grouped by EventId and build Event objects + Map> eventData = getBulkEventData(c, transformedTopLevelSuperPkgs, rawEventData, eventDataExtensibleFields, eventDataExtraFields, includeEmptySubPackages); + + // Final assembly of Event objects, setting narratives and attributes Map eventsById = events.stream().collect(Collectors.toMap(Event::getEventId, (Event event) -> { boolean hasPermission = skipPermissionCheck || SNDSecurityManager.get().hasPermissionForTopLevelSuperPkgs(c, u, topLevelSuperPkgs.get(event.getEventId()), event, QCStateActionEnum.READ); @@ -3855,6 +3874,7 @@ public Map getBulkEvents(Container c, User u, List even .findFirst() .orElse(Collections.emptyMap()); + // Set event notes, project revisions, and event data if (!event.hasErrors() && hasPermission) { event.setNote(eventNotes.get(event.getEventId())); event.setProjectIdRev(projectIdRevs.get(event.getParentObjectId())); @@ -3883,26 +3903,29 @@ public Map getBulkEvents(Container c, User u, List even } /** - * Return a cached map of SuperPackage objects by superPkgId - * Builds each underlying SuperPackage object with child superPackages - * Retrieves SuperPackages for an eventId if it is not null, else retrieves SuperPackages for entire database - * @param c - * @param u - * @param packageExtraFields - * @param pkgQus - * @param eventId - * @param lookups - * @param errors - * @return + * Return a cached map of SuperPackage objects by superPkgId. + * Builds each underlying SuperPackage object, including child superPackages. + * Retrieves SuperPackages for a specific eventId if provided, otherwise retrieves SuperPackages for the entire database. + * + * @param c Container object representing the context of the request. + * @param u User object representing the current user. + * @param packageExtraFields Extra fields related to the SuperPackage that need to be included in the response. + * @param pkgQus QueryUpdateService for handling SuperPackage updates. + * @param eventId The ID of the event for which SuperPackages should be retrieved. Can be null to retrieve all SuperPackages. + * @param lookups Map for additional lookup criteria or filtering, used to retrieve SuperPackages. + * @param errors BatchValidationException object used to accumulate any errors encountered during the process. + * + * @return A map of SuperPackage objects, keyed by SuperPkgId. */ private Map getBulkSuperPkgs(Container c, User u, List packageExtraFields, QueryUpdateService pkgQus, @Nullable Integer eventId, Map lookups, BatchValidationException errors) { UserSchema schema = getSndUserSchema(c, u); - // Get All SuperPkg objects from database + // Query database for SuperPackages for an eventId if it exists, otherwise for all SuperPackages SQLFragment sql = new SQLFragment( - "SELECT sp.SuperPkgId, sp.PkgId, sp.SortOrder, sp.Required, pkg.PkgId, pkg.Description, pkg.Active, pkg.Narrative, pkg.Repeatable FROM "); + "SELECT DISTINCT sp.SuperPkgId, sp.ParentSuperPkgId, sp.PkgId, sp.SortOrder, sp.Required, " + + "pkg.PkgId, pkg.Description, pkg.Active, pkg.Narrative, pkg.Repeatable FROM "); sql.append(schema.getTable(SNDSchema.SUPERPKGS_TABLE_NAME), "sp"); sql.append(" JOIN " + SNDSchema.NAME + "." + SNDSchema.PKGS_TABLE_NAME + " pkg"); sql.append(" ON sp.PkgId = pkg.PkgId "); @@ -3971,18 +3994,22 @@ private Map getBulkSuperPkgs(Container c, User u, List> getBulkChildSuperPkgs (Container c, User u, List allSuperPackages, Map> childrenByParentId, @@ -4019,57 +4046,61 @@ private Map> getBulkChildSuperPkgs (Container c, Use /** - * Populate eventData and attribute data for a set of top level SuperPackages - * @param c - * @param currentLevelSuperPkgs - * @param currentEventData - * @param eventDataExtensibleFields - * @param eventDataExtraFields - * @return + * Populate and retrieve bulk event data with attributes and subpackages for a set of top-level SuperPackages. + * + * @param c Container context for the request. + * @param currentLevelSuperPkgs Map of SuperPackages by eventId and eventDataId, paired with an optional parentEventDataId. + * @param currentEventData List of existing EventData objects. + * @param eventDataExtensibleFields Collection of additional extensible fields for the EventData. + * @param eventDataExtraFields List of extra fields for EventData. + * @param includeEmptySubPackages Whether to include empty subpackages for EventData that don't already have them populated. + * @return A Map of eventIds to their corresponding list of EventData objects. */ - private Map> getBulkEventData(Container c, Map> currentLevelSuperPkgs, + private Map> getBulkEventData(Container c, Map>> currentLevelSuperPkgs, List currentEventData, Collection> eventDataExtensibleFields, List eventDataExtraFields, boolean includeEmptySubPackages) { - if (currentLevelSuperPkgs == null) { return null; } + // Collect all eventDataIds from the SuperPackages List eventDataIds = currentLevelSuperPkgs.values() .stream() - .flatMap((Map map) -> map.keySet().stream()) + .flatMap((Map> map) -> map.keySet().stream()) .collect(Collectors.toList()); + // Collect existing eventDataIds List originalEventDataIds = currentEventData.stream().filter(e -> e.getEventDataId() != null).map(EventData::getEventDataId).toList(); List allEventData = new ArrayList<>(currentEventData); + // Generate empty EventData objects for eventDataIds in the currentLevelSuperPkgs map not in the existing list if (includeEmptySubPackages) { List emptyEventData = currentLevelSuperPkgs.entrySet().stream() .flatMap(outerEntry -> outerEntry.getValue().entrySet().stream().map(innerEntry -> Map.entry(outerEntry.getKey(), innerEntry))) .filter(entry -> !originalEventDataIds.contains(entry.getValue().getKey())) .map(entry -> { EventData eventData = new EventData(); - eventData.setEventDataId(entry.getValue().getKey()); eventData.setEventId(entry.getKey()); - eventData.setSuperPkgId(entry.getValue().getValue().getSuperPkgId()); + eventData.setEventDataId(entry.getValue().getKey()); + eventData.setSuperPkgId(entry.getValue().getValue().getLeft().getSuperPkgId()); + eventData.setParentEventDataId(entry.getValue().getValue().getRight()); return eventData; }) .toList(); - allEventData.addAll(emptyEventData); } - // Child EventData grouped by EventId + // Group child eventData by EventId Map> childEventData = getBulkChildEventData(allEventData, eventDataIds); - // Build eventData from attributes and superPkgs and group by eventId + // Build and map eventData objects, grouping them by EventId Map> eventDataByEventId = allEventData.stream().filter(e -> eventDataIds.contains(e.getEventDataId())).map((EventData eventData) -> { Map properties = OntologyManager.getPropertyObjects(c, eventData.getObjectURI()); - Map superPackagesByEventDataId = currentLevelSuperPkgs.get(eventData.getEventId()); - SuperPackage superPackage = superPackagesByEventDataId.get(eventData.getEventDataId()); + Map> superPackagesByEventDataId = currentLevelSuperPkgs.get(eventData.getEventId()); + SuperPackage superPackage = superPackagesByEventDataId.get(eventData.getEventDataId()).getLeft(); if (superPackage != null) { List attributeData = superPackage.getPkg().getAttributes() @@ -4090,21 +4121,22 @@ private Map> getBulkEventData(Container c, Map> nextLevelSuperPkgs = getNextLevelEventDataSuperPkgs(eventData, childEventData, currentLevelSuperPkgs, includeEmptySubPackages, hasSubpackages); + // Recursively process next level of subpackages if they exist + Map>> nextLevelSuperPkgs = getNextLevelEventDataSuperPkgs(eventData, childEventData, currentLevelSuperPkgs, includeEmptySubPackages, hasSubpackages); if (nextLevelSuperPkgs != null && !nextLevelSuperPkgs.get(eventData.getEventId()).isEmpty()) { - // Recursion for next child level of sub packages Map> subEventData = getBulkEventData(c, nextLevelSuperPkgs, currentEventData, eventDataExtensibleFields, eventDataExtraFields, includeEmptySubPackages); if (subEventData != null && subEventData.containsKey(eventData.getEventId())) { List sorted = subEventData.get(eventData.getEventId()).stream().sorted(Comparator.comparing( - (EventData child) -> nextLevelSuperPkgs.get(child.getEventId()).get(child.getEventDataId()).getTreePath())) + (EventData child) -> nextLevelSuperPkgs.get(child.getEventId()).get(child.getEventDataId()).getLeft().getTreePath())) .filter(ed -> ed.getParentEventDataId() == null || ed.getParentEventDataId().equals(eventData.getEventDataId())) .map(ed -> { if (!originalEventDataIds.contains(ed.getEventDataId())) { ed.setEventId(0); ed.setEventDataId(null); + ed.setParentEventDataId(null); } return ed; }) @@ -4115,16 +4147,17 @@ private Map> getBulkEventData(Container c, Map> getBulkChildEventData(List allEventData, List parentEventDataIds) { @@ -4132,20 +4165,20 @@ private Map> getBulkChildEventData(List allE return Collections.emptyMap(); } - List childEventData = allEventData.stream().filter(e -> parentEventDataIds.contains(e.getParentEventDataId())).toList(); - - // Group childEventData by eventId - Map> childEventDataByEventId = childEventData.stream().collect(Collectors.groupingBy(EventData::getEventId)); + // Filter child EventData based on parentEventDataIds and group by EventId + Map> childEventDataByEventId = allEventData.stream() + .filter(e -> parentEventDataIds.contains(e.getParentEventDataId())) + .collect(Collectors.groupingBy(EventData::getEventId)); return childEventDataByEventId; } /** - * Get the attribute data for a given PropertyDescriptor object + * Create an AttributeData object for the specified PropertyDescriptor. * - * @param propertyDescriptor - * @param properties - * @return + * @param propertyDescriptor The property descriptor for the attribute. + * @param properties The map of property URIs to object properties. + * @return The AttributeData object with formatted values. */ private AttributeData getAttributeData(GWTPropertyDescriptor propertyDescriptor, Map properties) { @@ -4154,6 +4187,7 @@ private AttributeData getAttributeData(GWTPropertyDescriptor propertyDescriptor, attribute.setPropertyName(propertyDescriptor.getName()); attribute.setPropertyDescriptor(propertyDescriptor); attribute.setPropertyId(propertyDescriptor.getPropertyId()); + // Retrieve and format the property value if it exists if (properties.get(propertyDescriptor.getPropertyURI()) != null) { property = properties.get(propertyDescriptor.getPropertyURI()).value(); if (property != null) { @@ -4172,16 +4206,20 @@ private AttributeData getAttributeData(GWTPropertyDescriptor propertyDescriptor, } /** - * Get the next child level of SuperPackages for a given map of top level SuperPackages + * Retrieves the next level of child SuperPackages for a given map of current level SuperPackages by EventDataId. * - * @param eventData - * @param childEventData - * @param currentLevelSuperPkgs - * @return + * @param eventData The current eventData object being processed + * @param childEventData A map of current level child event data keyed by the event ID. + * @param currentLevelSuperPkgs A map containing the current level of SuperPackages, grouped by event ID and event data ID. + * @param includeEmptySubPackages Boolean flag indicating whether empty subpackages should be included. + * @param hasSubpackages Boolean flag to check if there are any subpackages associated with the eventData. + * + * @return A map of the next level SuperPackages grouped by event ID and event data ID. */ - private Map> getNextLevelEventDataSuperPkgs(EventData eventData, Map> childEventData, Map> currentLevelSuperPkgs, - boolean includeEmptySubPackages, boolean hasSubpackages) { + private Map>> getNextLevelEventDataSuperPkgs(EventData eventData, Map> childEventData, Map>> currentLevelSuperPkgs, + boolean includeEmptySubPackages, boolean hasSubpackages) + { if ((!includeEmptySubPackages && !childEventData.containsKey(eventData.getEventId())) || (includeEmptySubPackages && !hasSubpackages)) { return null; @@ -4191,52 +4229,76 @@ private Map> getNextLevelEventDataSuperPkgs( Map childSuperPkgs = currentLevelSuperPkgs .getOrDefault(eventData.getEventId(), Collections.emptyMap()) .get(eventData.getEventDataId()) + .getLeft() .getChildPackages() .stream() .collect(Collectors.toMap( - SuperPackage::getSuperPkgId, - (SuperPackage superPackage) -> superPackage, - (s1, s2) -> s1 + SuperPackage::getSuperPkgId, // Key by SuperPkgId + (SuperPackage superPackage) -> superPackage, // Value is SuperPackage object + (s1, s2) -> s1 // Keep first key in case of duplicates (there should not be more than one SuperPkg per EventDataId) )); - // Get superPkg for eventData and group by eventId and then by eventId - Map> nextLevelEventDataSuperPkgs = new HashMap<>(); + // Prepare a map for the next level of SuperPackages + Map>> nextLevelEventDataSuperPkgs = new HashMap<>(); nextLevelEventDataSuperPkgs.put(eventData.getEventId(), new HashMap<>()); - if (!childSuperPkgs.isEmpty() && !childEventData.isEmpty()) { - Map children = childSuperPkgs; - nextLevelEventDataSuperPkgs = childEventData.get(eventData.getEventId()) - .stream() - .filter((EventData child) -> children.containsKey(child.getSuperPkgId())) - .collect( - Collectors.groupingBy( - EventData::getEventId, - Collectors.toMap( - EventData::getEventDataId, - (EventData child) -> children.get(child.getSuperPkgId()) - ) - ) - ); - } - - if (!nextLevelEventDataSuperPkgs.containsKey(eventData.getEventId())) { - nextLevelEventDataSuperPkgs.put(eventData.getEventId(), new HashMap<>()); - } + // Iterate over child event data and link it to the corresponding child SuperPackage + childEventData.getOrDefault(eventData.getEventId(), Collections.emptyList()).stream() + .filter(child -> childSuperPkgs.containsKey(child.getSuperPkgId())) // Only include subpackages from the specific SuperPkg structure + .forEach(child -> { + nextLevelEventDataSuperPkgs.get(eventData.getEventId()) + .put(child.getEventDataId(), Pair.of(childSuperPkgs.get(child.getSuperPkgId()), child.getParentEventDataId())); + }); + // Ensure the eventDataSuperPkgs map is initialized for the current EventId + nextLevelEventDataSuperPkgs.computeIfAbsent(eventData.getEventId(), k -> new HashMap<>()); if (includeEmptySubPackages) { + // Collect the SuperPackage IDs from the current level + List currentLevelSuperPkgIds = currentLevelSuperPkgs.get(eventData.getEventId()) + .values() + .stream() + .map(pair -> pair.getLeft().getSuperPkgId()).toList(); + + // Identify repeated SuperPackage IDs + Set uniqueIds = new HashSet<>(); + List repeatedIds = currentLevelSuperPkgIds + .stream() + .filter(superPkgId -> !uniqueIds.add(superPkgId)) + .toList(); - List eventDataSuperPkgIds = !nextLevelEventDataSuperPkgs.get(eventData.getEventId()).isEmpty() - ? nextLevelEventDataSuperPkgs.get(eventData.getEventId()).values().stream().map(SuperPackage::getSuperPkgId).toList() - : new ArrayList<>(); + // Handle next level logic for repeated IDs and empty subpackages + List nextLevelSuperPkgIds = nextLevelEventDataSuperPkgs.get(eventData.getEventId()) + .values() + .stream() + .map(pair -> pair.getLeft().getSuperPkgId()).toList(); + + AtomicInteger missingEventDataId = new AtomicInteger(0); + + if (!repeatedIds.isEmpty()) { + // Group subpackage superPkgIds for the repeated parent superPkgs by their parentEventDataId + Map> childSuperPkgIdsByParentEventDataId = childEventData.get(eventData.getEventId()).stream() + .filter(e -> e.getParentEventDataId() != null + && nextLevelEventDataSuperPkgs.get(eventData.getEventId()).containsKey(e.getEventDataId()) + && repeatedIds.contains(nextLevelEventDataSuperPkgs.get(eventData.getEventId()).get(e.getEventDataId()).getLeft().getParentSuperPkgId())) // Ensure parentEventDataId exists + .collect(Collectors.groupingBy( + EventData::getParentEventDataId, // Group by parentEventDataId + Collectors.mapping(EventData::getSuperPkgId, Collectors.toList()) // Collect superPackageId in a list + )); + // Add the missing SuperPackages for the next level for each Grouped set of child SuperPkgIds + childSuperPkgIdsByParentEventDataId.entrySet().forEach(entry -> { + childSuperPkgs.values().stream() + .filter(spkg -> !entry.getValue().contains(spkg.getSuperPkgId())) + .forEach(spkg -> nextLevelEventDataSuperPkgs.get(eventData.getEventId()).put(missingEventDataId.getAndIncrement(), + Pair.of(spkg, entry.getKey()))); + }); + } - AtomicInteger emptyEventDataId = new AtomicInteger(0); - Map> eventDataSuperPkgs = nextLevelEventDataSuperPkgs; + // Add any remaining missing SuperPackages that are not part of the repeated IDs childSuperPkgs.values().stream() - .filter(superPkg -> !eventDataSuperPkgIds.contains(superPkg.getSuperPkgId())) - .forEach(spkg -> { - eventDataSuperPkgs.get(eventData.getEventId()).put(emptyEventDataId.getAndAdd(1), spkg); - }); + .filter(superPkg -> !nextLevelSuperPkgIds.contains(superPkg.getSuperPkgId()) && !repeatedIds.contains(superPkg.getParentSuperPkgId())) + .forEach(spkg -> nextLevelEventDataSuperPkgs.get(eventData.getEventId()).put(missingEventDataId.getAndIncrement(), + Pair.of(spkg, null))); } return nextLevelEventDataSuperPkgs;