diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 201e7fb564c..910b20e288f 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -32,6 +32,7 @@ import org.labkey.api.attachments.ImageServlet; import org.labkey.api.attachments.LookAndFeelResourceType; import org.labkey.api.attachments.SecureDocumentType; +import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.cache.BlockingCache; import org.labkey.api.collections.ArrayListMap; import org.labkey.api.collections.CaseInsensitiveHashMap; @@ -463,6 +464,7 @@ public void registerServlets(ServletContext servletCtx) public @NotNull Set getIntegrationTests() { return Set.of( + AbstractAuditDomainKind.TestCase.class, AbstractForeignKey.TestCase.class, AbstractQueryUpdateService.TestCase.class, ActionURL.TestCase.class, diff --git a/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java b/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java index 366aa1484c1..e5fc84d182f 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java +++ b/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java @@ -23,7 +23,6 @@ import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.AbstractTableInfo; -import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; @@ -31,11 +30,8 @@ import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.DbScope; import org.labkey.api.data.MutableColumnInfo; -import org.labkey.api.data.PropertyStorageSpec; import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.SchemaTableInfo; import org.labkey.api.data.SqlExecutor; -import org.labkey.api.data.TableChange; import org.labkey.api.data.TableInfo; import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.dataiterator.ExistingRecordDataIterator; @@ -44,7 +40,6 @@ import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.StorageProvisioner; import org.labkey.api.exp.property.Domain; -import org.labkey.api.exp.property.DomainKind; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.gwt.client.DefaultValueType; @@ -55,14 +50,12 @@ import org.labkey.api.security.User; import org.labkey.api.util.DateUtil; import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import java.sql.Time; import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -90,9 +83,9 @@ public abstract class AbstractAuditTypeProvider implements AuditTypeProvider public static final String COLUMN_NAME_TRANSACTION_ID = "TransactionID"; public static final String COLUMN_NAME_DATA_CHANGES = "DataChanges"; - final AbstractAuditDomainKind _domainKind; - + private final AbstractAuditDomainKind _domainKind; + @Deprecated // Call the other constructor and stop overriding getDomainKind() public AbstractAuditTypeProvider() { this(null); @@ -100,8 +93,7 @@ public AbstractAuditTypeProvider() public AbstractAuditTypeProvider(@NotNull AbstractAuditDomainKind domainKind) { - // TODO : consolidate domain kind initialization to either this constructor or to override - // getDomainKind. + // TODO: consolidate domain kind initialization to this constructor and stop overriding getDomainKind() _domainKind = domainKind; // Register the DomainKind PropertyService.get().registerDomainKind(getDomainKind()); @@ -115,10 +107,18 @@ protected AbstractAuditDomainKind getDomainKind() return _domainKind; } + // Expose the domain kind to AbstractAuditDomainKind$TestCase without touching every subclass + public AbstractAuditDomainKind getAuditDomainKind() + { + return getDomainKind(); + } + @Override - public void initializeProvider(User user) + public final void initializeProvider(User user) { AbstractAuditDomainKind domainKind = getDomainKind(); + domainKind.validate(); + Domain domain = getDomain(true); // if the domain doesn't exist, create it @@ -145,52 +145,6 @@ public void initializeProvider(User user) ensureProperties(user, domain); } - private void updateIndices(Domain domain, AbstractAuditDomainKind domainKind) - { - if (domain.getStorageTableName() == null) - return; - - // Issue 50059, acquiring the schema table info this way ensures that the domain fields are properly fixed up. See ProvisionedSchemaOptions. - SchemaTableInfo schemaTableInfo = StorageProvisioner.get().getSchemaTableInfo(domain); - if (schemaTableInfo != null) - { - Map>> existingIndices = schemaTableInfo.getAllIndices(); - Set newIndices = new HashSet<>(domainKind.getPropertyIndices(domain)); - Set toRemove = new HashSet<>(); - for (String name : existingIndices.keySet()) - { - if (existingIndices.get(name).first == TableInfo.IndexType.Primary) - continue; - Pair> columnIndex = existingIndices.get(name); - String[] columnNames = new String[columnIndex.second.size()]; - for (int i = 0; i < columnIndex.second.size(); i++) - { - columnNames[i] = columnIndex.second.get(i).getColumnName(); - } - PropertyStorageSpec.Index existingIndex = new PropertyStorageSpec.Index(columnIndex.first == TableInfo.IndexType.Unique, columnNames); - boolean foundIt = false; - for (PropertyStorageSpec.Index propertyIndex : newIndices) - { - if (PropertyStorageSpec.Index.isSameIndex(propertyIndex, existingIndex)) - { - foundIt = true; - newIndices.remove(propertyIndex); - break; - } - } - - if (!foundIt) - toRemove.add(name); - } - - if (!toRemove.isEmpty()) - StorageProvisioner.get().dropTableIndices(domain, toRemove); - if (!newIndices.isEmpty()) - StorageProvisioner.get().addTableIndices(domain, newIndices, TableChange.IndexSizeMode.Normal); - } - } - - // NOTE: Changing the name of an existing PropertyDescriptor will lose data! private void ensureProperties(User user, Domain domain) { @@ -254,7 +208,11 @@ private void ensureProperties(User user, Domain domain) domain.save(user); } - updateIndices(domain, domainKind); + assert domain.getStorageTableName() != null; + assert domain.getDomainKind() != null; + assert domain.getDomainKind().getClass().equals(domainKind.getClass()); + + StorageProvisioner.get().ensureTableIndices(domain); transaction.commit(); } catch (ChangePropertyDescriptorException e) @@ -301,7 +259,7 @@ public final Domain getDomain() @Override public final Domain getDomain(boolean forUpdate) { - DomainKind domainKind = getDomainKind(); + AbstractAuditDomainKind domainKind = getDomainKind(); String domainURI = domainKind.generateDomainURI(QUERY_SCHEMA_NAME, getEventName(), getDomainContainer(), null); diff --git a/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java b/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java index 6b48c6f29ac..c3dfbbb0b11 100644 --- a/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java +++ b/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java @@ -18,8 +18,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONObject; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.AuditLogService; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.collections.LabKeyCollectors; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbSchema; @@ -45,23 +49,27 @@ import org.labkey.api.query.QueryService; import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; +import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.view.ActionURL; import org.labkey.api.view.NavTree; import org.labkey.api.writer.ContainerUser; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; + +import static org.labkey.api.audit.query.DefaultAuditTypeTable.LEGACY_UNION_AUDIT_TABLE; -/** - * User: klum - * Date: 7/8/13 - */ public abstract class AbstractAuditDomainKind extends DomainKind { - private static final String XAR_SUBSTITUTION_SCHEMA_NAME = "SchemaName"; private static final String XAR_SUBSTITUTION_TABLE_NAME = "TableName"; @@ -393,7 +401,9 @@ public Set getNonProvisionedTableNames() // omit the legacy auditlog table, this can be removed once the // table is dropped after migration Set tables = new CaseInsensitiveHashSet(); - tables.add("auditlog"); + + if (OptionalFeatureService.get().isFeatureEnabled(LEGACY_UNION_AUDIT_TABLE)) + tables.add("auditlog"); return tables; } @@ -418,4 +428,61 @@ public boolean isUserCreatedType() { return false; } + + private static final Set PREFIXES = new HashSet<>(); + + // Called at audit provider initialization time to forbid domain kinds with shared and overlapping namespace prefixes + public void validate() + { + String prefix = getNamespacePrefix(); + // This check for length >= 12 is arbitrary, meant to prevent "Audit-" and other trivial namespace prefixes that + // are likely to overlap and cause domain URIs to resolve to the wrong domain kinds. + if (prefix.length() < 12) + throw new IllegalStateException("Namespace prefix must be unique and longer than this: " + prefix); + + List overlappers = PREFIXES.stream() + .filter(p -> p.startsWith(prefix) || prefix.startsWith(p)) + .map(p -> "\"" + p + "\"") + .toList(); + + if (overlappers.isEmpty()) + PREFIXES.add(prefix); + else + throw new IllegalStateException("Namespace prefix \"" + prefix + "\" overlaps with " + overlappers); + } + + public static class TestCase extends Assert + { + // Also see AuditDomainUriTest which tests the DomainURIs in the database + @Test + public void flagDuplicateNamespacePrefixes() + { + Map> map = AuditLogService.get().getAuditProviders().stream() + .filter(AbstractAuditTypeProvider.class::isInstance) + .map(p -> ((AbstractAuditTypeProvider)p).getAuditDomainKind()) + .collect(LabKeyCollectors.toMultiValuedMap(AbstractAuditDomainKind::getNamespacePrefix, dk -> dk.getClass().getSimpleName())) + .asMap(); + + List failures = map.entrySet().stream() + .filter(e -> e.getValue().size() > 1) + .map(e -> e.getValue() + " share the same namespace prefix: \"" + e.getKey() + "\"!") + .collect(Collectors.toCollection(LinkedList::new)); + + failures.addAll(map.entrySet().stream() + .map(e1 -> { + String key1 = e1.getKey(); + List overlappers = map.entrySet().stream() + .filter(e2 -> !e1.equals(e2) && e2.getKey().startsWith(key1)) + .map(e2 -> "\"" + e2.getKey() + "\"") + .toList(); + + return overlappers.isEmpty() ? null : "Prefix " + key1 + " (" + e1.getValue() + ") overlaps with prefixes " + overlappers + ")!"; + }) + .filter(Objects::nonNull) + .toList()); + + if (!failures.isEmpty()) + Assert.fail(failures.toString()); + } + } } diff --git a/api/src/org/labkey/api/data/DbSchemaCache.java b/api/src/org/labkey/api/data/DbSchemaCache.java index c29b1d18f28..5657c72b989 100644 --- a/api/src/org/labkey/api/data/DbSchemaCache.java +++ b/api/src/org/labkey/api/data/DbSchemaCache.java @@ -26,12 +26,6 @@ import org.labkey.api.cache.CacheTimeChooser; import org.labkey.api.module.ModuleLoader; -/* -* User: adam -* Date: Mar 20, 2011 -* Time: 2:53:51 PM -*/ - // Every scope has its own cache of DbSchemas public class DbSchemaCache { @@ -68,7 +62,7 @@ public DbSchemaCache(DbScope scope) void remove(String schemaName, DbSchemaType type) { - LOG.debug("remove " + type + " schema: " + schemaName); + LOG.debug("remove {} schema: {}", type, schemaName); _cache.removeUsingFilter(new Cache.StringPrefixFilter(getKey(schemaName, type))); } diff --git a/api/src/org/labkey/api/data/PropertyStorageSpec.java b/api/src/org/labkey/api/data/PropertyStorageSpec.java index 66a40909c0e..9003a17fae0 100644 --- a/api/src/org/labkey/api/data/PropertyStorageSpec.java +++ b/api/src/org/labkey/api/data/PropertyStorageSpec.java @@ -31,14 +31,9 @@ import java.util.Set; /** - * User: newton - * Date: Aug 24, 2010 - * Time: 4:08:39 PM - * * The reason that we have this class, instead of doing something like reusing PropertyDescriptor, is that we also need * a storage spec like this when there is not a full property descriptor such as for the base properties of DomainKinds. * ColumnInfo and SqlColumn are also mismatched for various reasons. - * */ public class PropertyStorageSpec { @@ -470,7 +465,7 @@ public Index(boolean unique, boolean clustered, String... columnNames) /** * Determines if two indices are the same modulo the isClustered setting. This is useful for updating indices - * when an audit domain type changes, for example. + * when a domain changes, for example. */ public static boolean isSameIndex(Index propertyIndex, Index tableIndex) { diff --git a/api/src/org/labkey/api/data/TableChange.java b/api/src/org/labkey/api/data/TableChange.java index f5924b02fe7..02e6c3fef92 100644 --- a/api/src/org/labkey/api/data/TableChange.java +++ b/api/src/org/labkey/api/data/TableChange.java @@ -234,7 +234,7 @@ public void addColumn(PropertyDescriptor prop) addColumn(new PropertyStorageSpec(prop)); } - /** Add the property to the set of columns tracked in this change along with it's old scale. */ + /** Add the property to the set of columns tracked in this change along with its old scale. */ public void addColumnResize(PropertyDescriptor prop, Integer oldScale) { addColumn(prop); @@ -248,7 +248,7 @@ public void addColumnRename(String oldName, String newName) /** * Index will be renamed using the columns listed in the Index. - * The columns used by the index won't be changed. We need to + * The columns used by the index won't be changed. We need to * pass the list of columns since the index name is created by the dialect. * * @param oldIndex Old index to be renamed. @@ -309,7 +309,8 @@ public void setIndexedColumns(Domain domain, Collection indices) _indices = indices.stream().map(i -> i.translateToStorageNames(domain)).toList(); } - public Set getIndicesToBeDroppedByName(){ + public Set getIndicesToBeDroppedByName() + { return _indicesToBeDroppedByName; } diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 245fd01920b..7f3d6d43367 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -38,6 +38,7 @@ import org.labkey.api.data.JdbcType; import org.labkey.api.data.MetadataSqlSelector; import org.labkey.api.data.PropertyStorageSpec; +import org.labkey.api.data.PropertyStorageSpec.Index; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.Selector; @@ -1070,15 +1071,10 @@ private void addDropIndexByNameStatements(List statements, TableChange c { for (String indexName : change.getIndicesToBeDroppedByName()) { - statements.add(getDropIndexCommand(indexName, change)); + statements.add(getDropIndexCommand(change, indexName)); } } - private String getDropIndexCommand(String indexName, TableChange change) - { - return getDropIndexCommand(change,indexName); - } - private String getDropIndexCommand(TableChange change, String indexName) { return "DROP INDEX " + change.getSchemaName() + "." + indexName; @@ -1140,7 +1136,6 @@ else if (column.getJdbcType().isDecimal()) else { dbType = getSqlTypeName(column.getJdbcType()); - } //Postgres retains the existing null behavior @@ -1171,11 +1166,14 @@ private List getRenameColumnsStatement(TableChange change) } } - for (Map.Entry oldToNew : change.getIndexRenames().entrySet()) + // TODO: This loop should not guess the name of the old indices; instead, it should look them up. + // TableChange.setIndexedColumns() could set _indexRenames providing the name, and then this code uses that info. + // Or maybe schemaTableInfo.getAllIndices() and then use Index.isSameIndex() to find names. Issue 53838. + for (Map.Entry oldToNew : change.getIndexRenames().entrySet()) { - PropertyStorageSpec.Index oldIndex = oldToNew.getKey(); - PropertyStorageSpec.Index newIndex = oldToNew.getValue(); - String oldName = nameIndex(change.getTableName(), oldIndex.columnNames); + Index oldIndex = oldToNew.getKey(); + Index newIndex = oldToNew.getValue(); + String oldName = nameIndex(change.getTableName(), oldIndex.columnNames); // TODO: Look up name String newName = nameIndex(change.getTableName(), newIndex.columnNames); if (!oldName.equals(newName)) { @@ -1325,18 +1323,19 @@ private List getCreateIndexStatements(TableChange change) private void addCreateIndexStatements(List statements, TableChange change) { - for (PropertyStorageSpec.Index index : change.getIndexedColumns()) + for (Index index : change.getIndexedColumns()) { + String newIndexName = nameIndex(change.getTableName(), index.columnNames); statements.add(String.format("CREATE %sINDEX %s ON %s (%s);", index.isUnique ? "UNIQUE " : "", - nameIndex(change.getTableName(), index.columnNames), + newIndexName, makeTableIdentifier(change), makePropertyIdentifiers(index.columnNames))); if (index.isClustered) { statements.add(String.format("%s %s.%s USING %s", PropertyStorageSpec.CLUSTER_TYPE.CLUSTER, change.getSchemaName(), - change.getTableName(), nameIndex(change.getTableName(), index.columnNames))); + change.getTableName(), newIndexName)); } } } diff --git a/api/src/org/labkey/api/exp/api/StorageProvisioner.java b/api/src/org/labkey/api/exp/api/StorageProvisioner.java index 52a05ca79a0..b4a23ef71e5 100644 --- a/api/src/org/labkey/api/exp/api/StorageProvisioner.java +++ b/api/src/org/labkey/api/exp/api/StorageProvisioner.java @@ -22,7 +22,6 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.PropertyStorageSpec; import org.labkey.api.data.SchemaTableInfo; -import org.labkey.api.data.TableChange; import org.labkey.api.data.TableInfo; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.property.Domain; @@ -36,6 +35,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Supplier; /** * Creates and maintains "hard" tables in the underlying database based on dynamically configured data types. @@ -83,10 +83,8 @@ static TableInfo createTableInfo(@NotNull Domain domain) */ void createStorageTable(Domain domain, DomainKind kind, DbScope scope); - void dropNotRequiredIndices(Domain domain); - void addMissingRequiredIndices(Domain domain); - void addTableIndices(Domain domain, Set indices, TableChange.IndexSizeMode sizeMode); - void dropTableIndices(Domain domain, Set indexNames); + void ensureTableIndices(@NotNull Domain domain); + void ensureTableIndices(@NotNull Domain domain, Supplier afterAddSupplier); SchemaTableInfo getSchemaTableInfo(Domain domain); diff --git a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java index 4f4634368b8..d5ade35f243 100644 --- a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java @@ -50,11 +50,10 @@ public class DomainAuditProvider extends AbstractAuditTypeProvider implements AuditTypeProvider { public static final String EVENT_TYPE = "DomainAuditEvent"; - public static final String COLUMN_NAME_DOMAIN_URI = "DomainUri"; public static final String COLUMN_NAME_DOMAIN_NAME = "DomainName"; - static final List defaultVisibleColumns = new ArrayList<>(); + private static final List defaultVisibleColumns = new ArrayList<>(); static { diff --git a/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java index 0b48e114797..e4d3f2bd777 100644 --- a/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java @@ -125,8 +125,7 @@ protected void initColumn(MutableColumnInfo col) public TableInfo getLookupTableInfo() { DomainAuditProvider provider = new DomainAuditProvider(); - TableInfo table = provider.createTableInfo(getUserSchema(), ContainerFilter.getUnsafeEverythingFilter()); - return table; + return provider.createTableInfo(getUserSchema(), ContainerFilter.getUnsafeEverythingFilter()); } }); } diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index 6b83d805e70..e096f4189c8 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -774,7 +774,7 @@ public static ValidationException updateDomainDescriptor(GWTDomain * // Register the Security Escalation Audit Log * AuditLogService.get().registerAuditType(new SecurityEscalatorAuditProvider()); @@ -53,8 +50,6 @@ */ public abstract class SecurityEscalationAuditProvider extends AbstractAuditTypeProvider implements AuditTypeProvider { - private static final Logger _log = LogManager.getLogger(SecurityEscalationAuditProvider.class); - protected SecurityEscalationAuditProvider() { super(); @@ -92,10 +87,10 @@ public enum CustomColumn { LEVEL ("level", "Level", PropertyType.INTEGER) ; - String displayName; - String columnName; + final String displayName; + final String columnName; boolean isHidden = false; - PropertyType type; + final PropertyType type; CustomColumn(String columnName, String displayName, PropertyType type) { this.displayName = displayName; @@ -285,9 +280,10 @@ public void setStackTrace(String stackTrace) { */ public static abstract class SecurityEscalationAuditDomainKind extends AbstractAuditDomainKind { - private static final String NAMESPACE_PREFIX = "Audit-"; private static Set _fields = null; + protected static final String NAMESPACE_PREFIX = "Audit-"; + private final String _eventType; private final String _tableName; @@ -310,11 +306,6 @@ public String getKindName() { return _eventType; } - @Override - protected String getNamespacePrefix() { - return NAMESPACE_PREFIX; - } - /** * Returns a list of the custom columns to be recorded in the audit log. * diff --git a/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java b/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java index b15658ee964..d3393974337 100644 --- a/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java +++ b/api/src/org/labkey/api/study/security/StudySecurityEscalationAuditProvider.java @@ -47,7 +47,7 @@ public String getAuditLogTitle() { public StudySecurityEscalationAuditProvider() { - super(new StudySecurityEscalationDomain()); + super(new StudySecurityEscalationDomainKind()); } public static class StudySecurityEscalationEvent extends SecurityEscalationEvent @@ -67,9 +67,17 @@ public String getEventType() { } } - public static class StudySecurityEscalationDomain extends SecurityEscalationAuditDomainKind { - public StudySecurityEscalationDomain() { + public static class StudySecurityEscalationDomainKind extends SecurityEscalationAuditDomainKind + { + public StudySecurityEscalationDomainKind() + { super(EVENT_TYPE, StudySecurityEscalationEvent.class.getName()); } + + @Override + protected String getNamespacePrefix() + { + return NAMESPACE_PREFIX + "StudySecurityEscalationDomain"; + } } } diff --git a/api/src/org/labkey/api/wiki/WikiService.java b/api/src/org/labkey/api/wiki/WikiService.java index 7b04f3519d6..dfae120d75b 100644 --- a/api/src/org/labkey/api/wiki/WikiService.java +++ b/api/src/org/labkey/api/wiki/WikiService.java @@ -30,11 +30,6 @@ import java.sql.SQLException; import java.util.List; -/** - * User: Mark Igra - * Date: Jun 12, 2006 - * Time: 2:48:54 PM - */ public interface WikiService { static @Nullable WikiService get() @@ -47,8 +42,8 @@ static void setInstance(WikiService impl) ServiceRegistry.get().registerService(WikiService.class, impl); } - WebPartView getView(Container c, String name, boolean renderContentOnly); - WebPartView getHistoryView(Container c, String name); + WebPartView getView(Container c, String name, boolean renderContentOnly); + WebPartView getHistoryView(Container c, String name); HtmlString getHtml(Container c, String name); diff --git a/audit/src/org/labkey/audit/model/LogManager.java b/audit/src/org/labkey/audit/model/LogManager.java index 91984edc2a9..0f64bba8367 100644 --- a/audit/src/org/labkey/audit/model/LogManager.java +++ b/audit/src/org/labkey/audit/model/LogManager.java @@ -291,5 +291,4 @@ private K validateFields(@NotNull AuditTypeProvider p else return type; } - } diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-25.007-25.008.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-25.007-25.008.sql new file mode 100644 index 00000000000..5d3a3400d9a --- /dev/null +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-25.007-25.008.sql @@ -0,0 +1,10 @@ +-- Update all "security escalation" domains with a proper namespace prefix +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, '--', '-StudySecurityEscalationDomain-') WHERE StorageSchemaName = 'audit' AND (DomainURI ILIKE '%StudySecurityEscalationAuditProvider' OR DomainURI ILIKE '%StudySecurityEscalationEvent'); +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, '--', '-EHRSecurityEscalationDomain-') WHERE StorageSchemaName = 'audit' AND DomainURI ILIKE '%EHRSecurityEscalationEvent'; + +-- Update LDAP sync domain with a unique namespace prefix so it doesn't overlap with the ONPRC version +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, 'LdapSyncAuditDomain', 'PremiumLdapSyncAuditDomain') WHERE StorageSchemaName = 'audit' AND DomainURI ILIKE '%PremiumLdapAuditEvent'; + +-- Update query audit events that advertised the same generic namespace prefix +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, 'QueryAuditDomain', 'QueryExportAuditDomain') WHERE StorageSchemaName = 'audit' AND DomainURI ILIKE '%QueryExportAuditEvent'; +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, 'QueryAuditDomain', 'LoggedQueryAuditDomain') WHERE StorageSchemaName = 'audit' AND DomainURI ILIKE '%LoggedQuery'; \ No newline at end of file diff --git a/experiment/resources/schemas/dbscripts/sqlserver/exp-25.007-25.008.sql b/experiment/resources/schemas/dbscripts/sqlserver/exp-25.007-25.008.sql new file mode 100644 index 00000000000..dbfb724e1cf --- /dev/null +++ b/experiment/resources/schemas/dbscripts/sqlserver/exp-25.007-25.008.sql @@ -0,0 +1,10 @@ +-- Update all "security escalation" domains with a proper namespace prefix +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, '--', '-StudySecurityEscalationDomain-') WHERE StorageSchemaName = 'audit' AND (DomainURI LIKE '%StudySecurityEscalationAuditProvider' OR DomainURI LIKE '%StudySecurityEscalationEvent'); +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, '--', '-EHRSecurityEscalationDomain-') WHERE StorageSchemaName = 'audit' AND DomainURI LIKE '%EHRSecurityEscalationEvent'; + +-- Update LDAP sync domain with a unique namespace prefix so it doesn't overlap with the ONPRC version +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, 'LdapSyncAuditDomain', 'PremiumLdapSyncAuditDomain') WHERE StorageSchemaName = 'audit' AND DomainURI LIKE '%PremiumLdapAuditEvent'; + +-- Update query audit events that advertised the same generic namespace prefix +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, 'QueryAuditDomain', 'QueryExportAuditDomain') WHERE StorageSchemaName = 'audit' AND DomainURI LIKE '%QueryExportAuditEvent'; +UPDATE exp.domainDescriptor SET DomainURI = REPLACE(DomainURI, 'QueryAuditDomain', 'LoggedQueryAuditDomain') WHERE StorageSchemaName = 'audit' AND DomainURI LIKE '%LoggedQuery'; \ No newline at end of file diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 34168ffa84b..3694ca50fb7 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -191,7 +191,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.007; + return 25.008; } @Nullable @@ -1004,6 +1004,7 @@ public Set getIntegrationTests() DomainImpl.TestCase.class, DomainPropertyImpl.TestCase.class, ExpDataTableImpl.TestCase.class, + ExperimentServiceImpl.AuditDomainUriTest.class, ExperimentServiceImpl.LineageQueryTestCase.class, ExperimentServiceImpl.ParseInputOutputAliasTestCase.class, ExperimentServiceImpl.TestCase.class, diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 584ce9afeda..c65ddea958e 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -58,6 +58,7 @@ import org.labkey.api.cache.CacheManager; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.collections.CsvSet; import org.labkey.api.collections.LongArrayList; import org.labkey.api.collections.LongHashMap; import org.labkey.api.collections.Sets; @@ -74,6 +75,7 @@ import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbSequenceManager; +import org.labkey.api.data.Filter; import org.labkey.api.data.JdbcType; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.ObjectFactory; @@ -304,6 +306,7 @@ import static org.labkey.api.data.NameGenerator.EXPERIMENTAL_ALLOW_GAP_COUNTER; import static org.labkey.api.data.NameGenerator.EXPERIMENTAL_WITH_COUNTER; import static org.labkey.api.dataiterator.DataIteratorUtil.DUPLICATE_COLUMN_IN_DATA_ERROR; +import static org.labkey.api.exp.OntologyManager.getTinfoDomainDescriptor; import static org.labkey.api.exp.OntologyManager.getTinfoObject; import static org.labkey.api.exp.XarContext.XAR_JOB_ID_NAME; import static org.labkey.api.exp.api.ExpProtocol.ApplicationType.ExperimentRun; @@ -311,11 +314,11 @@ import static org.labkey.api.exp.api.ExpProtocol.ApplicationType.ProtocolApplication; import static org.labkey.api.exp.api.ExperimentJSONConverter.DATA_INPUTS_ALIAS_PREFIX; import static org.labkey.api.exp.api.ExperimentJSONConverter.MATERIAL_INPUTS_ALIAS_PREFIX; -import static org.labkey.api.util.IntegerUtils.asIntegerElseNull; -import static org.labkey.api.util.IntegerUtils.asLong; import static org.labkey.api.exp.api.NameExpressionOptionService.NAME_EXPRESSION_REQUIRED_MSG; import static org.labkey.api.exp.api.NameExpressionOptionService.NAME_EXPRESSION_REQUIRED_MSG_WITH_SUBFOLDERS; import static org.labkey.api.exp.api.ProvenanceService.PROVENANCE_PROTOCOL_LSID; +import static org.labkey.api.util.IntegerUtils.asIntegerElseNull; +import static org.labkey.api.util.IntegerUtils.asLong; import static org.labkey.api.util.IntegerUtils.asLongElseNull; import static org.labkey.experiment.api.SampleTypeServiceImpl.SampleChangeType.rollup; @@ -10329,6 +10332,41 @@ private int countMaterialInputObjects(Container c) } } + @SuppressWarnings("JUnitMalformedDeclaration") + public static class AuditDomainUriTest extends Assert + { + @Test + public void testAuditDomainUris() + { + // Each audit domain URI in the database should resolve to a unique DomainKind. A failure here indicates a + // problem with a DomainKind's namespace prefix or the resolution mechanism. Test lives here because it + // queries exp tables. See related test AbstractAuditDomainKind$TestCase.flagDuplicateNamespacePrefixes(). + TableInfo dd = getTinfoDomainDescriptor(); + Filter filter = new SimpleFilter(FieldKey.fromParts("StorageSchemaName"), "audit"); + PropertyService svc = PropertyService.get(); + Set> retrievedKinds = new HashSet<>(); + // This domain kind changed names at some point, so old deployments could have two rows that map to it -- tolerate + Set ignore = Set.of("StudySecurityEscalationEvent"); + List failures = new TableSelector(dd, new CsvSet("Name, DomainURI"), filter, new Sort("Name")).mapStream() + .map(map -> { + String name = (String)map.get("Name"); + String uri = (String)map.get("DomainURI"); + DomainKind kind = svc.getDomainKind(uri); + String ret = null; + if (null == kind) + LOG.info("{} ({}) did not resolve", uri, name); + else if (!ignore.contains(kind.getKindName()) && !retrievedKinds.add(kind)) + ret = name + ": " + uri + " ==> " + kind; + return ret; + }) + .filter(Objects::nonNull) + .toList(); + + if (!failures.isEmpty()) + Assert.fail(StringUtilsLabKey.pluralize(failures.size(), "duplicate domain kind") + "! " + failures); + } + } + public record edge(long to, int from) {} public static class InnerResult { diff --git a/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java b/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java index 5e451c144da..75c01943dcd 100644 --- a/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java @@ -28,7 +28,6 @@ import org.labkey.api.action.SpringActionController; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; -import org.labkey.api.collections.CaseInsensitiveMapWrapper; import org.labkey.api.collections.Sets; import org.labkey.api.data.BaseColumnInfo; import org.labkey.api.data.ColumnInfo; @@ -45,6 +44,7 @@ import org.labkey.api.data.MVDisplayColumnFactory; import org.labkey.api.data.ParameterMapStatement; import org.labkey.api.data.PropertyStorageSpec; +import org.labkey.api.data.PropertyStorageSpec.Index; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SchemaTableInfo; @@ -98,6 +98,7 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -131,7 +132,7 @@ private StorageProvisionerImpl() } // #42641: Track recently created tables in a cache to limit size and duration - private static final Cache RECENTLY_CREATED_TABLES = CacheBuilder.newBuilder() + private static final Cache<@NotNull String, StackTraceElement @NotNull []> RECENTLY_CREATED_TABLES = CacheBuilder.newBuilder() .maximumSize(10000) .expireAfterWrite(1, TimeUnit.DAYS) .build(); @@ -203,7 +204,7 @@ private String _create(DbScope scope, DomainKind kind, Domain domain, boolean } } - List indices = new ArrayList<>(); + List indices = new ArrayList<>(); indices.addAll(kind.getPropertyIndices(domain)); indices.addAll(domain.getPropertyIndices()); change.setIndexedColumns(domain, indices); @@ -533,7 +534,7 @@ public void resizeProperty(Domain domain, DomainProperty prop, Integer oldScale) DomainKind kind = domain.getDomainKind(); DbScope scope = kind.getScope(); - // should be in a transaction with propertydescriptor changes + // should be in a transaction with property descriptor changes if (!scope.isTransactionActive()) throw new ChangePropertyDescriptorException("Unable to change property size. Transaction is not active within change scope"); @@ -674,7 +675,7 @@ public SQLFragment getValueSql(String tableAlias) @Override public String toString() { - // really shouldn't be doing this any more, use getSelectName()? + // really shouldn't be doing this anymore, use getSelectName()? return _inner.toString(); } @@ -853,163 +854,7 @@ public void createStorageTable(Domain domain, DomainKind kind, DbScope scope) _create(scope, kind, domain, true); } - enum RequiredIndicesAction - { - Drop - { - @Override - public void doOperation(StorageProvisionerImpl provisioner, Domain domain, SchemaTableInfo schemaTableInfo, Map requiredIndicesMap) - { - provisioner.dropNotRequiredIndices(domain, schemaTableInfo, requiredIndicesMap); - } - }, - Add - { - @Override - public void doOperation(StorageProvisionerImpl provisioner, Domain domain, SchemaTableInfo schemaTableInfo, Map requiredIndicesMap) - { - provisioner.addMissingRequiredIndices(domain, schemaTableInfo, requiredIndicesMap); - } - }; - - public abstract void doOperation(StorageProvisionerImpl provisioner, Domain domain, SchemaTableInfo schemaTableInfo, Map requiredIndicesMap); - } - - @Override - public void dropNotRequiredIndices(Domain domain) - { - if (!domain.isProvisioned()){ - return; - } - updateTableIndices(domain, RequiredIndicesAction.Drop); - } - - @Override - public void addMissingRequiredIndices(Domain domain) - { - updateTableIndices(domain, RequiredIndicesAction.Add); - } - - private void updateTableIndices(Domain domain, @NotNull RequiredIndicesAction requiredIndicesAction) - { - SchemaTableInfo schemaTableInfo = getSchemaTableInfo(domain); - - SqlDialect sqlDialect = getSqlDialect(domain); - - Map requiredIndicesMap = getRequiredIndices(domain, sqlDialect); - - requiredIndicesAction.doOperation(this, domain, schemaTableInfo, requiredIndicesMap); - } - - @NotNull - private Map getRequiredIndices(Domain domain, SqlDialect sqlDialect) - { - Collection requiredIndices = new ArrayList<>(); - requiredIndices.addAll(domain.getDomainKind().getPropertyIndices(domain)); - requiredIndices.addAll(domain.getPropertyIndices()); - - Map requiredIndicesMap = new CaseInsensitiveMapWrapper<>(new HashMap<>()); - - String storageTableName = domain.getStorageTableName(); - - if (storageTableName == null) - { - storageTableName = makeTableName(getDomainKind(domain),domain); - } - - for (PropertyStorageSpec.Index index : requiredIndices) - { - // TODO: Bad!! Shouldn't be making up an index name here! Ideally, we use the AbstractAuditTypeProvider.updateIndices() approach instead. - requiredIndicesMap.put(sqlDialect.nameIndex(storageTableName, index.columnNames), index); - } - return requiredIndicesMap; - } - - private void dropNotRequiredIndices(Domain domain, SchemaTableInfo schemaTableInfo, Map requiredIndicesMap) - { - Set indicesToDrop = new HashSet<>(); - - // Determine indices to drop - for (Map.Entry>> index : schemaTableInfo.getAllIndices().entrySet()) - { - boolean isPrimaryKey = index.getValue().getKey().equals(TableInfo.IndexType.Primary); - boolean tableIndexNameNotFoundInRequiredIndices = !requiredIndicesMap.containsKey(index.getKey().toLowerCase()); - - if (!isPrimaryKey && tableIndexNameNotFoundInRequiredIndices) - { - indicesToDrop.add(index.getKey()); - } - } - - if (!indicesToDrop.isEmpty()) - { - TableChange change = new TableChange(domain, ChangeType.DropIndicesByName); - - change.setIndicesToBeDroppedByName(indicesToDrop); - - try (Transaction transaction = getScope(domain).ensureTransaction()) - { - change.execute(); - transaction.commit(); - } - } - } - - private void addMissingRequiredIndices(Domain domain, SchemaTableInfo schemaTableInfo, Map requiredIndicesMap) - { - Set indicesToAdd = new HashSet<>(); - - // Build the list of indices to add - CaseInsensitiveHashSet tableIndexNames = new CaseInsensitiveHashSet(schemaTableInfo.getAllIndices().keySet()); - for (Map.Entry requiredIndexEntry : requiredIndicesMap.entrySet()) - { - boolean requiredIndexNotFoundInTable = !tableIndexNames.contains(requiredIndexEntry.getKey()); - if (requiredIndexNotFoundInTable) - { - ensureIndexToBeAddedHasNoPrimaryKeys(schemaTableInfo, requiredIndexEntry); - indicesToAdd.add(requiredIndexEntry.getValue()); - } - } - - if (!indicesToAdd.isEmpty()) - { - TableChange change; - if (domain.getStorageTableName() == null) - { - change = new TableChange(domain, ChangeType.AddIndices, makeTableName(getDomainKind(domain),domain)); - } - else - { - change = new TableChange(domain, ChangeType.AddIndices); - } - - change.getIndexedColumns().addAll(indicesToAdd); - - try (Transaction transaction = getScope(domain).ensureTransaction()) - { - change.execute(); - transaction.commit(); - } - } - } - - private static void ensureIndexToBeAddedHasNoPrimaryKeys(SchemaTableInfo schemaTableInfo, Map.Entry requiredIndexEntry) - { - for (String indexColumnName : requiredIndexEntry.getValue().columnNames) - { - for (String primaryKeyName : schemaTableInfo.getPkColumnNames()) - { - if (indexColumnName.equalsIgnoreCase(primaryKeyName)) - { - throw new UnsupportedOperationException( - "Adding an index with primary key columns is not supported. Primary keys are " + String.join(",", schemaTableInfo.getPkColumnNames())); - } - } - } - } - - @Override - public void addTableIndices(Domain domain, Set indices, TableChange.IndexSizeMode sizeMode) + private void addTableIndices(Domain domain, Set indices, TableChange.IndexSizeMode sizeMode) { DbScope scope = validateDomain(domain); @@ -1042,8 +887,7 @@ private DbScope validateDomain(Domain domain) return scope; } - @Override - public void dropTableIndices(Domain domain, Set indexNames) + private void dropTableIndices(Domain domain, Set indexNames) { DbScope scope = validateDomain(domain); @@ -1060,6 +904,69 @@ public void dropTableIndices(Domain domain, Set indexNames) } } + @Override + public void ensureTableIndices(@NotNull Domain domain) + { + ensureTableIndices(domain, () -> true); + } + + @Override + public void ensureTableIndices(@NotNull Domain domain, Supplier afterAddSupplier) + { + if (!domain.isProvisioned()) + throw new IllegalStateException("Domain " + domain.getName() + " is not provisioned!"); + + // Issue 50059, acquiring the schema table info this way ensures that the domain fields are properly fixed up. See ProvisionedSchemaOptions. + SchemaTableInfo schemaTableInfo = StorageProvisioner.get().getSchemaTableInfo(domain); + if (schemaTableInfo != null) + { + DomainKind kind = domain.getDomainKind(); + if (null == kind) + throw new IllegalStateException("Domain kind of " + domain.getName() + " is null!"); + Map>> existingIndices = schemaTableInfo.getAllIndices(); + // Determine the desired indexes. Note that the index lists provided by Domain and DomainKind may overlap, + // so we need to uniquify. Domain indices never specify "clustered" but DomainKind indices may (e.g., + // DatasetDomainKind), so compare using only column names and give preference to DomainKind. + Set newIndices = new TreeSet<>(Comparator.comparing(index -> String.join("_", index.columnNames), String.CASE_INSENSITIVE_ORDER)); + newIndices.addAll(domain.getPropertyIndices()); + newIndices.addAll(kind.getPropertyIndices(domain)); + Set toRemove = new HashSet<>(); + for (String name : existingIndices.keySet()) + { + if (existingIndices.get(name).first == TableInfo.IndexType.Primary) + continue; + Pair> columnIndex = existingIndices.get(name); + String[] columnNames = new String[columnIndex.second.size()]; + for (int i = 0; i < columnIndex.second.size(); i++) + { + columnNames[i] = columnIndex.second.get(i).getColumnName(); + } + Index existingIndex = new Index(columnIndex.first == TableInfo.IndexType.Unique, columnNames); + boolean foundIt = false; + for (Index propertyIndex : newIndices) + { + if (Index.isSameIndex(propertyIndex, existingIndex)) + { + foundIt = true; + newIndices.remove(propertyIndex); + break; + } + } + + if (!foundIt) + toRemove.add(name); + } + + if (!toRemove.isEmpty()) + dropTableIndices(domain, toRemove); + + boolean successfulAdd = afterAddSupplier.get(); + + if (successfulAdd && !newIndices.isEmpty()) + addTableIndices(domain, newIndices, TableChange.IndexSizeMode.Normal); + } + } + private static DbScope getScope(Domain domain) { return getDomainKind(domain).getScope(); @@ -1828,8 +1735,8 @@ public Set getReservedPropertyNames(Domain domain, User user) { d = new DomainImpl(c, uri, "test", true) { - @Override @Nullable - public DomainKind getDomainKind() + @Override + public @NotNull DomainKind getDomainKind() { return k; } diff --git a/query/src/org/labkey/query/audit/QueryExportAuditProvider.java b/query/src/org/labkey/query/audit/QueryExportAuditProvider.java index f878af01139..3af2648722a 100644 --- a/query/src/org/labkey/query/audit/QueryExportAuditProvider.java +++ b/query/src/org/labkey/query/audit/QueryExportAuditProvider.java @@ -41,9 +41,6 @@ import java.util.Set; /** - * User: klum - * Date: 7/21/13 - * * UNDONE: Fancy QueryAuditViewFactory.QueryDetailsColumn */ public class QueryExportAuditProvider extends AbstractAuditTypeProvider implements AuditTypeProvider @@ -219,7 +216,7 @@ public Map getAuditLogMessageElements() public static class QueryExportAuditDomainKind extends AbstractAuditDomainKind { - public static final String NAME = "QueryAuditDomain"; + public static final String NAME = "QueryExportAuditDomain"; public static String NAMESPACE_PREFIX = "Audit-" + NAME; private final Set _fields; diff --git a/query/src/org/labkey/query/reports/ReportAuditProvider.java b/query/src/org/labkey/query/reports/ReportAuditProvider.java index 315c13f817d..762d6d4f5c5 100644 --- a/query/src/org/labkey/query/reports/ReportAuditProvider.java +++ b/query/src/org/labkey/query/reports/ReportAuditProvider.java @@ -25,11 +25,10 @@ public class ReportAuditProvider extends AbstractAuditTypeProvider private static final String COLUMN_NAME_REPORT_NAME = "ReportName"; private static final String COLUMN_NAME_REPORT_KEY = "ReportKey"; private static final String COLUMN_NAME_REPORT_TYPE = "ReportType"; + private static final List defaultVisibleColumns = new ArrayList<>(); - - static final List defaultVisibleColumns = new ArrayList<>(); - - static { + static + { defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CONTAINER)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED_BY)); @@ -41,10 +40,9 @@ public class ReportAuditProvider extends AbstractAuditTypeProvider defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); } - @Override - protected AbstractAuditDomainKind getDomainKind() + public ReportAuditProvider() { - return new ReportAuditDomainKind(); + super(new ReportAuditDomainKind()); } @Override diff --git a/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java b/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java index 6d5f631c06a..953beb49d20 100644 --- a/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java +++ b/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java @@ -38,11 +38,10 @@ public class ParticipantGroupAuditProvider extends AbstractAuditTypeProvider imp private static final String EVENT_NAME = "ParticipantGroupEvent"; private static final String PARTICIPANT_CATEGORY_ID_COLUMN_NAME = "ParticipantCategory"; private static final String PARTICIPANT_GROUP_ID_COLUMN_NAME = "ParticipantGroup"; + private static final List defaultVisibleColumns = new ArrayList<>(); - - static final List defaultVisibleColumns = new ArrayList<>(); - - static { + static + { defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CONTAINER)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED_BY)); @@ -53,6 +52,11 @@ public class ParticipantGroupAuditProvider extends AbstractAuditTypeProvider imp defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_DATA_CHANGES)); } + public ParticipantGroupAuditProvider() + { + super(new ParticipantGroupAuditDomainKind()); + } + @Override public String getEventName() { @@ -112,12 +116,6 @@ else if (PARTICIPANT_GROUP_ID_COLUMN_NAME.equalsIgnoreCase(col.getName())) return table; } - @Override - protected AbstractAuditDomainKind getDomainKind() - { - return new ParticipantGroupAuditDomainKind(); - } - public static class ParticipantGroupAuditEvent extends DetailedAuditTypeEvent { private Integer _participantCategory; diff --git a/study/src/org/labkey/study/model/DatasetDomainKind.java b/study/src/org/labkey/study/model/DatasetDomainKind.java index c67c068b278..7210771238e 100644 --- a/study/src/org/labkey/study/model/DatasetDomainKind.java +++ b/study/src/org/labkey/study/model/DatasetDomainKind.java @@ -520,7 +520,7 @@ else if (!timepointType.isVisitBased() && getKindName().equals(VisitDatasetDomai List indices = domain.getIndices(); newDomain.setPropertyIndices(indices, lowerReservedNames); - StorageProvisioner.get().addMissingRequiredIndices(newDomain); + StorageProvisioner.get().ensureTableIndices(newDomain); QueryService.get().saveCalculatedFieldsMetadata("study", name, null, domain.getCalculatedFields(), false, user, container); } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 78b07b00df0..d600d6aca94 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -102,6 +102,7 @@ import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainAuditProvider; import org.labkey.api.exp.property.DomainProperty; +import org.labkey.api.exp.property.DomainUtil; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.exp.property.SystemProperty; import org.labkey.api.gwt.client.AuditBehaviorType; @@ -177,6 +178,7 @@ import org.labkey.api.util.Pair; import org.labkey.api.util.Path; import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.NavTree; import org.labkey.api.view.UnauthorizedException; @@ -236,7 +238,7 @@ public class StudyManager public static final SearchService.SearchCategory datasetCategory = new SearchService.SearchCategory("dataset", "Study Datasets"); public static final SearchService.SearchCategory subjectCategory = new SearchService.SearchCategory("subject", "Study Subjects"); - private static final Logger _log = LogManager.getLogger(StudyManager.class); + private static final Logger _log = LogHelper.getLogger(StudyManager.class, "Dataset operations"); private static final StudyManager _instance = new StudyManager(); private static final StudySchema SCHEMA = StudySchema.getInstance(); private static final String LSID_REQUIRED = "LSID_REQUIRED"; @@ -3593,17 +3595,16 @@ else if (d.isModified) // optional param to control whether field additions or deletions are permitted if (allowDomainUpdates) { - // generate dataset domain changes for new datasets + // Generate dataset domain changes for new datasets domainChangeMap.clear(); buildPropertySaveAndDeleteLists(datasetDefEntryMap, list, domainChangeMap, false); - // now that we actually have datasets, create/update the domains - dropNotRequiredIndices(reader, datasetDefEntryMap, domainChangeMap); + // Now that we actually have datasets, create or update the domains. This ensures that all domains and + // property changes are saved before adding indices. + ensurePropertiesAndRequiredIndices(reader, datasetDefEntryMap, domainChangeMap, user, errors); - if (!deleteAndSaveProperties(user, errors, domainChangeMap)) + if (errors.hasErrors()) return false; - - addMissingRequiredIndices(reader, datasetDefEntryMap, domainChangeMap); } List orderedIds = reader.getDatasetOrder(); @@ -3616,25 +3617,23 @@ else if (d.isModified) return true; } - private boolean deleteAndSaveProperties(User user, BindException errors, Map domainChangeMap) + // see if we need to delete any columns from an existing domain and create the domain if it doesn't already exist + private boolean deleteAndSaveProperties(User user, BindException errors, _DatasetDomainChange domainChange) { - // see if we need to delete any columns from an existing domain - for (_DatasetDomainChange domainChange : domainChangeMap.values()) + // see if we need to delete any columns from the domain + for (DomainProperty p : domainChange.propsToDelete) { - for (DomainProperty p : domainChange.propsToDelete) - { - p.delete(); - } + p.delete(); + } - try - { - domainChange.domain.save(user); - } - catch (ChangePropertyDescriptorException ex) - { - errors.reject("importDatasetSchemas", ex.getMessage() == null ? ex.toString() : ex.getMessage()); - return false; - } + try + { + domainChange.domain.save(user); + } + catch (ChangePropertyDescriptorException ex) + { + errors.reject("importDatasetSchemas", ex.getMessage() == null ? ex.toString() : ex.getMessage()); + return false; } return true; } @@ -3764,7 +3763,7 @@ private void buildPropertySaveAndDeleteLists(Map } } - private void addMissingRequiredIndices(SchemaReader reader, Map datasetDefEntryMap, Map domainChangeMap) + private void ensurePropertiesAndRequiredIndices(SchemaReader reader, Map datasetDefEntryMap, Map domainChangeMap, User user, BindException errors) { for (SchemaReader.DatasetImportInfo datasetImportInfo : reader.getDatasetInfo().values()) { @@ -3773,28 +3772,25 @@ private void addMissingRequiredIndices(SchemaReader reader, Map datasetDefEntryMap, Map domainChangeMap) - { - for (SchemaReader.DatasetImportInfo datasetImportInfo : reader.getDatasetInfo().values()) - { - DatasetDefinitionEntry datasetDefinitionEntry = datasetDefEntryMap.get(datasetImportInfo.name); - if (datasetDefinitionEntry.datasetDefinition.isShared()) + if (domain.isProvisioned()) { - continue; + // If we're changing an existing domain, we may be dropping a column that has an admin-configured index. + // We need to drop the indices first, adjust the properties, and then add the new indices. This method + // allows for that. + StorageProvisioner.get().ensureTableIndices(domain, () -> deleteAndSaveProperties(user, errors, domainChange)); + } + else + { + // deleteAndSaveProperties() calls domain.save() which ensures all the indices after provisioning the new domain + deleteAndSaveProperties(user, errors, domainChange); } - Domain domain = domainChangeMap.get(datasetDefinitionEntry.datasetDefinition.getTypeURI()).domain; - domain.setPropertyIndices(datasetImportInfo.indices); - StorageProvisioner.get().dropNotRequiredIndices(domain); } } - public String getDomainURI(Container c, User u, Dataset def) { if (null == def) diff --git a/wiki/src/org/labkey/wiki/WikiManager.java b/wiki/src/org/labkey/wiki/WikiManager.java index c216d00f8fe..8a20fc586cc 100644 --- a/wiki/src/org/labkey/wiki/WikiManager.java +++ b/wiki/src/org/labkey/wiki/WikiManager.java @@ -900,7 +900,7 @@ public void insertWiki(User user, Container c, String name, String body, WikiRen } @Override - public WebPartView getView(Container c, String name, boolean contentOnly) + public WebPartView getView(Container c, String name, boolean contentOnly) { try { @@ -913,8 +913,7 @@ public WebPartView getView(Container c, String name, boolean contentOnly) if (null == wiki) return null; WikiVersion version = wiki.getLatestVersion(); - WikiView view = new WikiView(wiki, version, true); - return view; + return new WikiView(wiki, version, true); } catch (Exception x) { @@ -923,14 +922,13 @@ public WebPartView getView(Container c, String name, boolean contentOnly) } @Override - public WebPartView getHistoryView(Container c, String name) + public WikiVersionsGrid getHistoryView(Container c, String name) { Wiki wiki = WikiSelectManager.getWiki(c, name); if (null == wiki) return null; WikiVersion version = wiki.getLatestVersion(); - WikiVersionsGrid view = new WikiVersionsGrid(wiki, version, null); - return view; + return new WikiVersionsGrid(wiki, version, null); } @Override