Два разных подхода к разделению полей для предложения WHERE - PullRequest
0 голосов
/ 20 июня 2019

Я разместил более подробную версию этого вопроса на странице обмена отзывами, проверьте эту ссылку .Позже один из моих коллег рассмотрел мою реализацию, и он предложил другой подход к решению проблемы.Таким образом, этот вопрос должен помочь мне выяснить, какой из двух подходов следует выбрать и почему?

Если это не подходит для StackOverflow и все еще должно быть в CodeReviewExchange, пожалуйста, дайте мне знать, и я переместу его туда.

Также я хочу сказать это в самом начале, потому что я былкритиковали за это в моем предыдущем посте.Фактический код отличается от того, что я предоставил здесь, он более сложный, поэтому мне пришлось упростить его, чтобы спросить, что именно я хочу выяснить, то есть наследование против композиции, что здесь лучше?

Вот мой набор классов:

public abstract class ConditionBuilder<TContext> : IConditionBuilder where TContext : FieldSearchContext
{
    public virtual bool QuotedValues { get; set; } = true;

    public abstract string OperatorSymbol { get; }

    public string BuildCondition(SearchCondition searchCondition)
    {
        var conditionBuilder = new StringBuilder();

        var context = searchCondition.GetContext<TContext>();

        conditionBuilder.Append(context.FieldId);
        conditionBuilder.Append(OperatorSymbol);
        conditionBuilder.Append(GetValue(context));

        return conditionBuilder.ToString();
    }

    public abstract bool CanHandle(FilterAction filterAction);

    public abstract object GetValue(TContext context);

}

public class TextLikeConditionBuilder : ConditionBuilder<TextContext>
{
    public override string OperatorSymbol => " LIKE ";

    public override bool CanHandle(FilterAction action) => action == FilterAction.TextLike;

    public override object GetValue(TextContext context)
    {
        if (context.Text == null)
        {
            return null;
        }

        return string.Concat("%", context.Text, "%");
    }
}

public class TextEqualsConditionBuilder : ConditionBuilder<TextContext>
{
    public override string OperatorSymbol => " = ";

    public override bool CanHandle(FilterAction action) => action == FilterAction.TextEqual;

    public override object GetValue(TextContext context)
    {
        return context.Text;
    }
}

В своей реализации по умолчанию классы создают предложение WHERE для SQL.Эти классы используются другими разработчиками и могут быть переопределены для создания условия WHERE, которое не обязательно предназначено для базы данных. Например, можно переопределить protected virtual StringBuilder GetCondition(SearchFilterCondition filterCondition, TContext context) для создания QUERY для Elasticsearch также.

Возвращаясь к теме, в одном конкретном случае, когда эти классы используются с их поведением по умолчанию, я хотел разграничить FieldId, чтобы убедиться, что он работает, когда FieldId содержит пробелы и специальные символы.Вот как я реализовал решение:

public interface IDelimitedIdentifier
{
    string Delimit(string input);
}

internal class SqlServerDelimitedIdentifier : IDelimitedIdentifier
{
    public string Delimit(string input)
    {
        return "[" + input.Replace("]", "]]") + "]";
    }
}

internal class OracleDelimitedIdentifier : IDelimitedIdentifier
{
    public string Delimit(string input)
    {
        return "\"" + input + "\"";
    }
}

public abstract class ConditionBuilder<TContext> : IConditionBuilder where TContext : FieldSearchContext
{
    public virtual bool QuotedValues { get; set; } = true;

    public abstract string OperatorSymbol { get; }

    public string BuildCondition(SearchCondition searchCondition)
    {
        var conditionBuilder = new StringBuilder();

        var context = searchCondition.GetContext<TContext>();

        conditionBuilder.Append(SanitizeFieldId(context.FieldId));
        conditionBuilder.Append(OperatorSymbol);
        conditionBuilder.Append(GetValue(context));

        return conditionBuilder.ToString();
    }

    public abstract bool CanHandle(FilterAction filterAction);

    public abstract object GetValue(TContext context);

    protected virtual string SanitizeFieldId(string fieldId)
    {
        return _delimitedIdentifier.Delimit(fieldId);
    }

}

