Skip to content

Commit 0d7f730

Browse files
qqmyersqqmyers
committed
simplify2
1 parent c55a4a5 commit 0d7f730

3 files changed

Lines changed: 189 additions & 102 deletions

File tree

‎src/main/java/edu/harvard/iq/dataverse/search/SearchPermissionsServiceBean.java‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -94,53 +94,6 @@ public List<String> findDvObjectPerms(DvObject dvObject) {
9494
return permStrings;
9595
}
9696

97-
98-
public Map<DatasetVersion.VersionState, Boolean> getDesiredCards(Dataset dataset) {
99-
Map<DatasetVersion.VersionState, Boolean> desiredCards = new LinkedHashMap<>();
100-
DatasetVersion latestVersion = dataset.getLatestVersion();
101-
DatasetVersion.VersionState latestVersionState = latestVersion.getVersionState();
102-
DatasetVersion releasedVersion = dataset.getReleasedVersion();
103-
boolean atLeastOnePublishedVersion = false;
104-
if (releasedVersion != null) {
105-
atLeastOnePublishedVersion = true;
106-
} else {
107-
atLeastOnePublishedVersion = false;
108-
}
109-
110-
if (atLeastOnePublishedVersion == false) {
111-
if (latestVersionState.equals(DatasetVersion.VersionState.DRAFT)) {
112-
desiredCards.put(DatasetVersion.VersionState.DRAFT, true);
113-
desiredCards.put(DatasetVersion.VersionState.DEACCESSIONED, false);
114-
desiredCards.put(DatasetVersion.VersionState.RELEASED, false);
115-
} else if (latestVersionState.equals(DatasetVersion.VersionState.DEACCESSIONED)) {
116-
desiredCards.put(DatasetVersion.VersionState.DEACCESSIONED, true);
117-
desiredCards.put(DatasetVersion.VersionState.RELEASED, false);
118-
desiredCards.put(DatasetVersion.VersionState.DRAFT, false);
119-
} else {
120-
String msg = "No-op. Unexpected condition reached: There is no published version and the latest published version is neither " + DatasetVersion.VersionState.DRAFT + " nor " + DatasetVersion.VersionState.DEACCESSIONED + ". Its state is " + latestVersionState + ".";
121-
logger.info(msg);
122-
}
123-
} else if (atLeastOnePublishedVersion == true) {
124-
if (latestVersionState.equals(DatasetVersion.VersionState.RELEASED)
125-
|| latestVersionState.equals(DatasetVersion.VersionState.DEACCESSIONED)) {
126-
desiredCards.put(DatasetVersion.VersionState.RELEASED, true);
127-
desiredCards.put(DatasetVersion.VersionState.DRAFT, false);
128-
desiredCards.put(DatasetVersion.VersionState.DEACCESSIONED, false);
129-
} else if (latestVersionState.equals(DatasetVersion.VersionState.DRAFT)) {
130-
desiredCards.put(DatasetVersion.VersionState.DRAFT, true);
131-
desiredCards.put(DatasetVersion.VersionState.RELEASED, true);
132-
desiredCards.put(DatasetVersion.VersionState.DEACCESSIONED, false);
133-
} else {
134-
String msg = "No-op. Unexpected condition reached: There is at least one published version but the latest version is neither published nor draft";
135-
logger.info(msg);
136-
}
137-
} else {
138-
String msg = "No-op. Unexpected condition reached: Has a version been published or not?";
139-
logger.info(msg);
140-
}
141-
return desiredCards;
142-
}
143-
14497
private boolean hasBeenPublished(Dataverse dataverse) {
14598
return dataverse.isReleased();
14699
}

‎src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java‎

