[Improvement] Use template to reduce Privileges duplicate codes

[xunliu]

What would you like to be improved?

Currently Privileges.java have more than one thousand duplicate code to instantiation CreateCatalog, AlterCatalog, ...

public abstract static class CreateCatalog implements Privilege {

    private static final CreateCatalog ALLOW_INSTANCE =
        new CreateCatalog() {
          @Override
          public Condition condition() {
            return Condition.ALLOW;
          }
        };

    private static final CreateCatalog DENY_INSTANCE =
        new CreateCatalog() {
          @Override
          public Condition condition() {
            return Condition.DENY;
          }
        };

    private CreateCatalog() {}

    /** @return The instance with allow condition of the privilege. */
    public static CreateCatalog allow() {
      return ALLOW_INSTANCE;
    }

    /** @return The instance with deny condition of the privilege. */
    public static CreateCatalog deny() {
      return DENY_INSTANCE;
    }

    /** @return The generic name of the privilege. */
    @Override
    public Name name() {
      return CREATE_CATALOG;
    }

    /** @return A readable string representation for the privilege. */
    @Override
    public String simpleString() {
      return condition().name() + " create catalog";
    }
  }

How should we improve?

I think we can use the template to reduce these duplicate codes.

GenericPrivilege.java Template code just like there:

public abstract class GenericPrivilege<T extends GenericPrivilege<T>> implements Privilege {
  @FunctionalInterface
  public interface GenericPrivilegeFactory<T extends GenericPrivilege<T>> {
    T create(Privilege.Condition condition, Privilege.Name name);
  }

  private final Condition condition;
  private final Name name;

  protected GenericPrivilege(Condition condition, Name name) {
    this.condition = condition;
    this.name = name;
  }

  /** @return The instance with allow condition of the privilege. */
  public static <T extends GenericPrivilege<T>> T allow(
      Name name, GenericPrivilegeFactory<T> factory) {
    return factory.create(Condition.ALLOW, name);
  }

  /** @return The instance with deny condition of the privilege. */
  public static <T extends GenericPrivilege<T>> T deny(
      Name name, GenericPrivilegeFactory<T> factory) {
    return factory.create(Condition.DENY, name);
  }

  /** @return The generic name of the privilege. */
  @Override
  public Name name() {
    return name;
  }

  /** @return The condition of the privilege. */
  @Override
  public Condition condition() {
    return condition;
  }

  /** @return A readable string representation for the privilege. */
  @Override
  public String simpleString() {
    return condition.name() + " " + name.name().toLowerCase().replace('_', ' ');
  }
}

We can use GenericPrivilege.java to instantiation CreateCatalog, AlterCatalog, ...

public static class CreateCatalog extends GenericPrivilege<CreateCatalog> {
    private CreateCatalog(Condition condition, Name name) {
      super(condition, name);
    }

    /** @return The instance with allow condition of the privilege. */
    public static CreateCatalog allow() {
      return GenericPrivilege.allow(Name.CREATE_CATALOG, CreateCatalog::new);
    }

    /** @return The instance with deny condition of the privilege. */
    public static CreateCatalog deny() {
      return GenericPrivilege.deny(Name.CREATE_CATALOG, CreateCatalog::new);
    }
  }

[rich7420]

I want to have a try. Can you assign to me? Thanks a lot.

[xunliu]

Hi @rich7420 Thank you for your interesting in this issue, I assign it to you, If you have any problem, please add comments to this issue. :-)

[rich7420]

@xunliu thank you very much!

[rich7420]

@xunliu I have a PR in #4010 . But actually, when I trying to run ./gradlew build in local, this error came out. I think is because there are different numbers before <SecurableObject: [fullName=metalake], [type=METALAKE], [privileges=[ALLOW use metalake]]>. How should I resolve this issue?

[rich7420]

Is it possible to modify the content in the test to use the actual values of the test instead of the memory addresses?