Remove obsolete row filter and column mask methods

The current deprecation policy, introduced around the time these methods were deprecated, is that we promise to keep deprecated methods for one more release. According to this, removal is long overdue.
trinodb · Jun 28, 2022 · ac8d2d4 · ac8d2d4
1 parent d809130
commit ac8d2d4
Show file tree

Hide file tree

Showing 12 changed files with 9 additions and 133 deletions.
diff --git a/core/trino-main/src/test/java/io/trino/security/TestInjectedConnectorAccessControl.java b/core/trino-main/src/test/java/io/trino/security/TestInjectedConnectorAccessControl.java
@@ -13,11 +13,7 @@
  */
 package io.trino.security;
 
-import com.google.common.collect.ImmutableSet;
 import io.trino.spi.connector.ConnectorAccessControl;
-import io.trino.spi.connector.ConnectorSecurityContext;
-import io.trino.spi.connector.SchemaTableName;
-import io.trino.spi.type.Type;
 import org.testng.annotations.Test;
 
 import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
@@ -28,8 +24,6 @@ public class TestInjectedConnectorAccessControl
     public void testEverythingImplemented()
             throws NoSuchMethodException
     {
-        assertAllMethodsOverridden(ConnectorAccessControl.class, InjectedConnectorAccessControl.class, ImmutableSet.of(
-                InjectedConnectorAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class),
-                InjectedConnectorAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class)));
+        assertAllMethodsOverridden(ConnectorAccessControl.class, InjectedConnectorAccessControl.class);
     }
 }
diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java
@@ -608,20 +608,6 @@ default void checkCanExecuteFunction(ConnectorSecurityContext context, FunctionK
         denyExecuteFunction(function.toString());
     }
 