Lines changed: 49 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,10 @@ public List<DvObjectSolrDoc> determineSolrDocs(DvObject dvObject) {
9797
solrDocs.addAll(datasetSolrDocs);
9898
} else if (dvObject.isInstanceofDataFile()) {
9999
DataFile datafile = (DataFile) dvObject;
100-
Map<DatasetVersion.VersionState, Boolean> desiredCards = searchPermissionsService.getDesiredCards(datafile.getOwner());
101100
Set<DatasetVersion> datasetVersions = datasetVersionsToBuildCardsFor(datafile.getOwner());
102101
List<String> downloaders = searchPermissionsService.findDvObjectPerms(datafile);
103102
for (DatasetVersion version : datasetVersions) {
104-
if(desiredCards.containsKey(version.getVersionState()) && desiredCards.get(version.getVersionState()) && datafile.isInDatasetVersion(version)) {
103+
if(datafile.isInDatasetVersion(version)) {
105104
List<String> cachedPerms = searchPermissionsService.findDatasetVersionPerms(version);
106105
String solrIdEnd = getDatasetOrDataFileSolrEnding(version.getVersionState());
107106
Long versionId = version.getId();
@@ -148,13 +147,9 @@ private DvObjectSolrDoc constructDataverseSolrDoc(Dataverse dataverse) {
148147
private List<DvObjectSolrDoc> constructDatasetSolrDocs(Dataset dataset) {
149148
List<DvObjectSolrDoc> emptyList = new ArrayList<>();
150149
List<DvObjectSolrDoc> solrDocs = emptyList;
151-
Map<DatasetVersion.VersionState, Boolean> desiredCards = searchPermissionsService.getDesiredCards(dataset);
152150
for (DatasetVersion version : datasetVersionsToBuildCardsFor(dataset)) {
153-
boolean cardShouldExist = desiredCards.get(version.getVersionState());
154-
if (cardShouldExist) {
155-
DvObjectSolrDoc datasetSolrDoc = makeDatasetSolrDoc(version);
156-
solrDocs.add(datasetSolrDoc);
157-
}
151+
DvObjectSolrDoc datasetSolrDoc = makeDatasetSolrDoc(version);
152+
solrDocs.add(datasetSolrDoc);
158153
}
159154
return solrDocs;
160155
}
@@ -173,43 +168,49 @@ private DvObjectSolrDoc constructDatafileSolrDoc(DataFileProxy fileProxy, List<S
173168

174169
private List<DvObjectSolrDoc> constructDatafileSolrDocsFromDataset(Dataset dataset) {
175170
List<DvObjectSolrDoc> datafileSolrDocs = new ArrayList<>();
176-
Map<DatasetVersion.VersionState, Boolean> desiredCards = searchPermissionsService.getDesiredCards(dataset);
177171
for (DatasetVersion datasetVersionFileIsAttachedTo : datasetVersionsToBuildCardsFor(dataset)) {
178-
boolean cardShouldExist = desiredCards.get(datasetVersionFileIsAttachedTo.getVersionState());
179-
if (cardShouldExist) {
180-
List<String> perms = new ArrayList<>();
181-
if (datasetVersionFileIsAttachedTo.isReleased()) {
182-
perms.add(IndexServiceBean.getPublicGroupString());
183-
} else {
184-
perms = searchPermissionsService.findDatasetVersionPerms(datasetVersionFileIsAttachedTo);
185-
}
172+
List<String> perms = new ArrayList<>();
173+
if (datasetVersionFileIsAttachedTo.isReleased()) {
174+
perms.add(IndexServiceBean.getPublicGroupString());
175+
} else {
176+
perms = searchPermissionsService.findDatasetVersionPerms(datasetVersionFileIsAttachedTo);
177+
}
186178

187-
for (FileMetadata fileMetadata : datasetVersionFileIsAttachedTo.getFileMetadatas()) {
188-
Long fileId = fileMetadata.getDataFile().getId();
189-
String solrIdStart = IndexServiceBean.solrDocIdentifierFile + fileId;
190-
String solrIdEnd = getDatasetOrDataFileSolrEnding(datasetVersionFileIsAttachedTo.getVersionState());
191-
String solrId = solrIdStart + solrIdEnd;
192-
List<String> ftperms = new ArrayList<>();
193-
if (fileMetadata.getDataFile().isRestricted()) {
194-
ftperms = searchPermissionsService.findDvObjectPerms(fileMetadata.getDataFile());
195-
}
196-
DvObjectSolrDoc dataFileSolrDoc = new DvObjectSolrDoc(fileId.toString(), solrId, datasetVersionFileIsAttachedTo.getId(), fileMetadata.getLabel(), perms, ftperms);
197-
logger.finest("adding fileid " + fileId);
198-
datafileSolrDocs.add(dataFileSolrDoc);
179+
for (FileMetadata fileMetadata : datasetVersionFileIsAttachedTo.getFileMetadatas()) {
180+
Long fileId = fileMetadata.getDataFile().getId();
181+
String solrIdStart = IndexServiceBean.solrDocIdentifierFile + fileId;
182+
String solrIdEnd = getDatasetOrDataFileSolrEnding(datasetVersionFileIsAttachedTo.getVersionState());
183+
String solrId = solrIdStart + solrIdEnd;
184+
List<String> ftperms = new ArrayList<>();
185+
if (fileMetadata.getDataFile().isRestricted()) {
186+
ftperms = searchPermissionsService.findDvObjectPerms(fileMetadata.getDataFile());
199187
}
188+
DvObjectSolrDoc dataFileSolrDoc = new DvObjectSolrDoc(fileId.toString(), solrId, datasetVersionFileIsAttachedTo.getId(), fileMetadata.getLabel(), perms, ftperms);
189+
logger.finest("adding fileid " + fileId);
190+
datafileSolrDocs.add(dataFileSolrDoc);
200191
}
201192
}
202193
return datafileSolrDocs;
203194
}
204195

196+
/** Find the versions to index. The overall logic is
197+
* If there is only one version, or no released version (all non-draft versions are deaccessioned)
198+
* then index it regardless of it's versionstate
199+
* If there are released versions
200+
* then index the latest released version and a draft version if one exists
201+
* Hence - the latest deaccessioned version is only indexed if there is no released version
202+
* @param dataset
203+
* @return the set of versions to build cards for
204+
*/
205205
private Set<DatasetVersion> datasetVersionsToBuildCardsFor(Dataset dataset) {
206206
Set<DatasetVersion> datasetVersions = new HashSet<>();
207207
DatasetVersion latest = dataset.getLatestVersion();
208-
if (latest != null) {
208+
DatasetVersion released = dataset.getReleasedVersion();
209+
if (latest != null && (released == null || latest.isDraft())) {
209210
datasetVersions.add(latest);
210211
}
211-
DatasetVersion released = dataset.getReleasedVersion();
212212
if (released != null) {
213+
//May be the same as the latest version - only one copy will be in the set in that case
213214
datasetVersions.add(released);
214215
}
215216
return datasetVersions;
@@ -435,17 +436,14 @@ public IndexResponse indexPermissionsOnSelfAndChildren(DvObject definitionPoint)
435436
*
436437
**/
437438
Map<Long, List<String>> fileDownloadersMap = roleAssigneeSvc.findAssigneesWithDownloadPermissionOnDatasetFiles(dataset.getId());
438-
Map<DatasetVersion.VersionState, Boolean> desiredCards = searchPermissionsService.getDesiredCards(dataset);
439439
List<DatasetVersion> versionsToIndex = new ArrayList<>();
440-
for (DatasetVersion version : versionsToReIndexPermissionsFor(dataset)) {
441-
if (desiredCards.get(version.getVersionState())) {
442-
int fileCount = dataFileService.findCountByDatasetVersionId(version.getId()).intValue();
443-
if (fileCount >= fileQueryMin) {
444-
// IMPORTANT: This triggers the loading of fileMetadatas within the current transaction
445-
version.getFileMetadatas().size();
446-
}
447-
versionsToIndex.add(version);
440+
for (DatasetVersion version : datasetVersionsToBuildCardsFor(dataset)) {
441+
int fileCount = dataFileService.findCountByDatasetVersionId(version.getId()).intValue();
442+
if (fileCount >= fileQueryMin) {
443+
// IMPORTANT: This triggers the loading of fileMetadatas within the current transaction
444+
version.getFileMetadatas().size();
448445
}
446+
versionsToIndex.add(version);
449447
}
450448

451449
// Process the dataset's files in a new transaction, passing the pre-loaded data
@@ -471,11 +469,19 @@ public void indexDatasetBatchInNewTransaction(List<Long> datasetIds, final int[]
471469

472470
// Process files for this dataset
473471
Map<Long, List<String>> fileDownloadersMap = roleAssigneeSvc.findAssigneesWithDownloadPermissionOnDatasetFiles(dataset.getId());
474-
List<DatasetVersion> versions = versionsToReIndexPermissionsFor(dataset);
472+
Set<DatasetVersion> versions = datasetVersionsToBuildCardsFor(dataset);
475473
final List<Long> changedFileIds = new ArrayList<>();
476474
if(versions.size()>1) {
477-
Long releasedVersionId = versions.get(versions.get(0).isReleased() ? 0 : 1).getId();
478-
Long draftVersionId = versions.get(versions.get(0).isReleased() ? 1 : 0).getId();
475+
Long releasedVersionId = null;
476+
Long draftVersionId = null;
477+
478+
for (DatasetVersion version : versions) {
479+
if (version.isReleased()) {
480+
releasedVersionId = version.getId();
481+
} else if (version.isDraft()) {
482+
draftVersionId = version.getId();
483+
}
484+
}
479485

480486
populateChangedFileIds(
481487
releasedVersionId,
@@ -632,18 +638,6 @@ private void reindexFilesInBatches(List<DataFileProxy> filesToReindexAsBatch, Li
632638
}
633639
}
634640

635-
private List<DatasetVersion> versionsToReIndexPermissionsFor(Dataset dataset) {
636-
List<DatasetVersion> versionsToReindexPermissionsFor = new ArrayList<>();
637-
Map<DatasetVersion.VersionState, Boolean> desiredCards = searchPermissionsService.getDesiredCards(dataset);
638-
for (DatasetVersion version : datasetVersionsToBuildCardsFor(dataset)) {
639-
boolean cardShouldExist = desiredCards.get(version.getVersionState());
640-
if (cardShouldExist) {
641-
versionsToReindexPermissionsFor.add(version);
642-
}
643-
}
644-
return versionsToReindexPermissionsFor;
645-
}
646-
647641
public IndexResponse deleteMultipleSolrIds(List<String> solrIdsToDelete) {
648642
if (solrIdsToDelete.isEmpty()) {
649643
return new IndexResponse("nothing to delete");

‎src/test/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBeanTest.java‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
2+
package edu.harvard.iq.dataverse.search;
3+
4+
import edu.harvard.iq.dataverse.Dataset;
5+
import edu.harvard.iq.dataverse.DatasetVersion;
6+
import org.junit.jupiter.api.BeforeEach;
7+
import org.junit.jupiter.api.Test;
8+
import org.mockito.InjectMocks;
9+
import org.mockito.MockitoAnnotations;
10+
11+
import java.util.Set;
12+
13+
import static org.junit.jupiter.api.Assertions.*;
14+
import static org.mockito.Mockito.*;
15+
16+
public class SolrIndexServiceBeanTest {
17+
18+
@InjectMocks
19+
private SolrIndexServiceBean solrIndexServiceBean;
20+
21+
@BeforeEach
22+
public void setUp() {
23+
MockitoAnnotations.openMocks(this);
24+
}
25+
26+
@Test
27+
public void testDatasetVersionsToBuildCardsFor_OnlyDraft() {
28+
// Arrange
29+
Dataset dataset = mock(Dataset.class);
30+
DatasetVersion draftVersion = createMockVersion(1L, DatasetVersion.VersionState.DRAFT, true);
31+
32+
when(dataset.getLatestVersion()).thenReturn(draftVersion);
33+
when(dataset.getReleasedVersion()).thenReturn(null);
34+
35+
// Act
36+
Set<DatasetVersion> result = invokeDatasetVersionsToBuildCardsFor(dataset);
37+
38+
// Assert
39+
assertEquals(1, result.size());
40+
assertTrue(result.contains(draftVersion));
41+
}
42+
43+
@Test
44+
public void testDatasetVersionsToBuildCardsFor_OnlyDeaccessioned() {
45+
// Arrange
46+
Dataset dataset = mock(Dataset.class);
47+
DatasetVersion deaccessionedVersion = createMockVersion(1L, DatasetVersion.VersionState.DEACCESSIONED, false);
48+
49+
when(dataset.getLatestVersion()).thenReturn(deaccessionedVersion);
50+
when(dataset.getReleasedVersion()).thenReturn(null);
51+
52+
// Act
53+
Set<DatasetVersion> result = invokeDatasetVersionsToBuildCardsFor(dataset);
54+
55+
// Assert
56+
assertEquals(1, result.size());
57+
assertTrue(result.contains(deaccessionedVersion));
58+
}
59+
60+
@Test
61+
public void testDatasetVersionsToBuildCardsFor_OnlyReleased() {
62+
// Arrange
63+
Dataset dataset = mock(Dataset.class);
64+
DatasetVersion releasedVersion = createMockVersion(1L, DatasetVersion.VersionState.RELEASED, false);
65+
66+
when(dataset.getLatestVersion()).thenReturn(releasedVersion);
67+
when(dataset.getReleasedVersion()).thenReturn(releasedVersion);
68+
69+
// Act
70+
Set<DatasetVersion> result = invokeDatasetVersionsToBuildCardsFor(dataset);
71+
72+
// Assert
73+
assertEquals(1, result.size());
74+
assertTrue(result.contains(releasedVersion));
75+
}
76+
77+
@Test
78+
public void testDatasetVersionsToBuildCardsFor_ReleasedAndDraft() {
79+
// Arrange
80+
Dataset dataset = mock(Dataset.class);
81+
DatasetVersion releasedVersion = createMockVersion(1L, DatasetVersion.VersionState.RELEASED, false);
82+
DatasetVersion draftVersion = createMockVersion(2L, DatasetVersion.VersionState.DRAFT, true);
83+
84+
when(dataset.getLatestVersion()).thenReturn(draftVersion);
85+
when(dataset.getReleasedVersion()).thenReturn(releasedVersion);
86+
87+
// Act
88+
Set<DatasetVersion> result = invokeDatasetVersionsToBuildCardsFor(dataset);
89+
90+
// Assert
91+
assertEquals(2, result.size());
92+
assertTrue(result.contains(releasedVersion));
93+
assertTrue(result.contains(draftVersion));
94+
}
95+
96+
@Test
97+
public void testDatasetVersionsToBuildCardsFor_ReleasedAndDeaccessioned() {
98+
// Arrange
99+
Dataset dataset = mock(Dataset.class);
100+
DatasetVersion releasedVersion = createMockVersion(1L, DatasetVersion.VersionState.RELEASED, false);
101+
DatasetVersion deaccessionedVersion = createMockVersion(2L, DatasetVersion.VersionState.DEACCESSIONED, false);
102+
103+
// Latest is deaccessioned, but there's a released version
104+
when(dataset.getLatestVersion()).thenReturn(deaccessionedVersion);
105+
when(dataset.getReleasedVersion()).thenReturn(releasedVersion);
106+
107+
// Act
108+
Set<DatasetVersion> result = invokeDatasetVersionsToBuildCardsFor(dataset);
109+
110+
// Assert
111+
// Should only return the released version, not the deaccessioned one
112+
assertEquals(1, result.size());
113+
assertTrue(result.contains(releasedVersion));
114+
assertFalse(result.contains(deaccessionedVersion));
115+
}
116+
117+
// Helper method to create mock DatasetVersion
118+
private DatasetVersion createMockVersion(Long id, DatasetVersion.VersionState state, boolean isDraft) {
119+
DatasetVersion version = mock(DatasetVersion.class);
120+
when(version.getId()).thenReturn(id);
121+
when(version.getVersionState()).thenReturn(state);
122+
when(version.isDraft()).thenReturn(isDraft);
123+
when(version.isReleased()).thenReturn(state == DatasetVersion.VersionState.RELEASED);
124+
when(version.isDeaccessioned()).thenReturn(state == DatasetVersion.VersionState.DEACCESSIONED);
125+
return version;
126+
}
127+
128+
// Helper method to invoke the private method using reflection
129+
@SuppressWarnings("unchecked")
130+
private Set<DatasetVersion> invokeDatasetVersionsToBuildCardsFor(Dataset dataset) {
131+
try {
132+
java.lang.reflect.Method method = SolrIndexServiceBean.class.getDeclaredMethod(
133+
"datasetVersionsToBuildCardsFor", Dataset.class);
134+
method.setAccessible(true);
135+
return (Set<DatasetVersion>) method.invoke(solrIndexServiceBean, dataset);
136+
} catch (Exception e) {
137+
throw new RuntimeException("Failed to invoke private method", e);
138+
}
139+
}
140+
}

0 commit comments

Comments
 (0)