public class SanitizedFieldConditionBuilder<TContext> : ConditionBuilder<TContext> where TContext : FieldSearchContextBase
{
    private readonly ConditionBuilder<TContext> _baseConditionBuilder;
    private readonly IDelimitedIdentifier _delimitedIdentifier;

    public SanitizedFieldConditionBuilder(ConditionBuilder<TContext> baseConditionBuilder, IDelimitedIdentifier delimitedIdentifier)
    {
        QuotedValues = false;

        _baseConditionBuilder = baseConditionBuilder;
        _delimitedIdentifier = delimitedIdentifier;
    }

    public override string OperatorSymbol => _baseConditionBuilder.OperatorSymbol;

    public override bool CanHandle(SearchFilterAction action) => _baseConditionBuilder.CanHandle(action);

    public override object GetValue(TContext context) => _baseConditionBuilder.GetValue(context);

    protected override string SanitizeFieldId(string fieldId)
    {
        return _delimitedIdentifier.Delimit(fieldId);
    }
}

public static class ConditionBuilderExtensions
{
    public static SanitizedFieldConditionBuiler<TContext> SanitizeField<TContext>(this ConditionBuilder<TContext> source, IColumnSanitizer columnSanitizer) where TContext : FieldSearchContext
    {
        return new SanitizedFieldConditionBuiler<TContext>(source, columnSanitizer);
    }

    public static ParameterizedConditionBuilder<TContext> WithParameters<TContext>(this ConditionBuilder<TContext> source,
        ParameterCollection parameterCollection, bool parameterizeNullValues = false) where TContext : FieldSearchContext
    {
        return new ParameterizedConditionBuilder<TContext>(source, parameterCollection, parameterizeNullValues);
    }
}

Классы могут быть созданы, как показано ниже:

class Program
{
  static void Main(string[] args)
  {
   var conditionBuilders = new List<IConditionBuilder>()
        {
            new TextEqualsConditionBuilder().SanitizeField(new SqlServerDelimitedIdentifier()),
            new TextLikeConditionBuilder().SanitizeField(new SqlServerDelimitedIdentifier())
        };
        }
}

Мой коллега предложил использовать композицию вместо расширения.Вот как классы должны выглядеть согласно его рекомендации.

 public abstract class ConditionBuilder<TContext> : IConditionBuilder where TContext : FieldSearchContext
    {
        private readonly IDelimitedIdentifier DelimitedIdentifier;

        public virtual bool QuotedValues { get; set; } = true;

        public abstract string OperatorSymbol { get; }

        protected ConditionBuilder(IDelimitedIdentifier delimitedIdentifier)
        {
            DelimitedIdentifier = delimitedIdentifier;
        }

        public string BuildCondition(SearchCondition searchCondition)
        {
            var conditionBuilder = new StringBuilder();

            var context = searchCondition.GetContext<TContext>();

            conditionBuilder.Append(DelimitedIdentifier.Delimit(context.FieldId));
            conditionBuilder.Append(OperatorSymbol);
            conditionBuilder.Append(GetValue(context));

            return conditionBuilder.ToString();
        }

        public abstract bool CanHandle(FilterAction filterAction);

        public abstract object GetValue(TContext context);

    }

    public class TextLikeConditionBuilder : ConditionBuilder<TextContext>
    {

        public TextLikeConditionBuilder(IDelimitedIdentifier delimitedIdentifier) : base(delimitedIdentifier)
        {

        }

        public override string OperatorSymbol => " LIKE ";

        public override bool CanHandle(FilterAction action) => action == FilterAction.TextLike;

        public override object GetValue(TextContext context)
        {
            if (context.Text == null)
            {
                return null;
            }

            return string.Concat("%", context.Text, "%");
        }
    }

    public class TextEqualsConditionBuilder : ConditionBuilder<TextContext>
    {

        public TextEqualsConditionBuilder(IDelimitedIdentifier delimitedIdentifier) : base(delimitedIdentifier)
        {

        }

        public override string OperatorSymbol => " = ";

        public override bool CanHandle(FilterAction action) => action == FilterAction.TextEqual;

        public override object GetValue(TextContext context)
        {
            if (context.Text == null)
            {
                return null;
            }

            return context.Text;
        }
    }