-    /**
-     * Get a row filter associated with the given table and identity.
-     * <p>
-     * The filter must be a scalar SQL expression of boolean type over the columns in the table.
-     *
-     * @return the filter, or {@link Optional#empty()} if not applicable
-     * @deprecated use {@link #getRowFilters(ConnectorSecurityContext, SchemaTableName)} instead
-     */
-    @Deprecated
-    default Optional<ViewExpression> getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName)
-    {
-        return Optional.empty();
-    }
-
     /**
      * Get row filters associated with the given table and identity.
      * <p>
@@ -634,21 +620,6 @@ default List<ViewExpression> getRowFilters(ConnectorSecurityContext context, Sch
         return emptyList();
     }
 
-    /**
-     * Get a column mask associated with the given table, column and identity.
-     * <p>
-     * The mask must be a scalar SQL expression of a type coercible to the type of the column being masked. The expression
-     * must be written in terms of columns in the table.
-     *
-     * @return the mask, or {@link Optional#empty()} if not applicable
-     * @deprecated use {@link #getColumnMasks(ConnectorSecurityContext, SchemaTableName, String, Type)} instead
-     */
-    @Deprecated
-    default Optional<ViewExpression> getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type)
-    {
-        return Optional.empty();
-    }
-
     /**
      * Get column masks associated with the given table, column and identity.
      * <p>

diff --git a/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/security/SystemAccessControl.java
@@ -829,20 +829,6 @@ default void checkCanExecuteTableProcedure(SystemSecurityContext systemSecurityC
         denyExecuteTableProcedure(table.toString(), procedure);
     }
 
-    /**
-     * Get a row filter associated with the given table and identity.
-     * <p>
-     * The filter must be a scalar SQL expression of boolean type over the columns in the table.
-     *
-     * @return the filter, or {@link Optional#empty()} if not applicable
-     * @deprecated use {@link #getRowFilters(SystemSecurityContext, CatalogSchemaTableName)} instead
-     */
-    @Deprecated
-    default Optional<ViewExpression> getRowFilter(SystemSecurityContext context, CatalogSchemaTableName tableName)
-    {
-        return Optional.empty();
-    }
-
     /**
      * Get row filters associated with the given table and identity.
      * <p>
@@ -852,22 +838,7 @@ default Optional<ViewExpression> getRowFilter(SystemSecurityContext context, Cat
      */
     default List<ViewExpression> getRowFilters(SystemSecurityContext context, CatalogSchemaTableName tableName)
     {
-        return getRowFilter(context, tableName).map(List::of).orElseGet(List::of);
-    }
-
-    /**
-     * Get a column mask associated with the given table, column and identity.
-     * <p>
-     * The mask must be a scalar SQL expression of a type coercible to the type of the column being masked. The expression
-     * must be written in terms of columns in the table.
-     *
-     * @return the mask, or {@link Optional#empty()} if not applicable
-     * @deprecated use {@link #getColumnMasks(SystemSecurityContext, CatalogSchemaTableName, String, Type)} instead
-     */
-    @Deprecated
-    default Optional<ViewExpression> getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type)
-    {
-        return Optional.empty();
+        return List.of();
     }
 
     /**
@@ -880,7 +851,7 @@ default Optional<ViewExpression> getColumnMask(SystemSecurityContext context, Ca
      */
     default List<ViewExpression> getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type)
     {
-        return getColumnMask(context, tableName, columnName, type).map(List::of).orElseGet(List::of);
+        return List.of();
     }
 
     /**

diff --git a/...toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java b/...toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java
@@ -393,24 +393,12 @@ public void checkCanExecuteFunction(ConnectorSecurityContext context, FunctionKi
         delegate().checkCanExecuteFunction(context, functionKind, function);
     }
 
-    @Override
-    public Optional<ViewExpression> getRowFilter(ConnectorSecurityContext context, SchemaTableName tableName)
-    {
-        return delegate().getRowFilter(context, tableName);
-    }
-
     @Override
     public List<ViewExpression> getRowFilters(ConnectorSecurityContext context, SchemaTableName tableName)
     {
         return delegate().getRowFilters(context, tableName);
     }
 
-    @Override
-    public Optional<ViewExpression> getColumnMask(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type)
-    {
-        return delegate().getColumnMask(context, tableName, columnName, type);
-    }
-
     @Override
     public List<ViewExpression> getColumnMasks(ConnectorSecurityContext context, SchemaTableName tableName, String columnName, Type type)
     {

diff --git a/...in-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java b/...in-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingSystemAccessControl.java
@@ -487,24 +487,12 @@ public Iterable<EventListener> getEventListeners()
         return delegate().getEventListeners();
     }
 
-    @Override
-    public Optional<ViewExpression> getRowFilter(SystemSecurityContext context, CatalogSchemaTableName tableName)
-    {
-        return delegate().getRowFilter(context, tableName);
-    }
-
     @Override
     public List<ViewExpression> getRowFilters(SystemSecurityContext context, CatalogSchemaTableName tableName)
     {
         return delegate().getRowFilters(context, tableName);
     }
 
-    @Override
-    public Optional<ViewExpression> getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type)
-    {
-        return delegate().getColumnMask(context, tableName, columnName, type);
-    }
-
     @Override
     public List<ViewExpression> getColumnMasks(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type)
     {

diff --git a/...n-toolkit/src/test/java/io/trino/plugin/base/classloader/TestClassLoaderSafeWrappers.java b/...n-toolkit/src/test/java/io/trino/plugin/base/classloader/TestClassLoaderSafeWrappers.java
@@ -13,23 +13,19 @@
  */
 package io.trino.plugin.base.classloader;
 
-import com.google.common.collect.ImmutableSet;
 import io.trino.spi.connector.ConnectorAccessControl;
 import io.trino.spi.connector.ConnectorMetadata;
 import io.trino.spi.connector.ConnectorNodePartitioningProvider;
 import io.trino.spi.connector.ConnectorPageSink;
 import io.trino.spi.connector.ConnectorPageSinkProvider;
 import io.trino.spi.connector.ConnectorPageSourceProvider;
 import io.trino.spi.connector.ConnectorRecordSetProvider;
-import io.trino.spi.connector.ConnectorSecurityContext;
 import io.trino.spi.connector.ConnectorSplitManager;
 import io.trino.spi.connector.ConnectorSplitSource;
 import io.trino.spi.connector.RecordSet;
