Skip to content

Commit

Permalink
Wow, side effects causing bugs!
Browse files Browse the repository at this point in the history
  • Loading branch information
Luke Sikina committed Dec 28, 2023
1 parent 6bf6c60 commit 572f66c
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
import javax.ws.rs.core.Response.Status;

import edu.harvard.hms.dbmi.avillach.hpds.service.filesharing.FileSharingService;
import edu.harvard.hms.dbmi.avillach.hpds.service.filesharing.FileSystemService;
import edu.harvard.hms.dbmi.avillach.hpds.service.util.Paginator;
import edu.harvard.hms.dbmi.avillach.hpds.service.util.QueryUUIDGen;
import edu.harvard.hms.dbmi.avillach.hpds.service.util.QueryDecorator;
import org.apache.http.entity.ContentType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -30,7 +29,6 @@

import edu.harvard.dbmi.avillach.domain.*;
import edu.harvard.dbmi.avillach.service.IResourceRS;
import edu.harvard.dbmi.avillach.util.UUIDv5;
import edu.harvard.hms.dbmi.avillach.hpds.crypto.Crypto;
import edu.harvard.hms.dbmi.avillach.hpds.data.genotype.FileBackedByteIndexedInfoStore;
import edu.harvard.hms.dbmi.avillach.hpds.data.phenotype.ColumnMeta;
Expand All @@ -47,7 +45,7 @@ public class PicSureService implements IResourceRS {
@Autowired
public PicSureService(QueryService queryService, TimelineProcessor timelineProcessor, CountProcessor countProcessor,
VariantListProcessor variantListProcessor, AbstractProcessor abstractProcessor,
Paginator paginator, FileSharingService fileSystemService, QueryUUIDGen queryUUIDGen
Paginator paginator, FileSharingService fileSystemService, QueryDecorator queryDecorator
) {
this.queryService = queryService;
this.timelineProcessor = timelineProcessor;
Expand All @@ -56,7 +54,7 @@ public PicSureService(QueryService queryService, TimelineProcessor timelineProce
this.abstractProcessor = abstractProcessor;
this.paginator = paginator;
this.fileSystemService = fileSystemService;
this.queryUUIDGen = queryUUIDGen;
this.queryDecorator = queryDecorator;
Crypto.loadDefaultKey();
}

Expand All @@ -78,7 +76,7 @@ public PicSureService(QueryService queryService, TimelineProcessor timelineProce

private final FileSharingService fileSystemService;

private final QueryUUIDGen queryUUIDGen;
private final QueryDecorator queryDecorator;

private static final String QUERY_METADATA_FIELD = "queryMetadata";
private static final int RESPONSE_CACHE_SIZE = 50;
Expand Down Expand Up @@ -228,7 +226,7 @@ private QueryStatus convertToQueryStatus(AsyncResult entity) {
status.setStatus(entity.status.toPicSureStatus());

Map<String, Object> metadata = new HashMap<String, Object>();
queryUUIDGen.setId(entity.query);
queryDecorator.setId(entity.query);
metadata.put("picsureQueryId", entity.query.getId());
status.setResultMetadata(metadata);
return status;
Expand Down Expand Up @@ -271,7 +269,7 @@ public Response writeQueryResult(
// query IDs within HPDS are a different concept that query IDs in PIC-SURE
// Generally, equivalent queries with different PIC-SURE query IDs will have the SAME
// HPDS query ID.
queryUUIDGen.setId(query);
queryDecorator.setId(query);
AsyncResult result = queryService.getResultFor(query.getId());
// the queryResult has this DIY retry logic that blocks a system thread.
// I'm not going to do that here. If the service can't find it, you get a 404.
Expand Down Expand Up @@ -433,7 +431,7 @@ private Response _querySync(QueryRequest resultRequest) throws IOException {
}

private ResponseBuilder queryOkResponse(Object obj, Query incomingQuery) {
queryUUIDGen.setId(incomingQuery);
queryDecorator.setId(incomingQuery);
return Response.ok(obj).header(QUERY_METADATA_FIELD, incomingQuery.getId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,10 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;

import edu.harvard.hms.dbmi.avillach.hpds.service.util.QueryUUIDGen;
import edu.harvard.hms.dbmi.avillach.hpds.service.util.QueryDecorator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.ImmutableMap;

import edu.harvard.dbmi.avillach.util.UUIDv5;
import edu.harvard.hms.dbmi.avillach.hpds.data.query.Query;
import edu.harvard.hms.dbmi.avillach.hpds.processing.*;
import edu.harvard.hms.dbmi.avillach.hpds.processing.AsyncResult.Status;
Expand Down Expand Up @@ -42,21 +39,21 @@ public class QueryService {
private final QueryProcessor queryProcessor;
private final TimeseriesProcessor timeseriesProcessor;
private final CountProcessor countProcessor;
private final QueryUUIDGen queryUUIDGen;
private final QueryDecorator queryDecorator;

HashMap<String, AsyncResult> results = new HashMap<>();


@Autowired
public QueryService (
AbstractProcessor abstractProcessor, QueryProcessor queryProcessor, TimeseriesProcessor timeseriesProcessor,
CountProcessor countProcessor, QueryUUIDGen queryUUIDGen
CountProcessor countProcessor, QueryDecorator queryDecorator
) {
this.abstractProcessor = abstractProcessor;
this.queryProcessor = queryProcessor;
this.timeseriesProcessor = timeseriesProcessor;
this.countProcessor = countProcessor;
this.queryUUIDGen = queryUUIDGen;
this.queryDecorator = queryDecorator;

SMALL_JOB_LIMIT = getIntProp("SMALL_JOB_LIMIT");
SMALL_TASK_THREADS = getIntProp("SMALL_TASK_THREADS");
Expand All @@ -75,7 +72,6 @@ public QueryService (

public AsyncResult runQuery(Query query) throws ClassNotFoundException, IOException {
// Merging fields from filters into selected fields for user validation of results
mergeFilterFieldsIntoSelectedFields(query);

Collections.sort(query.getFields());

Expand Down Expand Up @@ -126,38 +122,13 @@ private AsyncResult initializeResult(Query query) throws ClassNotFoundException,
AsyncResult result = new AsyncResult(query, p.getHeaderRow(query));
result.status = AsyncResult.Status.PENDING;
result.queuedTime = System.currentTimeMillis();
queryUUIDGen.setId(query);
queryDecorator.setId(query);
result.id = query.getId();
result.processor = p;
queryUUIDGen.setId(query);
queryDecorator.setId(query);
results.put(result.id, result);
return result;
}


private void mergeFilterFieldsIntoSelectedFields(Query query) {
LinkedHashSet<String> fields = new LinkedHashSet<>();
fields.addAll(query.getFields());
if(!query.getCategoryFilters().isEmpty()) {
Set<String> categoryFilters = new TreeSet<String>(query.getCategoryFilters().keySet());
Set<String> toBeRemoved = new TreeSet<String>();
for(String categoryFilter : categoryFilters) {
System.out.println("In : " + categoryFilter);
if(VariantUtils.pathIsVariantSpec(categoryFilter)) {
toBeRemoved.add(categoryFilter);
}
}
categoryFilters.removeAll(toBeRemoved);
for(String categoryFilter : categoryFilters) {
System.out.println("Out : " + categoryFilter);
}
fields.addAll(categoryFilters);
}
fields.addAll(query.getAnyRecordOf());
fields.addAll(query.getRequiredFields());
fields.addAll(query.getNumericFilters().keySet());
query.setFields(fields);
}

private boolean ensureAllFieldsExist(Query query) {
TreeSet<String> allFields = new TreeSet<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package edu.harvard.hms.dbmi.avillach.hpds.service.util;

import edu.harvard.dbmi.avillach.util.UUIDv5;
import edu.harvard.hms.dbmi.avillach.hpds.data.query.Query;
import edu.harvard.hms.dbmi.avillach.hpds.processing.VariantUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Component;

import java.util.LinkedHashSet;
import java.util.Set;
import java.util.TreeSet;

@Component
public class QueryDecorator {
private static final Logger LOG = LoggerFactory.getLogger(QueryDecorator.class);

public void setId(Query query) {
query.setId(""); // the id is included in the toString
// I clear it here to keep the ID setting stable for any query
// of identical structure and content

// Some places where we call toString, we call mergeFilterFieldsIntoSelectedFields
// first. This can mutate the query, resulting in shifting UUIDs
// To stabilize things, we're always going to call that, and shift the logic here
mergeFilterFieldsIntoSelectedFields(query);

String id = UUIDv5.UUIDFromString(query.toString()).toString();
query.setId(id);
}

public void mergeFilterFieldsIntoSelectedFields(Query query) {
LinkedHashSet<String> fields = new LinkedHashSet<>(query.getFields());

if(!query.getCategoryFilters().isEmpty()) {
Set<String> categoryFilters = new TreeSet<>(query.getCategoryFilters().keySet());
Set<String> toBeRemoved = new TreeSet<>();
for(String categoryFilter : categoryFilters) {
LOG.debug("In : {}", categoryFilter);
if(VariantUtils.pathIsVariantSpec(categoryFilter)) {
toBeRemoved.add(categoryFilter);
}
}
categoryFilters.removeAll(toBeRemoved);
for(String categoryFilter : categoryFilters) {
LOG.debug("Out : {}", categoryFilter);
}
fields.addAll(categoryFilters);
}
fields.addAll(query.getAnyRecordOf());
fields.addAll(query.getRequiredFields());
fields.addAll(query.getNumericFilters().keySet());
query.setFields(fields);
}
}

This file was deleted.

0 comments on commit 572f66c

Please sign in to comment.