Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a race condition where a dataset draft could be indexed before its `datasetVersionId` was persisted to the database, which occasionally caused missing version information in the Solr index and 500 errors in some API endpoints.
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,16 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException {
prepareDatasetAndVersion();

DatasetVersion version = ctxt.datasets().storeVersion(newVersion);
ctxt.em().flush();
return version;
}

@Override
public boolean onSuccess(CommandContext ctxt, Object r) {
if (ctxt.index() != null) {
ctxt.index().asyncIndexDataset(dataset, true);
}
return version;
return true;
}

/**
Expand Down
165 changes: 86 additions & 79 deletions src/main/java/edu/harvard/iq/dataverse/search/SolrSearchResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -608,102 +608,109 @@ public JsonObjectBuilder json(boolean showRelevance, boolean showEntityIds, bool
if (this.entity.isInstanceofDataset()) {
nullSafeJsonBuilder.add("storageIdentifier", this.entity.getStorageIdentifier());
Dataset ds = (Dataset) this.entity;
DatasetVersion dv = ds.getVersionFromId(this.datasetVersionId);
DatasetVersion dv = null;
if (this.datasetVersionId > 0) {
dv = ds.getVersionFromId(this.datasetVersionId);
}

if (!dv.getKeywords().isEmpty()) {
JsonArrayBuilder keyWords = Json.createArrayBuilder();
for (String keyword : dv.getKeywords()) {
keyWords.add(keyword);
if (dv == null) {
logger.warning("DatasetVersion not found for dataset id " + ds.getId() + " and version id " + this.datasetVersionId + ". This may be due to a race condition between asynchronous indexing and the transaction commit. To fix this, you can call the re-index API: http://localhost:8080/api/admin/index/dataset?persistentId=" + ds.getGlobalId().toString());
} else {
if (!dv.getKeywords().isEmpty()) {
JsonArrayBuilder keyWords = Json.createArrayBuilder();
for (String keyword : dv.getKeywords()) {
keyWords.add(keyword);
}
nullSafeJsonBuilder.add("keywords", keyWords);
}
nullSafeJsonBuilder.add("keywords", keyWords);
}

JsonArrayBuilder subjects = Json.createArrayBuilder();
for (String subject : dv.getDatasetSubjects()) {
subjects.add(subject);
}
nullSafeJsonBuilder.add("subjects", subjects);
nullSafeJsonBuilder.add("fileCount", this.fileCount);
nullSafeJsonBuilder.add("versionId", dv.getId());
nullSafeJsonBuilder.add("versionState", dv.getVersionState().toString());
if (this.isPublishedState()) {
nullSafeJsonBuilder.add("majorVersion", dv.getVersionNumber());
nullSafeJsonBuilder.add("minorVersion", dv.getMinorVersionNumber());
}
JsonArrayBuilder subjects = Json.createArrayBuilder();
for (String subject : dv.getDatasetSubjects()) {
subjects.add(subject);
}
nullSafeJsonBuilder.add("subjects", subjects);
nullSafeJsonBuilder.add("fileCount", this.fileCount);
nullSafeJsonBuilder.add("versionId", dv.getId());
nullSafeJsonBuilder.add("versionState", dv.getVersionState().toString());
if (this.isPublishedState()) {
nullSafeJsonBuilder.add("majorVersion", dv.getVersionNumber());
nullSafeJsonBuilder.add("minorVersion", dv.getMinorVersionNumber());
}

nullSafeJsonBuilder.add("createdAt", ds.getCreateDate());
nullSafeJsonBuilder.add("updatedAt", ds.getModificationTime());
nullSafeJsonBuilder.add("createdAt", ds.getCreateDate());
nullSafeJsonBuilder.add("updatedAt", ds.getModificationTime());

if (!dv.getDatasetContacts().isEmpty()) {
JsonArrayBuilder contacts = Json.createArrayBuilder();
NullSafeJsonBuilder nullSafeJsonBuilderInner = jsonObjectBuilder();
for (String contact[] : dv.getDatasetContacts(false)) {
nullSafeJsonBuilderInner.add("name", contact[0]);
nullSafeJsonBuilderInner.add("affiliation", contact[1]);
contacts.add(nullSafeJsonBuilderInner);
if (!dv.getDatasetContacts().isEmpty()) {
JsonArrayBuilder contacts = Json.createArrayBuilder();
NullSafeJsonBuilder nullSafeJsonBuilderInner = jsonObjectBuilder();
for (String contact[] : dv.getDatasetContacts(false)) {
nullSafeJsonBuilderInner.add("name", contact[0]);
nullSafeJsonBuilderInner.add("affiliation", contact[1]);
contacts.add(nullSafeJsonBuilderInner);
}
nullSafeJsonBuilder.add("contacts", contacts);
}
nullSafeJsonBuilder.add("contacts", contacts);
}
if (!dv.getRelatedPublications().isEmpty()) {
JsonArrayBuilder relPub = Json.createArrayBuilder();
NullSafeJsonBuilder inner = jsonObjectBuilder();
for (DatasetRelPublication dsRelPub : dv.getRelatedPublications()) {
inner.add("title", dsRelPub.getTitle());
inner.add("citation", dsRelPub.getText());
inner.add("url", dsRelPub.getUrl());
relPub.add(inner);
if (!dv.getRelatedPublications().isEmpty()) {
JsonArrayBuilder relPub = Json.createArrayBuilder();
NullSafeJsonBuilder inner = jsonObjectBuilder();
for (DatasetRelPublication dsRelPub : dv.getRelatedPublications()) {
inner.add("title", dsRelPub.getTitle());
inner.add("citation", dsRelPub.getText());
inner.add("url", dsRelPub.getUrl());
relPub.add(inner);
}
nullSafeJsonBuilder.add("publications", relPub);
}
nullSafeJsonBuilder.add("publications", relPub);
}

if (!dv.getDatasetProducers().isEmpty()) {
JsonArrayBuilder producers = Json.createArrayBuilder();
for (String[] producer : dv.getDatasetProducers()) {
producers.add(producer[0]);
if (!dv.getDatasetProducers().isEmpty()) {
JsonArrayBuilder producers = Json.createArrayBuilder();
for (String[] producer : dv.getDatasetProducers()) {
producers.add(producer[0]);
}
nullSafeJsonBuilder.add("producers", producers);
}
nullSafeJsonBuilder.add("producers", producers);
}
if (!dv.getRelatedMaterial().isEmpty()) {
JsonArrayBuilder relatedMaterials = Json.createArrayBuilder();
for (String relatedMaterial : dv.getRelatedMaterial()) {
relatedMaterials.add(relatedMaterial);
if (!dv.getRelatedMaterial().isEmpty()) {
JsonArrayBuilder relatedMaterials = Json.createArrayBuilder();
for (String relatedMaterial : dv.getRelatedMaterial()) {
relatedMaterials.add(relatedMaterial);
}
nullSafeJsonBuilder.add("relatedMaterial", relatedMaterials);
}
nullSafeJsonBuilder.add("relatedMaterial", relatedMaterials);
}

if (!dv.getGeographicCoverage().isEmpty()) {
JsonArrayBuilder geoCov = Json.createArrayBuilder();
NullSafeJsonBuilder inner = jsonObjectBuilder();
for (String ind[] : dv.getGeographicCoverage()) {
inner.add("country", ind[0]);
inner.add("state", ind[1]);
inner.add("city", ind[2]);
inner.add("other", ind[3]);
geoCov.add(inner);
if (!dv.getGeographicCoverage().isEmpty()) {
JsonArrayBuilder geoCov = Json.createArrayBuilder();
NullSafeJsonBuilder inner = jsonObjectBuilder();
for (String ind[] : dv.getGeographicCoverage()) {
inner.add("country", ind[0]);
inner.add("state", ind[1]);
inner.add("city", ind[2]);
inner.add("other", ind[3]);
geoCov.add(inner);
}
nullSafeJsonBuilder.add("geographicCoverage", geoCov);
}
nullSafeJsonBuilder.add("geographicCoverage", geoCov);
}
if (!dv.getDataSource().isEmpty()) {
JsonArrayBuilder dataSources = Json.createArrayBuilder();
for (String dsource : dv.getDataSource()) {
dataSources.add(dsource);
if (!dv.getDataSource().isEmpty()) {
JsonArrayBuilder dataSources = Json.createArrayBuilder();
for (String dsource : dv.getDataSource()) {
dataSources.add(dsource);
}
nullSafeJsonBuilder.add("dataSources", dataSources);
}
nullSafeJsonBuilder.add("dataSources", dataSources);
}

if (CollectionUtils.isNotEmpty(metadataFields)) {
// create metadata fields map names
Map<String, List<String>> metadataFieldMapNames = computeRequestedMetadataFieldMapNames(
metadataFields);
if (CollectionUtils.isNotEmpty(metadataFields)) {
// create metadata fields map names
Map<String, List<String>> metadataFieldMapNames = computeRequestedMetadataFieldMapNames(
metadataFields);

// add metadatafields objet to wrap all requeested fields
NullSafeJsonBuilder metadataFieldBuilder = jsonObjectBuilder();
// add metadatafields objet to wrap all requeested fields
NullSafeJsonBuilder metadataFieldBuilder = jsonObjectBuilder();

Map<MetadataBlock, List<DatasetField>> groupedFields = DatasetField
.groupByBlock(dv.getFlatDatasetFields());
json(metadataFieldMapNames, groupedFields, metadataFieldBuilder);
Map<MetadataBlock, List<DatasetField>> groupedFields = DatasetField
.groupByBlock(dv.getFlatDatasetFields());
json(metadataFieldMapNames, groupedFields, metadataFieldBuilder);

nullSafeJsonBuilder.add("metadataBlocks", metadataFieldBuilder);
nullSafeJsonBuilder.add("metadataBlocks", metadataFieldBuilder);
}
}

if (this.collections != null && !this.collections.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public class SolrSearchServiceBean implements SearchService {
PermissionServiceBean permissionService;
@Inject
ThumbnailServiceWrapper thumbnailServiceWrapper;


@Override
public String getServiceName() {
Expand Down Expand Up @@ -154,7 +154,7 @@ public SolrQueryResponse search(
solrQuery.setParam("fl", "*,score");
solrQuery.setParam("qt", "/select");
solrQuery.setParam("facet", "true");

/**
* @todo: do we need facet.query?
*/
Expand Down Expand Up @@ -463,11 +463,11 @@ public SolrQueryResponse search(
Boolean datasetValid = (Boolean) solrDocument.getFieldValue(SearchFields.DATASET_VALID);
Long fileCount = (Long) solrDocument.getFieldValue(SearchFields.FILE_COUNT);
Long datasetCount = (Long) solrDocument.getFieldValue(SearchFields.DATASET_COUNT);

List<String> matchedFields = new ArrayList<>();

SolrSearchResult solrSearchResult = new SolrSearchResult(query, name);

if (addHighlights) {
List<Highlight> highlights = new ArrayList<>();
Map<SolrField, Highlight> highlightsMap = new HashMap<>();
Expand Down Expand Up @@ -501,8 +501,8 @@ public SolrQueryResponse search(
solrSearchResult.setHighlightsMap(highlightsMap);
solrSearchResult.setHighlightsAsMap(highlightsMap3);
}


/**
* @todo put all this in the constructor?
*/
Expand All @@ -529,7 +529,7 @@ public SolrQueryResponse search(
solrSearchResult.setNameSort(nameSort);
solrSearchResult.setReleaseOrCreateDate(release_or_create_date);
solrSearchResult.setMatchedFields(matchedFields);

Map<String, String> parent = new HashMap<>();
String description = (String) solrDocument.getFieldValue(SearchFields.DESCRIPTION);
solrSearchResult.setDescriptionNoSnippet(description);
Expand Down Expand Up @@ -585,8 +585,12 @@ public SolrQueryResponse search(
solrSearchResult.setDescriptionNoSnippet(String.join(" ", datasetDescriptions));
}
}
solrSearchResult.setDatasetVersionId(datasetVersionId);

if (datasetVersionId != null) {
solrSearchResult.setDatasetVersionId(datasetVersionId);
} else {
logger.warning("DatasetVersion id is missing for dataset id " + entityid + ". This may be due to a race condition between asynchronous indexing and the transaction commit. To fix this, you can call the re-index API: http://localhost:8080/api/admin/index/dataset?persistentId=" + identifier);
}
solrSearchResult.setCitation(citation);
solrSearchResult.setCitationHtml(citationPlainHtml);

Expand Down Expand Up @@ -666,7 +670,13 @@ public SolrQueryResponse search(
}

solrSearchResult.setUnf((String) solrDocument.getFieldValue(SearchFields.UNF));
solrSearchResult.setDatasetVersionId(datasetVersionId);

if (datasetVersionId > 0) {
solrSearchResult.setDatasetVersionId(datasetVersionId);
} else {
logger.warning("DatasetVersion id is missing for file id " + entityid + ". This may be due to a race condition between asynchronous indexing and the transaction commit. To fix this, you can call the re-index API: http://localhost:8080/api/admin/index/dataset?persistentId=" + parentGlobalId);
}

List<String> fileCategories = (List) solrDocument.getFieldValues(SearchFields.FILE_TAG);
if (fileCategories != null) {
solrSearchResult.setFileCategories(fileCategories);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import edu.harvard.iq.dataverse.authorization.Permission;
import edu.harvard.iq.dataverse.mocks.MocksFactory;
import static edu.harvard.iq.dataverse.mocks.MocksFactory.*;
import edu.harvard.iq.dataverse.engine.NoOpTestEntityManager;
import edu.harvard.iq.dataverse.engine.TestCommandContext;
import edu.harvard.iq.dataverse.engine.TestDataverseEngine;
import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException;
Expand Down Expand Up @@ -54,6 +55,9 @@ public void testSimpleVersionAddition() throws Exception {
final MockDatasetServiceBean serviceBean = new MockDatasetServiceBean();
TestDataverseEngine testEngine = new TestDataverseEngine( new TestCommandContext(){
@Override public DatasetServiceBean datasets() { return serviceBean; }
@Override public jakarta.persistence.EntityManager em() {
return new NoOpTestEntityManager();
}
} );

testEngine.submit(sut);
Expand Down Expand Up @@ -86,7 +90,10 @@ void testCantCreateTwoDraftVersions() {
public DatasetServiceBean datasets() {
return dsb;
}

@Override
public jakarta.persistence.EntityManager em() {
return new NoOpTestEntityManager();
}
});

assertThrows(IllegalCommandException.class, () -> testEngine.submit(sut));
Expand Down
Loading