Я разместил более подробную версию этого вопроса на странице обмена отзывами, проверьте эту ссылку .Позже один из моих коллег рассмотрел мою реализацию, и он предложил другой подход к решению проблемы.Таким образом, этот вопрос должен помочь мне выяснить, какой из двух подходов следует выбрать и почему?
Если это не подходит для 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;
}
}
Вот мои аргументы, почему это не должно быть сделано в соответствии с рекомендациями моего коллеги:
IDelimitedIdentifier
очень специфично для базы данных, почему это должно бытьперемещаться внутри базового класса, когда функциональность может поддерживаться с помощью расширения.- Расширение уменьшает количество изменений, в противном случае добавление этого конструктора будет означать изменение всех производных классов, и позвольте мне сказать вам, что существует 15 странных производных классов
- Разве добавление конструктора не противоречит принципу 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-инъекции