Mark ExperimentalFactor as auto-generated

gemma-core/src/main/java/ubic/gemma/core/analysis/preprocess/SplitExperimentServiceImpl.java

@@ -394,6 +394,7 @@ private Collection<ExperimentalFactor> cloneExperimentalFactors( Collection<Expe
             clone.setType( ef.getType() );
             clone.getFactorValues().addAll( this.cloneFactorValues( ef.getFactorValues(), clone, old2cloneFV ) );
             clone.setExperimentalDesign( ed );
+            clone.setAutoGenerated( ef.isAutoGenerated() );
             result.add( clone );
             //    assert clone.getId() == null;
         }

gemma-core/src/main/java/ubic/gemma/core/analysis/preprocess/batcheffects/BatchInfoPopulationHelperServiceImpl.java

@@ -906,7 +906,7 @@ private <T> ExperimentalFactor createExperimentalFactor( ExpressionExperiment ee
                  * persist
                  */
                 fv.setCharacteristics( chars );
-                experimentService.addFactorValue( ee, fv );
+                experimentService.addFactorValue( ee, fv, true );
 
                 for ( T d : descriptorsToBatch.get( batchId ) ) {
                     d2fv.put( d, fv );
@@ -924,9 +924,8 @@ private ExperimentalFactor makeFactorForBatch( ExpressionExperiment ee ) {
         ef.setExperimentalDesign( ed );
         ef.setName( ExperimentFactorUtils.BATCH_FACTOR_NAME );
         ef.setDescription( "Scan date or similar proxy for 'batch'" + " extracted from the raw data files." );
-
-        ef = this.persistFactor( ee, ef );
-        return ef;
+        ef.setAutoGenerated( true );
+        return this.persistFactor( ee, ef );
     }
 
     private ExperimentalFactor persistFactor( ExpressionExperiment ee, ExperimentalFactor factor ) {

gemma-core/src/main/java/ubic/gemma/core/analysis/preprocess/batcheffects/BatchInfoPopulationServiceImpl.java

@@ -31,7 +31,10 @@
 import ubic.gemma.model.common.description.DatabaseEntry;
 import ubic.gemma.model.expression.bioAssay.BioAssay;
 import ubic.gemma.model.expression.biomaterial.BioMaterial;
-import ubic.gemma.model.expression.experiment.*;
+import ubic.gemma.model.expression.experiment.ExperimentFactorUtils;
+import ubic.gemma.model.expression.experiment.ExperimentalDesign;
+import ubic.gemma.model.expression.experiment.ExperimentalFactor;
+import ubic.gemma.model.expression.experiment.ExpressionExperiment;
 import ubic.gemma.persistence.service.common.auditAndSecurity.AuditEventService;
 import ubic.gemma.persistence.service.common.auditAndSecurity.AuditTrailService;
 import ubic.gemma.persistence.service.expression.bioAssay.BioAssayService;
@@ -375,33 +378,22 @@ static Map<String, String> readFastqHeaders( Path headerFile ) throws IOExceptio
      */
     private void removeExistingBatchFactor( ExpressionExperiment ee ) {
         ExperimentalDesign ed = ee.getExperimentalDesign();
-
         if ( ed == null ) {
             log.warn( ee + " does not have an experimental design, cannot remove batch factor." );
             return;
         }
-
-        ExperimentalFactor toRemove = null;
-
         for ( ExperimentalFactor ef : ed.getExperimentalFactors() ) {
-
             if ( ExperimentFactorUtils.isBatchFactor( ef ) ) {
-                toRemove = ef;
-                break;
-                /*
-                 * FIXME handle the case where we somehow have two or more.
-                 */
+                if ( !ef.isAutoGenerated() ) {
+                    log.warn( ef + " was manually curated, not removing it." );
+                    continue;
+                }
+                BatchInfoPopulationServiceImpl.log.info( "Removing existing batch factor: " + ef );
+                experimentalFactorService.remove( ef );
+                ee.getExperimentalDesign().getExperimentalFactors().remove( ef );
+                this.expressionExperimentService.update( ee );
             }
         }
-
-        if ( toRemove == null ) {
-            return;
-        }
-
-        BatchInfoPopulationServiceImpl.log.info( "Removing existing batch factor: " + toRemove );
-        experimentalFactorService.remove( toRemove );
-        ee.getExperimentalDesign().getExperimentalFactors().remove( toRemove );
-        this.expressionExperimentService.update( ee );
     }
 
 }

gemma-core/src/main/java/ubic/gemma/core/loader/expression/geo/GeoConverterImpl.java

@@ -247,6 +247,7 @@ public void convertSubsetToExperimentalFactor( ExpressionExperiment expExp, GeoS
 
         experimentalFactor.setType( FactorType.CATEGORICAL );
         experimentalFactor.setDescription( "Converted from GEO subset " + geoSubSet.getGeoAccession() );
+        experimentalFactor.setAutoGenerated( true );
 
         boolean duplicateExists = false;
         for ( ExperimentalFactor existingExperimentalFactor : existingExperimentalFactors ) {
@@ -1600,9 +1601,8 @@ private ExperimentalFactor convertReplicationToFactor( GeoReplication replicatio
         result.setName( replication.getType().toString() );
         result.setDescription( replication.getDescription() );
         result.setType( FactorType.CATEGORICAL );
-        Characteristic term = this.convertReplicatationType( replication.getType() );
-
-        result.setCategory( term );
+        result.setCategory( this.convertReplicatationType( replication.getType() ) );
+        result.setAutoGenerated( true );
         return result;
 
     }
@@ -2250,8 +2250,8 @@ private ExperimentalFactor convertVariableToFactor( GeoVariable variable ) {
         result.setDescription( variable.getDescription() );
         Characteristic term = Characteristic.Factory.newInstance();
         this.convertVariableType( term, variable.getType() );
-
         if ( term.getCategory() != null ) result.setCategory( term );
+        result.setAutoGenerated( true );
         return result;
     }
 

gemma-core/src/main/java/ubic/gemma/core/util/BeanWrapperUtils.java

@@ -0,0 +1,23 @@
+package ubic.gemma.core.util;
+
+import org.springframework.beans.BeanWrapper;
+import org.springframework.beans.PropertyValue;
+
+import javax.annotation.Nullable;
+import java.util.Objects;
+
+public class BeanWrapperUtils {
+
+    /**
+     * Set a property value on a {@link BeanWrapper} and return whether the value was changed.
+     * @return if the value was changed as per {@link Objects#equals(Object, Object)}
+     * @see BeanWrapper#setPropertyValue(PropertyValue)
+     */
+    public static <T> boolean setPropertyValue( BeanWrapper bw, String property, @Nullable T newVal ) {
+        if ( !Objects.equals( bw.getPropertyValue( property ), newVal ) ) {
+            bw.setPropertyValue( property, newVal );
+            return true;
+        }
+        return false;
+    }
+}

gemma-core/src/main/java/ubic/gemma/model/expression/experiment/ExperimentalFactor.java

@@ -53,6 +53,7 @@ public class ExperimentalFactor extends AbstractDescribable implements SecuredCh
     @Deprecated
     private Set<Characteristic> annotations = new HashSet<>();
     private ExpressionExperiment securityOwner;
+    private boolean autoGenerated;
 
     /**
      * No-arg constructor added to satisfy javabean contract
@@ -146,6 +147,18 @@ public void setAnnotations( Set<Characteristic> annotations ) {
         this.annotations = annotations;
     }
 
+
+    /**
+     * Indicate if this factor was automatically generated.
+     */
+    public boolean isAutoGenerated() {
+        return autoGenerated;
+    }
+
+    public void setAutoGenerated( boolean autoGenerated ) {
+        this.autoGenerated = autoGenerated;
+    }
+
     @Override
     public boolean equals( Object obj ) {
         if ( this == obj )
@@ -162,7 +175,9 @@ public boolean equals( Object obj ) {
 
     @Override
     public String toString() {
-        return super.toString() + ( type != null ? " Type=" + type : "" );
+        return super.toString()
+                + ( type != null ? " Type=" + type : "" )
+                + ( autoGenerated ? " [Auto-Generated]" : "" );
     }
 
     public static final class Factory {

gemma-core/src/main/java/ubic/gemma/model/expression/experiment/ExperimentalFactorValueObject.java

@@ -62,6 +62,12 @@ public class ExperimentalFactorValueObject extends IdentifiableValueObject<Exper
     @JsonInclude(JsonInclude.Include.NON_NULL)
     private Collection<FactorValueValueObject> values;
 
+    /**
+     * Indicate if this factor was auto-generated. We only expose this to the Gemma Web frontend.
+     */
+    @GemmaWebOnly
+    private boolean autoGenerated;
+
     /**
      * Required when using the class as a spring bean.
      */
@@ -136,6 +142,8 @@ public ExperimentalFactorValueObject( ExperimentalFactor factor, boolean include
                 // never include the experimental factor in the FV serialization since those would be redundant
                 .map( value -> new FactorValueValueObject( value, false ) )
                 .collect( Collectors.toSet() );
+
+        this.autoGenerated = factor.isAutoGenerated();
     }
 
     /**

gemma-core/src/main/java/ubic/gemma/model/expression/experiment/FactorValueValueObject.java

@@ -12,9 +12,11 @@
 import io.swagger.v3.oas.annotations.media.Schema;
 import lombok.Data;
 import lombok.EqualsAndHashCode;
-import ubic.gemma.model.util.ModelUtils;
 import ubic.gemma.model.annotations.GemmaWebOnly;
 import ubic.gemma.model.common.description.Characteristic;
+import ubic.gemma.model.util.ModelUtils;
+
+import javax.annotation.Nullable;
 
 /**
  * Each {@link FactorValue} can be associated with multiple characteristics (or with a measurement). However, for
@@ -51,6 +53,7 @@ public class FactorValueValueObject extends AbstractFactorValueValueObject {
     /**
      * It could be the id of the measurement if there is no characteristic.
      */
+    @Nullable
     @GemmaWebOnly
     private Long charId;
     @GemmaWebOnly

gemma-core/src/main/java/ubic/gemma/persistence/service/AbstractService.java

@@ -12,6 +12,7 @@
 import java.util.*;
 import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.util.Objects.requireNonNull;
@@ -89,6 +90,28 @@ public Collection<O> load( Collection<Long> ids ) {
         return mainDao.load( ids );
     }
 
+    @Override
+    @Transactional(readOnly = true)
+    public Collection<O> loadOrFail( Collection<Long> ids ) throws NullPointerException {
+        return loadOrFail( ids, NullPointerException::new );
+    }
+
+    @Override
+    @Transactional(readOnly = true)
+    public <T extends Exception> Collection<O> loadOrFail( Collection<Long> ids, Function<String, T> exceptionSupplier ) throws T {
+        ids = new HashSet<>( ids );
+        Collection<O> result = load( ids );
+        if ( result.size() < ids.size() ) {
+            for ( O o : result ) {
+                ids.remove( o.getId() );
+            }
+            throw exceptionSupplier.apply( String.format( "No %s with IDs %s found.",
+                    mainDao.getElementClass().getName(),
+                    ids.stream().sorted().map( String::valueOf ).collect( Collectors.joining( ", " ) ) ) );
+        }
+        return result;
+    }
+
     @Override
     @Transactional(readOnly = true)
     public O load( Long id ) {

gemma-core/src/main/java/ubic/gemma/persistence/service/BaseReadOnlyService.java

@@ -46,6 +46,14 @@ public interface BaseReadOnlyService<O extends Identifiable> {
      */
     Collection<O> load( Collection<Long> ids );
 
+    /**
+     * Load multiple objects or fail with a {@link NullPointerException} if any of the objects does not exist in the
+     * persistent storage.
+     */
+    Collection<O> loadOrFail( Collection<Long> ids ) throws NullPointerException;
+
+    <T extends Exception> Collection<O> loadOrFail( Collection<Long> ids, Function<String, T> exceptionSupplier ) throws T;
+
     /**
      * Loads object with given ID.
      *

gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/BioAssaySetServiceImpl.java

@@ -11,8 +11,10 @@
 import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 @Service
@@ -56,15 +58,40 @@ public BioAssaySet findOrFail( BioAssaySet entity ) {
     }
 
     @Override
+    @Transactional(readOnly = true)
     public Collection<BioAssaySet> load( Collection<Long> ids ) {
         Collection<BioAssaySet> result = new ArrayList<>();
         result.addAll( expressionExperimentService.load( ids ) );
         result.addAll( expressionExperimentSubSetService.load( ids ) );
         return result;
     }
 
+    @Override
+    @Transactional(readOnly = true)
+    public Collection<BioAssaySet> loadOrFail( Collection<Long> ids ) throws NullPointerException {
+        return loadOrFail( ids, NullPointerException::new );
+    }
+
+    @Override
+    @Transactional(readOnly = true)
+    public <T extends Exception> Collection<BioAssaySet> loadOrFail( Collection<Long> ids, Function<String, T> exceptionSupplier ) throws T {
+        ids = new HashSet<>( ids );
+        Collection<BioAssaySet> result = new ArrayList<>();
+        result.addAll( expressionExperimentService.load( ids ) );
+        result.addAll( expressionExperimentSubSetService.load( ids ) );
+        if ( result.size() < ids.size() ) {
+            for ( BioAssaySet o : result ) {
+                ids.remove( o.getId() );
+            }
+            throw exceptionSupplier.apply( String.format( "No BioAssaySet with IDs %s found.",
+                    ids.stream().sorted().map( String::valueOf ).collect( Collectors.joining( ", " ) ) ) );
+        }
+        return result;
+    }
+
     @Nullable
     @Override
+    @Transactional(readOnly = true)
     public BioAssaySet load( Long id ) {
         BioAssaySet bas;
         bas = expressionExperimentService.load( id );
@@ -76,6 +103,7 @@ public BioAssaySet load( Long id ) {
 
     @Nonnull
     @Override
+    @Transactional(readOnly = true)
     public BioAssaySet loadOrFail( Long id ) throws NullPointerException {
         BioAssaySet bas;
         bas = expressionExperimentService.load( id );
@@ -87,6 +115,7 @@ public BioAssaySet loadOrFail( Long id ) throws NullPointerException {
 
     @Nonnull
     @Override
+    @Transactional(readOnly = true)
     public <T extends Exception> BioAssaySet loadOrFail( Long id, Supplier<T> exceptionSupplier ) throws T {
         BioAssaySet bas;
         bas = expressionExperimentService.load( id );
@@ -98,6 +127,7 @@ public <T extends Exception> BioAssaySet loadOrFail( Long id, Supplier<T> except
 
     @Nonnull
     @Override
+    @Transactional(readOnly = true)
     public <T extends Exception> BioAssaySet loadOrFail( Long id, Function<String, T> exceptionSupplier ) throws T {
         BioAssaySet bas;
         bas = expressionExperimentService.load( id );
@@ -109,6 +139,7 @@ public <T extends Exception> BioAssaySet loadOrFail( Long id, Function<String, T
 
     @Nonnull
     @Override
+    @Transactional(readOnly = true)
     public <T extends Exception> BioAssaySet loadOrFail( Long id, Function<String, T> exceptionSupplier, String message ) throws T {
         BioAssaySet bas;
         bas = expressionExperimentService.load( id );
@@ -134,11 +165,13 @@ public long countAll() {
     }
 
     @Override
+    @Transactional(readOnly = true)
     public Stream<BioAssaySet> streamAll() {
         return Stream.concat( expressionExperimentService.streamAll(), expressionExperimentSubSetService.streamAll() );
     }
 
     @Override
+    @Transactional(readOnly = true)
     public Stream<BioAssaySet> streamAll( boolean createNewSession ) {
         return Stream.concat( expressionExperimentService.streamAll( createNewSession ), expressionExperimentSubSetService.streamAll( createNewSession ) );
     }

gemma-core/src/main/java/ubic/gemma/persistence/service/expression/experiment/ExperimentalDesignDao.java

@@ -19,7 +19,6 @@
 package ubic.gemma.persistence.service.expression.experiment;
 
 import ubic.gemma.model.expression.experiment.ExperimentalDesign;
-import ubic.gemma.model.expression.experiment.ExpressionExperiment;
 import ubic.gemma.persistence.service.BaseDao;
 
 import javax.annotation.Nullable;
@@ -29,14 +28,6 @@
  */
 public interface ExperimentalDesignDao extends BaseDao<ExperimentalDesign> {
 
-    ExperimentalDesign loadWithExperimentalFactors( Long id );
-
-    @Nullable
-    ExpressionExperiment getExpressionExperiment( ExperimentalDesign experimentalDesign );
-
-    @Nullable
-    ExpressionExperiment getExpressionExperimentById( Long experimentalDesignId );
-
     /**
      * Pick a random experimental design that needs attention.
      * @param excludedDesign an excluded design from sampling