search

iskkonekb / kuvera

Система управленческого учета деятельности Ятры
MIT License
2 stars 0 forks source link

Провести рефакторинг сущности IEngine/Engine с преобразованием его в классический обработчик запросов #6

Open comdiv opened 7 years ago

comdiv commented 7 years ago

Эта задача - комплексная, состоит из многих меньших ссылающихся на эту. В github нет прямой возможности это делать, поэтому просто договариваемся

  1. Бранч имеет номер этой задачи
  2. Коммиты все вяжутся к дочерним задачам
  3. Закрываются сначала все дочерние, потом считаем закрытой этой
  4. Эта задача - по сути спецификация, потом превратится в документ, спецификация может меняться по ходу реализации, будем следить

Что сейчас

  1. Engine представляет из себя "швейцарский нож", в котором реализовано много методов расчета различных вариантов запросов в различных вариантах входных параметров, то есть у нас есть 
    class Engine
   decimal query1(p1, p2, p3)
   decimal query2(p1, p4)
   ...
   decimal queryX(p10,p2,pX)
  2. При этом все время чего-то не хватает и приходится добавлять новое (нестабильный класс и интерфейс), то есть появление нового запроса Y приведет к появлению метода, а изменение параметров прокаскадирует опять же по интерфейсу
  3. Внутри Engine не соответствует понятию Single-Response (то есть делает много дел, причем на уровне каждого метода):
  4. Разложить входные параметры на несколько подзапросов
  5. Выполнить подзапросы
  6. Преобразование параметров в наборы условий на Entries
  7. Контроль фаз выполнение
  8. Приведение подрезультатов к плюсу и минусу
  9. Сведение результатов

Что должно быть (на крупном плане)

  1. Engine - по своей роли это пользовательский фасад для обработки зпросов к модели, соответственно должен строиться как строятся подобные вещи в базах данных
  2. у Engine должен быть только один метод (реально единственный публичный метод) 
    public decimal Sum( Query query )
  3. Где Query - это нечто полностью себя описывающее (о Query - ниже)
  4. Чтобы не потерять удобство использования типовых запросов создается QueryBuilder 
    class QueryBuilder
   static Query query1(p1, p2, p3)
   static Query query2(p1, p4)
   ...
   static Query queryX(p10,p2,pX)
  5. Соответственно делается миграция: 
    var sum = engine.query1(p1,p2.p3) 