Вот мои аргументы, почему это не должно быть сделано в соответствии с рекомендациями моего коллеги:

  1. IDelimitedIdentifier очень специфично для базы данных, почему это должно бытьперемещаться внутри базового класса, когда функциональность может поддерживаться с помощью расширения.
  2. Расширение уменьшает количество изменений, в противном случае добавление этого конструктора будет означать изменение всех производных классов, и позвольте мне сказать вам, что существует 15 странных производных классов
  3. Разве добавление конструктора не противоречит принципу Open-Closed?

РЕДАКТИРОВАТЬ: так как многие люди обеспокоены проблемой инъекции SQL здесь, вместо того, чтобы отвечать на мои вопросы, я добавляю этот дополнительный код, который позаботится об этом.Я также обновил класс ConditionBuilderExtensions, определенный выше.

public class ParameterizedConditionBuilder<TContext> : ConditionBuilder<TContext>
        where TContext : FieldSearchContext
    {
        private readonly ConditionBuilder<TContext> baseConditionBuilder;
        private readonly ParameterCollection parameterCollection;
        private readonly bool parameterizeNullValues;

        public override bool QuotedValues { get; set; } = false;

        public ParameterizedConditionBuilder(ConditionBuilder<TContext> baseConditionBuilder,
            ParameterCollection parameterCollection, bool parameterizeNullValues = false)
        {
            this.baseConditionBuilder = baseConditionBuilder;
            this.parameterCollection = parameterCollection;
            this.parameterizeNullValues = parameterizeNullValues;
        }

        public override bool CanHandle(FilterAction action) => baseConditionBuilder.CanHandle(action);
        public override string OperatorSymbol => baseConditionBuilder.OperatorSymbol;

        public override object GetValue(TContext context)
        {
            object val = baseConditionBuilder.GetValue(context);
            if (val == null && !parameterizeNullValues)
            {
                return null;
            }

            string p = parameterCollection.AddParameter(val);
            return p;
        }
    }

Вот как я строю свои условия и затем использую их в DBCommand.Он невосприимчив к SQL-инъекциям

private static DbCommand CreateDbCommand(Database database, MergeOperation mergeOperation, IList<SearchCondition> searchCondition)
        {
            var whereConditions = new List<string>();

            ParameterCollection paramCollection = new ParameterCollection("{{{0}}}");

            var conditionBuilders = new List<IConditionBuilder>()
            {
                new TextEqualsConditionBuilder().WithParameters(paramCollection).SanitizeField(new SqlSanitizer()),
                new TextLikeConditionBuilder().WithParameters(paramCollection).SanitizeField(new SqlSanitizer())
            };

            foreach (var condition in searchCondition)
            {
                var context = condition.GetContext<FieldSearchContext>();
                var conditionBuilder = conditionBuilders.FirstOrDefault(u => u.CanHandle(condition.FilterAction));
                whereConditions.Add(conditionBuilder.BuildCondition(condition));
            }

            SqlBuilder sqlBuilder = new SqlBuilder();

            sqlBuilder.SELECT("Id");
            sqlBuilder.FROM("Students");
            if (whereConditions.Count > 0)
            {
                sqlBuilder.WHERE(string.Join(Convert.ToString(" " + mergeOperation + " "), whereConditions), paramCollection.GetParameters().Select(pair => pair.Value).ToArray());
            }

            DbExtensions.Database sqlBuilderDb = new DbExtensions.Database(database.CreateConnection());
            IDbCommand sqlBuilderCommand = sqlBuilderDb.CreateCommand(sqlBuilder);

            DbCommand databaseCommand = database.GetSqlStringCommand(sqlBuilderCommand.CommandText);
            if (sqlBuilderCommand.Parameters != null)
            {
                foreach (IDataParameter dataParameter in sqlBuilderCommand.Parameters)
                {
                    database.AddInParameter(databaseCommand, dataParameter.ParameterName, dataParameter.DbType, dataParameter.Value);
                }
            }

            return databaseCommand;
        }

SqlBuilder из пакета nuget DBExtensions.Я использую Microsoft.Enterprise.Library.Data для доступа к базе данных.

РЕДАКТИРОВАТЬ 2: Удален код, который был подвергнут SQL-инъекции

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...