Skip to content
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.

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

small refactorings #2595

Merged
merged 2 commits into from
Nov 28, 2024
Merged
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
Expand Up @@ -208,6 +208,11 @@ public void setModificationTime(Timestamp modificationTime) {
public Timestamp getIndexTime() {
return indexTime;
}

public boolean isIndexed() {

return this.indexTime != null;
}

/**
* indexTime is used for comparison with modificationTime so we know if the
Expand Down Expand Up @@ -329,6 +334,12 @@ public void setIdentifier(String identifier) {
public boolean isIdentifierRegistered() {
return identifierRegistered;
}

public boolean isStale() {
return isIndexed()
? this.indexTime.before(this.modificationTime)
: true;
}

public void setIdentifierRegistered(boolean identifierRegistered) {
this.identifierRegistered = identifierRegistered;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void setOwner(Dataverse owner) {

@Override
public Dataverse getOwner() {
return super.getOwner() != null ? (Dataverse) super.getOwner() : null;
return (Dataverse) super.getOwner();
}

public abstract boolean isPermissionRoot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import edu.harvard.iq.dataverse.persistence.datafile.DataFileCategory;
import edu.harvard.iq.dataverse.persistence.datafile.FileMetadata;
import edu.harvard.iq.dataverse.persistence.datafile.license.FileTermsOfUse;
import edu.harvard.iq.dataverse.persistence.dataverse.Dataverse;
import edu.harvard.iq.dataverse.persistence.dataverse.link.DatasetLinkingDataverse;
import edu.harvard.iq.dataverse.persistence.guestbook.Guestbook;
import edu.harvard.iq.dataverse.persistence.harvest.HarvestStyle;
Expand Down Expand Up @@ -336,6 +337,10 @@ public DatasetVersion getLatestVersionForCopy() {
public List<DatasetVersion> getVersions() {
return versions;
}

public Dataverse getRoot() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: IMHO it would fit better in Dataverse

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

return getOwner().getRoot();
}

public void setVersions(List<DatasetVersion> versions) {
this.versions = versions;
Expand Down Expand Up @@ -412,13 +417,12 @@ public boolean containsReleasedVersion() {
return isReleased() && getReleasedVersion() != null;
}

public DatasetVersion getReleasedVersion() {
for (DatasetVersion version : getVersions()) {
if (version.isReleased()) {
return version;
}
}
return null;
public DatasetVersion getReleasedVersion() {
return getVersions()
.stream()
.filter(DatasetVersion::isReleased)
.findFirst()
.orElse(null);
}

public List<DataFileCategory> getCategories() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
import java.util.stream.Stream;

import static com.google.common.collect.Lists.newArrayList;
import static org.apache.commons.lang3.StringUtils.EMPTY;
import static org.apache.commons.lang3.StringUtils.isNotBlank;

/**
* @author skraffmiller
Expand Down Expand Up @@ -414,7 +416,7 @@ public VersionState getPriorVersionState() {
public String getFriendlyVersionNumber() {
return isDraft()
? "DRAFT"
: versionNumber.toString() + "." + minorVersionNumber.toString();
: versionNumber + "." + minorVersionNumber;
}

public boolean isReleased() {
Expand Down Expand Up @@ -668,13 +670,8 @@ public String getDistributionDate() {

// TODO: Consider renaming this method since it's also used for getting the "provider" for Schema.org JSON-LD.
public String getRootDataverseNameForCitation() {
Dataverse root = dataset.getOwner();
while (root.getOwner() != null) {
root = root.getOwner();
}
String rootDataverseName = root.getName();
return StringUtils.isNotBlank(rootDataverseName)
? rootDataverseName : StringUtils.EMPTY;
final String rootDataverseName = this.dataset.getRoot().getName();
return isNotBlank(rootDataverseName) ? rootDataverseName : EMPTY;
}

public String getAuthorsStr() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ public boolean isHarvested() {
public boolean isRoot() {
return this.getOwner() == null;
}

public boolean isNotRoot() {
return this.getOwner() != null;
}

public List<Guestbook> getParentGuestbooks() {
List<Guestbook> retList = new ArrayList<>();
Expand Down Expand Up @@ -759,6 +763,10 @@ public List<Dataverse> getOwners() {
}
return owners;
}

public Dataverse getRoot() {
return isRoot() ? this : getOwner().getRoot();
}

@Override
public boolean equals(Object object) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
package edu.harvard.iq.dataverse.persistence.dataset;

import com.google.common.collect.Lists;
import edu.harvard.iq.dataverse.persistence.MocksFactory;
import edu.harvard.iq.dataverse.persistence.dataset.DatasetVersion.VersionState;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.sql.Timestamp;
import java.time.Instant;
import java.util.List;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import com.google.common.collect.Lists;

import edu.harvard.iq.dataverse.persistence.MocksFactory;
import edu.harvard.iq.dataverse.persistence.dataset.DatasetVersion.VersionState;
import edu.harvard.iq.dataverse.persistence.dataverse.Dataverse;


/**
Expand Down Expand Up @@ -115,6 +119,28 @@ public void testLocksManagement() {
assertFalse(sut.isLocked());

}

@Test
public void testGetRoot() {

Dataverse root = new Dataverse();
Dataverse child = new Dataverse();
child.setOwner(root);
Dataset grandChild = new Dataset();
grandChild.setOwner(child);

assertThat(root.getOwner()).isNull();
assertThat(root.isRoot()).isTrue();
assertThat(root.isNotRoot()).isFalse();

assertThat(child.getOwner()).isSameAs(root);
assertThat(child.isRoot()).isFalse();
assertThat(child.isNotRoot()).isTrue();
assertThat(child.getRoot()).isSameAs(root);

assertThat(grandChild.getOwner()).isSameAs(child);
assertThat(grandChild.getRoot()).isSameAs(root);
}

// -------------------- PRIVATE --------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
import edu.harvard.iq.dataverse.engine.command.impl.FinalizeDatasetPublicationCommand;
import edu.harvard.iq.dataverse.globalid.GlobalIdServiceBean;
import edu.harvard.iq.dataverse.persistence.DvObject;
import edu.harvard.iq.dataverse.persistence.datafile.DataFile;
import edu.harvard.iq.dataverse.persistence.dataset.Dataset;
import edu.harvard.iq.dataverse.persistence.dataset.DatasetField;
Expand Down Expand Up @@ -34,6 +35,10 @@
import javax.persistence.StoredProcedureQuery;
import javax.persistence.TemporalType;
import javax.persistence.TypedQuery;

import static java.lang.Math.max;
import static java.util.stream.Collectors.toList;

import java.io.InputStream;
import java.sql.Timestamp;
import java.time.Instant;
Expand All @@ -50,6 +55,7 @@
*/


@SuppressWarnings("serial")
@Stateless
public class DatasetDao implements java.io.Serializable {

Expand Down Expand Up @@ -90,6 +96,10 @@ public List<Dataset> findByOwnerId(Long ownerId) {
public List<Dataset> findAll() {
return em.createQuery("select object(o) from Dataset as o order by o.id", Dataset.class).getResultList();
}

public List<Dataset> findStaleOrMissingDatasets() {
return findAll().stream().filter(DvObject::isStale).collect(toList());
}

public List<Dataset> findNotIndexedAfterEmbargo() {
TypedQuery<Dataset> typedQuery = em.createQuery("select d from Dataset d, DvObject o where d.id = o.id and d.embargoDate < :actualTimestamp and d.embargoDate > o.indexTime", Dataset.class);
Expand All @@ -115,10 +125,7 @@ public List<Long> findAllUnindexed() {
* @see DataverseDao#findAllOrSubset(long, long, boolean)
*/
public List<Long> findAllOrSubset(long numPartitions, long partitionId, boolean skipIndexed) {
if (numPartitions < 1) {
long saneNumPartitions = 1;
numPartitions = saneNumPartitions;
}
numPartitions = max(numPartitions, 1);
String skipClause = skipIndexed ? "AND o.indexTime is null " : "";
TypedQuery<Long> typedQuery = em.createQuery("SELECT o.id FROM Dataset o WHERE MOD( o.id, :numPartitions) = :partitionId " +
skipClause +
Expand Down Expand Up @@ -216,9 +223,8 @@ public boolean isIdentifierUnique(String userIdentifier, Dataset dataset, Global
return !persistentIdSvc.alreadyExists(dataset);
} catch (Exception e) {
//we can live with failure - means identifier not found remotely
return true;
}

return true;
}

public boolean isIdentifierLocallyUnique(Dataset dataset) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
import javax.persistence.NonUniqueResultException;
import javax.persistence.PersistenceContext;
import javax.persistence.TypedQuery;

import static java.lang.Math.max;
import static java.util.stream.Collectors.toList;

import java.io.File;
import java.sql.Timestamp;
import java.util.ArrayList;
Expand All @@ -37,6 +41,7 @@
import java.util.Properties;
import java.util.concurrent.Future;
import java.util.logging.Logger;
import java.util.stream.Collectors;

/**
* @author gdurand
Expand Down Expand Up @@ -95,6 +100,18 @@ public Dataverse find(Object pk) {
public List<Dataverse> findAll() {
return em.createNamedQuery("Dataverse.findAll").getResultList();
}

/**
* @return Dataverses that should be reindexed either because they have
* never been indexed or their index time is before their modification time.
*/
public List<Dataverse> findStaleOrMissingDataverses() {
return findAll()
.stream()
.filter(Dataverse::isNotRoot)
.filter(Dataverse::isStale)
.collect(toList());
}

/**
* @param numPartitions The number of partitions you intend to split the
Expand All @@ -109,10 +126,7 @@ public List<Dataverse> findAll() {
* Otherwise, a subset of dataverses.
*/
public List<Dataverse> findAllOrSubset(long numPartitions, long partitionId, boolean skipIndexed) {
if (numPartitions < 1) {
long saneNumPartitions = 1;
numPartitions = saneNumPartitions;
}
numPartitions = max(numPartitions, 1);
String skipClause = skipIndexed ? "AND o.indexTime is null " : "";
TypedQuery<Dataverse> typedQuery = em.createQuery("SELECT OBJECT(o) FROM Dataverse AS o WHERE MOD( o.id, :numPartitions) = :partitionId " +
skipClause +
Expand All @@ -123,15 +137,16 @@ public List<Dataverse> findAllOrSubset(long numPartitions, long partitionId, boo
}

public List<Long> findDataverseIdsForIndexing(boolean skipIndexed) {
if (skipIndexed) {
return em.createQuery("SELECT o.id FROM Dataverse o WHERE o.indexTime IS null ORDER BY o.id", Long.class).getResultList();
}
return em.createQuery("SELECT o.id FROM Dataverse o ORDER BY o.id", Long.class).getResultList();
final String query = skipIndexed
? "SELECT o.id FROM Dataverse o WHERE o.indexTime IS null ORDER BY o.id"
: "SELECT o.id FROM Dataverse o ORDER BY o.id";
return em.createQuery(query, Long.class).getResultList();

}

public List<Dataverse> findByOwnerId(Long ownerId) {
return em.createNamedQuery("Dataverse.findByOwnerId").setParameter("ownerId", ownerId).getResultList();
return em.createNamedQuery("Dataverse.findByOwnerId", Dataverse.class)
.setParameter("ownerId", ownerId).getResultList();
}

public List<Long> findIdsByOwnerId(Long ownerId) {
Expand Down Expand Up @@ -348,16 +363,12 @@ public List<Long> findAllDataverseDataverseChildren(Long dvId) {
// get list of Dataverse children
List<Long> dataverseChildren = findIdsByOwnerId(dvId);

if (dataverseChildren == null) {
return dataverseChildren;
} else {
List<Long> newChildren = new ArrayList<>();
for (Long childDvId : dataverseChildren) {
newChildren.addAll(findAllDataverseDataverseChildren(childDvId));
}
dataverseChildren.addAll(newChildren);
return dataverseChildren;
List<Long> newChildren = new ArrayList<>();
for (Long childDvId : dataverseChildren) {
newChildren.addAll(findAllDataverseDataverseChildren(childDvId));
}
dataverseChildren.addAll(newChildren);
return dataverseChildren;
}

// function to recursively find ids of all children of a dataverse that are
Expand Down
Loading