-import io.trino.spi.connector.SchemaTableName;
 import io.trino.spi.connector.SystemTable;
 import io.trino.spi.eventlistener.EventListener;
 import io.trino.spi.ptf.ConnectorTableFunction;
-import io.trino.spi.type.Type;
 import org.testng.annotations.Test;
 
 import java.lang.reflect.Method;
@@ -47,9 +43,7 @@ public class TestClassLoaderSafeWrappers
     public void test()
             throws Exception
     {
-        testClassLoaderSafe(ConnectorAccessControl.class, ClassLoaderSafeConnectorAccessControl.class, ImmutableSet.of(
-                ClassLoaderSafeConnectorAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class),
-                ClassLoaderSafeConnectorAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class)));
+        testClassLoaderSafe(ConnectorAccessControl.class, ClassLoaderSafeConnectorAccessControl.class);
         testClassLoaderSafe(ConnectorMetadata.class, ClassLoaderSafeConnectorMetadata.class);
         testClassLoaderSafe(ConnectorPageSink.class, ClassLoaderSafeConnectorPageSink.class);
         testClassLoaderSafe(ConnectorPageSinkProvider.class, ClassLoaderSafeConnectorPageSinkProvider.class);

diff --git a/...plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllAccessControl.java b/...plugin-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllAccessControl.java
@@ -13,11 +13,7 @@
  */
 package io.trino.plugin.base.security;
 
-import com.google.common.collect.ImmutableSet;
 import io.trino.spi.connector.ConnectorAccessControl;
-import io.trino.spi.connector.ConnectorSecurityContext;
-import io.trino.spi.connector.SchemaTableName;
-import io.trino.spi.type.Type;
 import org.testng.annotations.Test;
 
 import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
@@ -28,8 +24,6 @@ public class TestAllowAllAccessControl
     public void testEverythingImplemented()
             throws NoSuchMethodException
     {
-        assertAllMethodsOverridden(ConnectorAccessControl.class, AllowAllAccessControl.class, ImmutableSet.of(
-                AllowAllAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class),
-                AllowAllAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class)));
+        assertAllMethodsOverridden(ConnectorAccessControl.class, AllowAllAccessControl.class);
     }
 }
diff --git a/...-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllSystemAccessControl.java b/...-toolkit/src/test/java/io/trino/plugin/base/security/TestAllowAllSystemAccessControl.java
@@ -13,11 +13,7 @@
  */
 package io.trino.plugin.base.security;
 
-import com.google.common.collect.ImmutableSet;
-import io.trino.spi.connector.CatalogSchemaTableName;
 import io.trino.spi.security.SystemAccessControl;
-import io.trino.spi.security.SystemSecurityContext;
-import io.trino.spi.type.Type;
 import org.testng.annotations.Test;
 
 import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
@@ -28,8 +24,6 @@ public class TestAllowAllSystemAccessControl
     public void testEverythingImplemented()
             throws ReflectiveOperationException
     {
-        assertAllMethodsOverridden(SystemAccessControl.class, AllowAllSystemAccessControl.class, ImmutableSet.of(
-                AllowAllSystemAccessControl.class.getMethod("getRowFilter", SystemSecurityContext.class, CatalogSchemaTableName.class),
-                AllowAllSystemAccessControl.class.getMethod("getColumnMask", SystemSecurityContext.class, CatalogSchemaTableName.class, String.class, Type.class)));
+        assertAllMethodsOverridden(SystemAccessControl.class, AllowAllSystemAccessControl.class);
     }
 }
diff --git a/...lugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java b/...lugin-toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedAccessControl.java
@@ -28,7 +28,6 @@
 import io.trino.spi.security.Privilege;
 import io.trino.spi.security.TrinoPrincipal;
 import io.trino.spi.security.ViewExpression;
