Skip to content

Stream StudyCachables in QueryHelper #7896

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
labkey-adam merged 3 commits into from
Jul 30, 2026
Merged

Stream StudyCachables in QueryHelper #7896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c106e0
Stream StudyCachables in QueryHelper
labkey-adam Jul 30, 2026
0d4cc29
Close stream before constructors do their work
labkey-adam Jul 30, 2026
fd9d5a8
This is what I meant...
labkey-adam Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/data/Selector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public interface Selector

/**
* Returns a sequential Stream of objects or records representing rows from the database. Converts each result row
* into an object the specified {@code Class}. The Stream is backed by a cached data structure (ResultSet and
* into an object of the specified {@code Class}. The Stream is backed by a cached data structure (ResultSet and
* Connection are closed before returning the stream), so no need to close or fully exhaust this stream. Cached
* streams are more convenient to use than uncached streams and should perform well in low-volume situations.
*/
Expand Down
32 changes: 20 additions & 12 deletions study/api-src/org/labkey/api/study/QueryHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@
import org.labkey.api.data.TableInfoGetter;
import org.labkey.api.data.TableSelector;
import org.labkey.api.security.User;
import org.labkey.api.study.QueryHelper.StudyCacheCollections;

import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.stream.Stream;

public class QueryHelper<K, T extends StudyCachable<K, T>, SC extends QueryHelper.StudyCacheCollections<K, T>>
public class QueryHelper<K, T extends StudyCachable<K, T>, SC extends StudyCacheCollections<K, T>>
{
private final BlockingCache<Container, SC> _cache;
private final Class<T> _objectClass;
Expand All @@ -52,12 +54,18 @@ public QueryHelper(TableInfoGetter tableInfoGetter, Class<T> objectClass, @Nulla
_objectClass = objectClass;
_defaultSortString = defaultSortString;
TableInfo tableInfo = _tableInfoGetter.getTableInfo();
_cache = DatabaseCache.get(tableInfo.getSchema().getScope(), tableInfo.getCacheSize(), "StudyCache: " + tableInfo.getName(), (key, argument) ->
createCollections(
getTableSelector(key).stream(_objectClass)
_cache = DatabaseCache.get(tableInfo.getSchema().getScope(), tableInfo.getCacheSize(), "StudyCache: " + tableInfo.getName(), (key, _) ->
{
final Map<K, T> map;
try (Stream<T> stream = getTableSelector(key).uncachedStream(_objectClass))
{
map = Collections.unmodifiableMap(stream
.peek(StudyCachable::lock)
.toList()
));
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v))
);
}
Comment on lines +60 to +66

@labkey-jeckels labkey-jeckels Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Marginal benefit from closing the stream/ResultSet prior to creating the collections.

Suggested change
try (Stream<T> stream = getTableSelector(key).uncachedStream(_objectClass))
{
return createCollections(Collections.unmodifiableMap(stream
.peek(StudyCachable::lock)
.toList()
));
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v))
));
}
Map<K, T> map;
try (Stream<T> stream = getTableSelector(key).uncachedStream(_objectClass))
{
map = stream.peek(StudyCachable::lock)
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v))
));
}
return createCollections(Collections.unmodifiableMap(map));

@labkey-adam labkey-adam Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's exactly what I intended, but not what my fingers typed. Should be fixed up now.

return createCollections(map);
});
}

/**
Expand All @@ -83,9 +91,10 @@ protected SC getCollections(Container c)
return _cache.get(c, null);
}

protected SC createCollections(Collection<T> collection)
// map is an unmodifiable, linked map of pk -> locked object
protected SC createCollections(Map<K, T> map)
{
return (SC) new QueryHelper.StudyCacheCollections<>(collection);
return (SC) new StudyCacheCollections<>(map);
}

public T create(User user, T obj)
Expand Down Expand Up @@ -133,11 +142,10 @@ public static class StudyCacheCollections<K, V extends StudyCachable<K, V>>
{
private final Map<K, V> _map;

// Receives a collection of locked T objects
public StudyCacheCollections(Collection<V> collection)
// map should be an unmodifiable, linked map of pk -> locked objects
protected StudyCacheCollections(Map<K, V> map)
{
_map = Collections.unmodifiableMap(collection.stream()
.collect(LabKeyCollectors.toLinkedMap(StudyCachable::getPrimaryKey, v -> v)));
_map = map;
}

public @Nullable V get(K key)
Expand Down
20 changes: 12 additions & 8 deletions study/src/org/labkey/study/model/StudyManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,19 +354,21 @@ private Collection<VisitImpl> getCollection(Container c, Order order)
}

@Override
protected VisitCollections createCollections(Collection<VisitImpl> collection)
protected VisitCollections createCollections(Map<Integer, VisitImpl> map)
{
return new VisitCollections(collection);
return new VisitCollections(map);
}

private static class VisitCollections extends StudyCacheCollections<Integer, VisitImpl>
{
private final Collection<VisitImpl> _sequenceNumVisits;
private final Collection<VisitImpl> _chronologicalVisits;

private VisitCollections(Collection<VisitImpl> collection)
private VisitCollections(Map<Integer, VisitImpl> map)
{
super(collection);
super(map);

Collection<VisitImpl> collection = getCollection();

// I'd prefer to push comparators into Visit.Order, but Visit (in API) doesn't know about the display
// order field.
Expand Down Expand Up @@ -427,9 +429,9 @@ public void clearCache(Container c)
}

@Override
protected DatasetCollections createCollections(Collection<DatasetDefinition> collection)
protected DatasetCollections createCollections(Map<Integer, DatasetDefinition> map)
{
return new DatasetCollections(collection);
return new DatasetCollections(map);
}

protected DatasetCollections getCollections(Study study)
Expand All @@ -447,9 +449,11 @@ private static class DatasetCollections extends StudyCacheCollections<Integer, D
private final Map<Integer, List<DatasetDefinition>> _cohortMap;
private final List<DatasetDefinition> _nullCohortDatasets;

private DatasetCollections(Collection<DatasetDefinition> collection)
private DatasetCollections(Map<Integer, DatasetDefinition> map)
{
super(collection);
super(map);

Collection<DatasetDefinition> collection = getCollection();

// study.Dataset has constraints on LOWER(Name) and LOWER(Label), so this code path should never attempt
// to put duplicates into these maps. Use asserts to verify this.
Expand Down
Loading