// превращается в =>
var sum = engine.Sum(QueryBuilder.query1(p1,p2,p3))
// а так как можно делать using static
var sum = engine.Sum(query1(p1,p2,p3)
  6. Ну и главный выигрыш, если нам нужен какой-то неведомый доселе запрос 
    var sum =  engine.Sum(new Query(...){...})

    то есть интерфейс стабильный, запросы обрабатываются любые запросы

Что известно про Query

  1. Для Query нужен корректор знака - часть запросов должна приводится к минусу (например при вычитании расходов), значит:

    class Query
  bool Negate = false

  2. Query бывают первичные (которые можно прямо запросить с данных) и производные, состоящие из нескольких подзапросов, следовательно это иерархия, а иерархии строятся так:

    class Query
  Query Parent
  List<Query> Children

    Также чтобы вызывающий Engine не лазал сам в Children (не воспитывал чужих детей) добавлем такой хэндлер

    class Query
    public IEnumerable<T> CollectSubresults<T> ( Func<Query,T> collector) {
         if(Children!=null){foreach( var c in Children ) {
                  yield return collector(c);
          }
}
       yield default(T)
   }

  3. У Query есть признак как его сводить, пока мы знаем один тип - суммирование и первичный (с базы), но все равно делаем как enum

    class Query  // можно назвать EntryQuery, но тогда все равно надо будет сделать IQuery и abstact Query
  QueryType = Primary  // Primary | Sum | Formula
 // на вырост
  IFormula Formula
    interface IFormula
    enum QueryType
    Primary // запрос выполняется относительно исходных данных (таблицы, массива и т.д.)
    Sum // запрос выполняется относительно Children, сам не лезет в исходные данные и суммирует результаты
    Formula // запрос выполняется по какой-то специальной переданной логике (не вырост)

И вот уже можно сделать наш остаток в один запрос

var ostatok = new Query {
     QueryType = Sum
     Children = {
            new Query ( QueryType=Primary, /* тут условие на доход */ ),
            new Query ( QueryType=Primary, Negate = true, /* тут условие на расход */ )
     }
}
  1. Для работы с исключением внутреннего оборота нам требуется еще один модификатор IgnoreMinus (надо определиться до или после Negate он применяется) чтобы расщепить разницу в переводах на расходы и доходы 
    class Query
  bool IgnoreMinus = false

    Тогда полная сумма расходов за исключением внутренних оборотов может выглядеть так

    var fullrshod = new Query() {
 QueryType = Sum
 Children = {
        new Query () { QueryType=Sum, IgnoreMinus=true, 
        Children= { 
            new Query() { QueryType  = Primary, /* условие на сумму расходов по переводам (с плюсом)*/},
            new Query() { QueryType  = Primary, Negate=true /* условие на сумму доходов по переводам (с минусом)*/},
    }},
        new Query (){ QueryType=Primary,  /* тут условие на обычные расходы */  }
 }
}

    а на уровне QueryBuilder выглядело бы так

    
var baseQuery = new Query( /* только условия отбора */ )
...
static class QueryBuilder {
 static Query Query.createRashodNoInternal(Query query) {
           var result = new Query() {
 QueryType = Sum
  Parent = query,
 Children = {
        new Query () { QueryType=Sum, IgnoreMinus=true, 
        Children= { 
            new Query() { QueryType  = Primary},
            new Query() { QueryType  = Primary, Negate=true},
    }},
        new Query (){ QueryType=Primary }
 }
}
          return result
 }

} ... var fullrashod = new Query(){ CreateDate.From = ... , CreateDate.To = ...}.createRashodNoInternal()


И так далее, то есть постепенно развивая различные опции запросов и добавляя возможно дополнительные потом агрегации (например формулы и т.п.) мы можем получить искомую ситуацию - любой запрос сводим к конкечному набору вложенных подзапросов, к в терминальных узлах сидят Primary. 
Соответственно можно эффективно в один запрос (не в 1 SELECT, а именно в 1 ЗАПРОС, который может быть и UNION и просто множественным, но в один сеанс связи) собрать все необходимые первичные сведения, а потом их быстро обсчитать.

# Как задать условие в Query?

Какие требования главные:
1. Должно иметь хорошее зацепление с SQL и Entity Framework - то есть быть простыми и выразимыми для LINQ или в плоском SQL
2. Их должно быть легко настраивать
3. Должно быть понятно как соотносится с Entry

Лучше всего выглядит это примерно так.

1. Вообще есть интерфейс ICondition:

interface ICondition IEnumerabe Apply( IEnumerable query );

но не

interface ICondition IEnumerabe Apply( IEnumerable query );

потому как может оказаться, что на уровне хранения в памяти у нас Entry, а в EF замапали SqlEntry
и соответственно какие-то условия должны по разному себя вести, например Department

class DepartmentCondition: ICondition { public override IEnumerabe Apply( IEnumerable query ){ if(typeof(T)==typeof(Entry))return Apply( (IEnumerable) query); else (typeof(T)==typeof(SqlEntry))return Apply( (IEnumerable) query); // else throw new NotSupportedException("Не применимо к типу ${typeof(T)}") // else return query } IEnumerabe Apply( IEnumerable query ){ return query.Where( => .Incomde.Department.Code == "kitchen" ) } IEnumerabe Apply( IEnumerable query ){ return query.Where( => .IncomeDepartmentCode == "kitchen" ); } IEnumerabe Apply( IEnumerable query ){ return query.Where( => .Department.Code == "kitchen" ); } }

внутри он в реализациях делает что-то такое:
IEnumerabe Apply( IEnumerable query ) {
       return query.Where( _ = _.Somefield = "value" );

}

Соответвтенно можно взять коллекцию условий, применить их на исходную коллекцию, например Entries и соответственно в случае с Entity Framework получить WHERE часть SQL или в случае просто со списками - обычный чейн фильтров

Соответственно Query типа Primary - должен включать в себя набор таких ICondition

class Query { List Conditions IEnumerable ApplyConditions( IEnumerable base ) { var result = base; if(null!=this.Parent) { result = this.Parent.ApplyConditions( result ) } foreach(var c in this.Conditions ) { result = c.Apply(result) } return result; } }

Соответственно для удобства надо сделать типизированные условия, например условие на период дат:

class DateRangeCondition : ICondition { DateTime From DateTime To public override IEnumerabe Apply( IEnumerable query ) { return query.Where( = .CreateDate >= From && _.CreateDate < To ); } }

Так мы достигнем соответствия SQL и EF - первого пункта требований и частично 2-е про удобное использование.

Но еще удобнее будет, если Query будет устроен примерно так (для всех возможных типов условий)

class Query private DateRangeCondition _createDateCondition; public DateRangeCondition CreateDate { get { if(null==_createDateCondition) CreateDate = new DateRangeCondition() return _createDateCondition; } set { this.Conditions.Remove(_createDateCondition); this._createDateCondition = value; if(null!=value){ this.Conditions.Add(value); } } }

Соответственно у нас с одной стороны - есть коллекция условий, которую можно в цикле обежать, с другой стороны есть типизированные с нормальными именами условия, к тому же с защитой от null

var query = new Query { CreateDate.From = new DateTime(...), CreateDate.To = new DateTime(...) }

причем читаемость можно и дальше увеличивать.

Условия - это творческий момент.
Очевидно - должны появиться AccountCondition, CategoryCondition и так по всем полям Entries , у них может есть какой-то базовый класс, надо предусмотреть поиск на "равно", на "вхождение (IN)" и тому подобное

# Как собственно сделать обход вычислений?

Пока будем делать внутри Engine, потому что пока сложно сообразить - будут ли существовать какие-то сложные сценарии более высокого уровня, потребующие отделить сущность выполнителя запросов от Engine. Может да - вот потом и проведем рефактор.

Сейчас сделаем в самом примитивном виде:

class Engine : IEngine public override decimal Sum ( Query query ) { decimal result = 0m; decimal rawresult = 0m; if ( query.QueryType == Primary ) { rawresult = PrimarySum ( query ); } else if (query.QueryType == Sum){ rawresult = query.CollectSubresults( => this.Sum ( ) ) .Sum(); // рекурсивный вызов с суммированием }else { rawresult = ExecuteFormula(query); } result = AdaptResult ( rawresult, query);

        return result;

   }

   decimal ExecuteFormula( Query query) {
       throw new NotImplementedException("Пока формулы не поддерживаются");
   }

   decimal AdaptResult ( decimal result, Query query ) { 
          if ( query.IgnoreMinus && result < 0 ) return 0;
          if ( query.Negate )  return -result;
      return  result;
   }

  decimal PrimarySum(Query query ) { 
    var base = Entries;
            var filtered = query.ApplyConditions(base)
            return base.Sum(_=>_.Value);
 }
Вот вроде и все - может где-то ошибся - но по мне так это вся реализация из того что уже известно.

# Где должно жить? В model или в engine

При беглом взгляде можно было бы считать, что Query и ICondition - часть Engine, так как используются им. То есть ближе к Engine.

Но правильнее помыслить чуть по другому:

1. самим Query и ICondition ничего не нужно от Engine - то есть они не ОБЯЗАНЫ жить в engine, они "вольноопределяющиеся" им нужны только классы модели
2. вопрос надо по другому ставить - могут ли потребоваться классы Query ДО engine - на уровне DAO или в самой model или в модулях типа rest, но вне Engine
3. Нужен ли он в исходной model - вопрос отпадает, по идее Entry и прочее не должно знать про Query
4. А вот насчет DAO и насчет REST ответ другой - 
а) можно представить себе хранение описаний запросов в БД и их кэширование - легко
б) мы можем представить себе редактор запросов в вебе, но без их исполнения на engine или использования DAO
5. Соотоветственно - лучшее размещение для Query и Condition - это библиотека model
6. Ну и соответственно QueryBuilder тоже туда тянется, хотя это уже и не столь очевидно - но он тянется туда так как это класс, который собственно диктует "мульки" в составе полей Query - это по сути нотация использования Query - он является наилучшим кандидатом для помощи в модульном тестировании - значит тоже живет вместе с Query

# Query главный - QueryBuilder - меньший приоритет, чем Query (!!!)

var query = new Query () { CreateDate.From = new DateTime(2017,7,1), CreateDate.To=new DateTime(2017,8,1).AddDays(-1) } // возникла мотивация упростить // НЕВЕРНО - делать через QueryBuilder static Query.applyMonthDateRange( year, month) { ... } // ВЕРНО - Улучшить DateRangeCondition, чтобы можно было написать var query = new Query() { CreateDate.Year = 2017, CreateDate.Month = 7 } // особенно это важно если потом делать DSL // SELECT SUM WHERE YEAR = 2017 MONTH = 8 // SELECT SUM WITH EXTENSION 'applyMonthDateRange' PARAMS (2017,8)

Entry == суть предметной области Query == основной метод работы с предметной области


# Среда тестирования (логика формирования по TDD)
1. Уйти в бранч #6 (заизолировали рабочий код)
2. Сделать пустой QueryBuilder но с заготовками под известные уже запросы (как в 1 пункте спецификации)
3. Переписать тест кейса 4 на новую реализации (на QueryBuilder) - понятно все завалится
4. Сделать пустые заготовки на Query, ICondition
5. Написать модульные тесты на Query - по всем методам и свойствам
5.1 уделить внимание Parent-Children с передачей Condtion
5.2 корректный расчет подрезультатов
5.3 - ICondition - моки, стабы

class QueryTest { class RashodCondition : ICondition { ... return query.Where( => .Type = Rashod ) } [TestMethod] // Работать с тестовыми кондициями }

5.4 Сделать и отдельно оттестировать все нужные (уже по кейсу 4) Conditions
5.5 Переписать Engine по спецификации
5.6 Добиться сходимости кейсового тестового теста 4
5.7 Теперь делаем тесты на Engine - также как с кейсом 4 только по-программистки на покрытие кода

class EngineTest { [TestMethod] void RetunZeroIfNoRealQuery(){ assertEquals(0, engine.Sum( new Query { QueryType = Sum } )) }

[TestMethod] void canEvalPrimaryByDateRange(){ assertEquals(123, engine.Sum( new Query(){ CreateDate.Year = 2017, CreateDate.Month = 8 }) } ...


5.8 Добиться покрытия Engine + Query + Condition без учета теста на  Кейс 4
5.9 Добавить нормальную документацию ко всем публичным классам и методам
6.0 Еще раз все свежим глазом обежать и убрать и заменить все что дурно пахнет

НО ЛУЧШЕ, ЧТОБЫ ОТДЕЛЬНО НЕ ПРИШЛОСЬ ДЕЛАТЬ 5.7 - 6.0
На каждом этапе
1. Работать от теста - сначала тест - потом реализация и только минимальная необходимая реализация - сразу обеспечит покрытие
2. Как только описали структуру класса или метода - тут же написать нормальную доку - не надо будет потом много времени тратить все опять отсматривать и править, заодно часто при написании доки выясняется, что и суть задачи не ясна, и архитектура какая-то не такая, непонятная
3. Рефактор каждый коммит и коммиты частые - в идеале "помидор тайминг"