Skip to content

Commit f20e75a

Browse files
authored
Fix for ConcurrentMod Exception seen in DatasetsIT test (#12128)
* Add synchronization, avoid sharing list across fmds, copy prov * make sure fmd is associated with correct version * make test use new method
1 parent 348dfad commit f20e75a

5 files changed

Lines changed: 27 additions & 74 deletions

File tree

src/main/java/edu/harvard/iq/dataverse/Dataset.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -384,21 +384,7 @@ private DatasetVersion createNewDatasetVersion(Template template, FileMetadata f
384384
in a pre-save validation SEK 12/6/2021
385385
*/
386386
for (FileMetadata fm : latestVersion.getFileMetadatas()) {
387-
FileMetadata newFm = new FileMetadata();
388-
// TODO:
389-
// the "category" will be removed, shortly.
390-
// (replaced by multiple, tag-like categories of
391-
// type DataFileCategory) -- L.A. beta 10
392-
//newFm.setCategory(fm.getCategory());
393-
// yep, these are the new categories:
394-
newFm.setCategories(fm.getCategories());
395-
newFm.setDescription(fm.getDescription());
396-
newFm.setLabel(fm.getLabel());
397-
newFm.setDirectoryLabel(fm.getDirectoryLabel());
398-
newFm.setRestricted(fm.isRestricted());
399-
newFm.setDataFile(fm.getDataFile());
400-
newFm.setDatasetVersion(dsv);
401-
newFm.setProvFreeForm(fm.getProvFreeForm());
387+
FileMetadata newFm = fm.createCopyInVersion(dsv);
402388
newFm.setInPriorVersion(true);
403389

404390
//fmVarMet would be updated in DCT
@@ -410,8 +396,6 @@ private DatasetVersion createNewDatasetVersion(Template template, FileMetadata f
410396
newFm.copyVarGroups(fm.getVarGroups());
411397
}
412398
}
413-
414-
dsv.getFileMetadatas().add(newFm);
415399
}
416400

417401
if (latestVersion.getTermsOfUseAndAccess()!= null){

src/main/java/edu/harvard/iq/dataverse/DatasetVersion.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -706,23 +706,7 @@ public DatasetVersion cloneDatasetVersion(){
706706
*/
707707

708708
for (FileMetadata fm : this.getFileMetadatas()) {
709-
FileMetadata newFm = new FileMetadata();
710-
// TODO:
711-
// the "category" will be removed, shortly.
712-
// (replaced by multiple, tag-like categories of
713-
// type DataFileCategory) -- L.A. beta 10
714-
//newFm.setCategory(fm.getCategory());
715-
// yep, these are the new categories:
716-
newFm.setCategories(fm.getCategories());
717-
newFm.setDescription(fm.getDescription());
718-
newFm.setLabel(fm.getLabel());
719-
newFm.setDirectoryLabel(fm.getDirectoryLabel());
720-
newFm.setRestricted(fm.isRestricted());
721-
newFm.setDataFile(fm.getDataFile());
722-
newFm.setDatasetVersion(dsv);
723-
newFm.setProvFreeForm(fm.getProvFreeForm());
724-
725-
dsv.getFileMetadatas().add(newFm);
709+
fm.createCopyInVersion(dsv);
726710
}
727711

728712
if (this.getTermsOfUseAndAccess()!= null){

src/main/java/edu/harvard/iq/dataverse/FileMetadata.java

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -154,21 +154,21 @@ public class FileMetadata implements Serializable {
154154
private Collection<VariableMetadata> variableMetadatas;
155155

156156
/**
157-
* Creates a copy of {@code this}, with identical business logic fields.
158-
* E.g., {@link #label} would be duplicated; {@link #version} will not.
157+
* Creates a copy of {@code this}, with identical business logic fields, making the bi-drectional connections to the specified version.
159158
*
160-
* @return A copy of {@code this}, except for the DB-related data.
159+
* @return A copy of {@code this}
161160
*/
162-
public FileMetadata createCopy() {
161+
public FileMetadata createCopyInVersion(DatasetVersion dsv) {
163162
FileMetadata fmd = new FileMetadata();
164163
fmd.setCategories(new LinkedList<>(getCategories()) );
165164
fmd.setDataFile( getDataFile() );
166-
fmd.setDatasetVersion( getDatasetVersion() );
165+
fmd.setDatasetVersion( dsv );
167166
fmd.setDescription( getDescription() );
168167
fmd.setLabel( getLabel() );
169168
fmd.setRestricted( isRestricted() );
170169
fmd.setDirectoryLabel(getDirectoryLabel());
171-
170+
fmd.setProvFreeForm(getProvFreeForm());
171+
dsv.getFileMetadatas().add(fmd);
172172
return fmd;
173173
}
174174

@@ -245,38 +245,26 @@ public void setVarGroups(List<VarGroup> varGroups) {
245245

246246
public List<DataFileCategory> getCategories() {
247247
if (fileCategories != null) {
248-
/*
249-
* fileCategories can sometimes be an
250-
* org.eclipse.persistence.indirection.IndirectList When that happens, the
251-
* comparator in the Collections.sort below is not called, possibly due to
252-
* https://bugs.eclipse.org/bugs/show_bug.cgi?id=446236 which is Java 1.8+
253-
* specific Converting to an ArrayList solves the problem, but the longer term
254-
* solution may be in avoiding the IndirectList or moving to a new version of
255-
* the jar it is in.
256-
*/
257-
if (!(fileCategories instanceof ArrayList)) {
258-
List<DataFileCategory> newDFCs = new ArrayList<DataFileCategory>();
259-
for (DataFileCategory fdc : fileCategories) {
260-
newDFCs.add(fdc);
248+
synchronized (this) {
249+
if (!(fileCategories instanceof ArrayList)) {
250+
fileCategories = new ArrayList<>(fileCategories);
261251
}
262-
setCategories(newDFCs);
252+
Collections.sort(fileCategories, FileMetadata.compareByNameWithSortCategories);
263253
}
264-
Collections.sort(fileCategories, FileMetadata.compareByNameWithSortCategories);
265254
}
266255
return fileCategories;
267256
}
268-
269-
public void setCategories(List<DataFileCategory> fileCategories) {
257+
258+
public synchronized void setCategories(List<DataFileCategory> fileCategories) {
270259
this.fileCategories = fileCategories;
271260
}
272-
273-
public void addCategory(DataFileCategory category) {
261+
262+
public synchronized void addCategory(DataFileCategory category) {
274263
if (fileCategories == null) {
275264
fileCategories = new ArrayList<>();
276265
}
277266
fileCategories.add(category);
278267
}
279-
280268
/**
281269
* Retrieve categories
282270
* @return

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateDatasetVersionCommand.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,11 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException {
6161

6262
registerExternalVocabValuesIfAny(ctxt, newVersion);
6363

64-
List<FileMetadata> newVersionMetadatum = new ArrayList<>(latest.getFileMetadatas().size());
65-
for ( FileMetadata fmd : latest.getFileMetadatas() ) {
66-
FileMetadata fmdCopy = fmd.createCopy();
67-
fmdCopy.setDatasetVersion(newVersion);
68-
newVersionMetadatum.add( fmdCopy );
64+
List<FileMetadata> latestVersionMetadata = latest.getFileMetadatas();
65+
newVersion.setFileMetadatas(new ArrayList<>(latest.getFileMetadatas().size()));
66+
for ( FileMetadata fmd : latestVersionMetadata ) {
67+
fmd.createCopyInVersion(newVersion);
6968
}
70-
newVersion.setFileMetadatas(newVersionMetadatum);
7169

7270
//moving prepare Dataset here
7371
//because it includes validation and we need the validation

src/test/java/edu/harvard/iq/dataverse/DatasetVersionDifferenceTest.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ public void testDifferencing() {
7676
datasetVersion2.setVersionState(DatasetVersion.VersionState.DRAFT);
7777
datasetVersion2.setTermsOfUseAndAccess(new TermsOfUseAndAccess());
7878
datasetVersion2.getTermsOfUseAndAccess().setLicense(license);
79-
79+
datasetVersion.setFileMetadatas(new ArrayList<>());
80+
8081
// Published version's two files
8182
DataFile dataFile = new DataFile();
8283
dataFile.setId(1L);
@@ -88,19 +89,17 @@ public void testDifferencing() {
8889

8990
FileMetadata fileMetadata2 = createFileMetadata(20L, datasetVersion, dataFile2, "file2.txt");
9091

92+
List<FileMetadata> fileMetadatas = new ArrayList<>(Arrays.asList(fileMetadata1, fileMetadata2));
93+
datasetVersion.setFileMetadatas(fileMetadatas);
94+
9195
// Draft version - same two files with one label change
92-
FileMetadata fileMetadata3 = fileMetadata1.createCopy();
96+
FileMetadata fileMetadata3 = fileMetadata1.createCopyInVersion(datasetVersion2);
9397
fileMetadata3.setId(30L);
9498

95-
FileMetadata fileMetadata4 = fileMetadata2.createCopy();
99+
FileMetadata fileMetadata4 = fileMetadata2.createCopyInVersion(datasetVersion2);
96100
fileMetadata4.setLabel("file3.txt");
97101
fileMetadata4.setId(40L);
98102

99-
List<FileMetadata> fileMetadatas = new ArrayList<>(Arrays.asList(fileMetadata1, fileMetadata2));
100-
datasetVersion.setFileMetadatas(fileMetadatas);
101-
List<FileMetadata> fileMetadatas2 = new ArrayList<>(Arrays.asList(fileMetadata3, fileMetadata4));
102-
datasetVersion2.setFileMetadatas(fileMetadatas2);
103-
104103
SimpleDateFormat dateFmt = new SimpleDateFormat("yyyyMMdd");
105104
Date publicationDate;
106105
try {

0 commit comments

Comments
 (0)