-import io.trino.spi.type.Type;
 import org.testng.Assert.ThrowingRunnable;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -538,9 +537,7 @@ public void testSchemaRulesForCheckCanShowTables()
     public void testEverythingImplemented()
             throws NoSuchMethodException
     {
-        assertAllMethodsOverridden(ConnectorAccessControl.class, FileBasedAccessControl.class, ImmutableSet.of(
-                FileBasedAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class),
-                FileBasedAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class)));
+        assertAllMethodsOverridden(ConnectorAccessControl.class, FileBasedAccessControl.class);
     }
 
     private static ConnectorSecurityContext user(String name, Set<String> groups)

diff --git a/...toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java b/...toolkit/src/test/java/io/trino/plugin/base/security/TestFileBasedSystemAccessControl.java
@@ -29,7 +29,6 @@
 import io.trino.spi.security.SystemSecurityContext;
 import io.trino.spi.security.TrinoPrincipal;
 import io.trino.spi.security.ViewExpression;
-import io.trino.spi.type.Type;
 import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -1370,8 +1369,6 @@ public void testEverythingImplemented()
             throws NoSuchMethodException
     {
         assertAllMethodsOverridden(SystemAccessControl.class, FileBasedSystemAccessControl.class, ImmutableSet.of(
-                FileBasedSystemAccessControl.class.getMethod("getRowFilter", SystemSecurityContext.class, CatalogSchemaTableName.class),
-                FileBasedSystemAccessControl.class.getMethod("getColumnMask", SystemSecurityContext.class, CatalogSchemaTableName.class, String.class, Type.class),
                 FileBasedSystemAccessControl.class.getMethod("checkCanViewQueryOwnedBy", SystemSecurityContext.class, Identity.class),
                 FileBasedSystemAccessControl.class.getMethod("filterViewQueryOwnedBy", SystemSecurityContext.class, Collection.class),
                 FileBasedSystemAccessControl.class.getMethod("checkCanKillQueryOwnedBy", SystemSecurityContext.class, Identity.class)));

diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestLegacyAccessControl.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/security/TestLegacyAccessControl.java
@@ -13,11 +13,7 @@
  */
 package io.trino.plugin.hive.security;
 
-import com.google.common.collect.ImmutableSet;
 import io.trino.spi.connector.ConnectorAccessControl;
-import io.trino.spi.connector.ConnectorSecurityContext;
-import io.trino.spi.connector.SchemaTableName;
-import io.trino.spi.type.Type;
 import org.testng.annotations.Test;
 
 import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
@@ -28,8 +24,6 @@ public class TestLegacyAccessControl
     public void testEverythingImplemented()
             throws NoSuchMethodException
     {
-        assertAllMethodsOverridden(ConnectorAccessControl.class, LegacyAccessControl.class, ImmutableSet.of(
-                LegacyAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class),
-                LegacyAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class)));
+        assertAllMethodsOverridden(ConnectorAccessControl.class, LegacyAccessControl.class);
     }
 }
diff --git a/.../trino-hive/src/test/java/io/trino/plugin/hive/security/TestSqlStandardAccessControl.java b/.../trino-hive/src/test/java/io/trino/plugin/hive/security/TestSqlStandardAccessControl.java
@@ -13,11 +13,7 @@
  */
 package io.trino.plugin.hive.security;
 
-import com.google.common.collect.ImmutableSet;
 import io.trino.spi.connector.ConnectorAccessControl;
-import io.trino.spi.connector.ConnectorSecurityContext;
-import io.trino.spi.connector.SchemaTableName;
-import io.trino.spi.type.Type;
 import org.testng.annotations.Test;
 
 import static io.trino.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
@@ -28,8 +24,6 @@ public class TestSqlStandardAccessControl
     public void testEverythingImplemented()
             throws NoSuchMethodException
     {
-        assertAllMethodsOverridden(ConnectorAccessControl.class, SqlStandardAccessControl.class, ImmutableSet.of(
-                SqlStandardAccessControl.class.getMethod("getRowFilter", ConnectorSecurityContext.class, SchemaTableName.class),
-                SqlStandardAccessControl.class.getMethod("getColumnMask", ConnectorSecurityContext.class, SchemaTableName.class, String.class, Type.class)));
+        assertAllMethodsOverridden(ConnectorAccessControl.class, SqlStandardAccessControl.class);
     }
 }