http://joxi.ru/a2X3dZMsyvwwKm

не забывай про .gitignore
не держи лишнего в репозитории
************************************
http://joxi.ru/Vm6QqxphxJ4xer

/*
    пока не ясно - для чего изменил код - какие таски генерятся по умолчанию
    пока не вижу пользы от этих изменений)

    мне кажется, данные поясняли - что есть title, что есть description
*/
*******************************

http://joxi.ru/1A5zNxjuKlDnbr

/*
    про реализацию reset - несколько моментов

    логика
        ты тут фактически дублируешь код  из http://joxi.ru/Vm6QqxphxJ4xer
        не DRY
        Не надо в данном методе сервиса - дублировать коллекцию tasks
        Нужно грамотно воспользоваться тем, что уже есть в коде
        Если начальное состояние фиксировать в некой коллекции tasks_default,
        а при старте и ресете - копировать в  tasks содержимое tasks_default
        В python есть способ  - скопировать содержимое одного списка в другой с помощью одной операции,
        а не переносить каждый элемент списка отдельно

        получишь более DRY код

        полезные линки
        https://docs.python.org/2/library/copy.html
        http://joxi.ru/Dr8vxzPFk3Kl32
        http://stackoverflow.com/questions/2612802/how-to-clone-or-copy-a-list-in-python
        http://joxi.ru/J2b9KdBt41qEpm

   теперь про то, какой метод HTTP ты для этого действия используешь
   сейчас у тебя используется DELETE с URI= /todo/api/v1.0/tasks/reset
   DELETE = удалить ресурс. И в URI по идее мы должны передать - что ж за ресурс мы удаляем
   Кроме того, после этой операции тасок может и прибавится)
   т к не факт, что на момент вызова метода было больше 2-ух тасок (а иначе - какой это delete)
   смыслы слегка "уплыли"


      какие наши варианты
        “идеология REST” говорит - что можно использовать только специальные http methods
        И только по определенному принципу
        Например,
        для создания - использовать post,
        для апдейта - метод put
        и т д

        И нам важно соблюсти этот принцип и свести написание сервисов к одному шаблону
        Вся идея REST была в том, чтобы чтобы потом все могли “общаться на одном языке” и таким образом “понимать друг друга”

        можно было бы реализовать что-то типа такого
        @app.route('/todo/api/v1.0/tasks/reset', methods = [‘PUT’]) (edited)

        НО, хотя мы и делаем “апдейт"
        но - апдейт не ресурса...
        т к reset - это не ресурс

        вот если бы мы для  /tasks посылали PUT, то было бы все честно
        а /tasks/reset - это уже не ресурс
        это просто “хак-урл” , чтобы нам выполнить определенное нужное нам действие
        потому в данном случае абсолютно нормально - посылать GET
        наверное, можно и PUT… решай самостоятельно

        уже лучше, чем DELETE, но все равно не очень...

        еще довод за GET
        обрати внимание - все остальные запросы что-то возвращают - делают return …
        это логично - клиент послал запрос, нужно ему что-то вернуть)
        и тут было бы логично вернуть все таски = (то же, что возвращает запрос get)

*/

*********************************

http://joxi.ru/krDOZldF07K01A

/*
    ага...
    ты так обошел ошибку - когда первую таску создаешь

    ну да...
    полезно исправить)

    только можно красивее
    фактически - у тебя в зависимости от условия - только заполнение id

    остальное - не зависит от условия
    разумно это остальное - вынести из if-а

    будет DRY )
*/
***********************************

package ua.net.vmarchenko.credentials;


public class TestData {

    public static final String URL = "http://localhost:5000/todo/api/v1.0/tasks";
    public static final String RESET_URL = "http://localhost:5000/todo/api/v1.0/tasks/reset";
    public static final String LOGIN = "miguel";
    public static final String PASSWORD = "python";

    public static String usernameAndPassword = LOGIN + ":" + PASSWORD;
    public static String authorizationHeaderValue = "Basic " + java.util.Base64.getEncoder().encodeToString(usernameAndPassword.getBytes());
}

/*
    давай отделим мух от котлет)

    пекедж называется credentials, класс TestData
    а храним мы там URI, логин-пассворд и их производные....

    ну нет)

    URI - точно ни credentials, ни TestData
    его разместим в другом месте)
    ниже напишу
    его - т к наш URI = "http://localhost:5000/todo/api/v1.0/tasks"
    остальное - добавки к нему
    тут тоже стоит быть DRY

    насчет логина-пароля и их производых
    этот сервис подразумевает каких-то других юзеров, кроме miguel?
    если бы да - то я согласилась бы с тобой, что стоило бы реализовать
    класс UsersData в пекедже credentials
    в котором есть информация о нескольких юзерах
    и уж точно в таком классе не место логике преобразования логина-пароля в зашифрованую строку

    вывод - нам в полном составе такой класс не нужен

    гугли Single Responsibility principle

    многих вопросов бы тут у тебя не возникло - если бы сначала добил работы по селениуму
    там бы скиллы прокачал по работе с классами
*/
************************************************

public class helpers {
  /*
    Имя класса - вспомни о конвеншенсах
    правильнее разместить универсальные хелперы в ветке src\main
    в пекедже core
    и уточнить имя класса до RestHelpers
  */

    public static Invocation.Builder requestTo(String url) {
     /*
        тут к реализации нет вопросов
        все же корректнее назвать параметр uri
        погугли при необходимости термин
     */

    public static Invocation.Builder authorized(Invocation.Builder requestBuilder) {
        return requestBuilder.header("Authorization", TestData.authorizationHeaderValue);
        /*
            а тут мы внутрь универсального метода впихнули прямую работу с тестовыми данными
            так лучше не делать - ни для универсальных методов
            ни для методов классов-пейджей или как будем делать сейчас - классов-ресурсов

            тестовые данные - отдельно
            вспомогательные методы - отдельно

            надо что-то передать - делай методу параметр

            вот тут реализуй параметр String credentials
            в который передашь "miguel:python"

            и тут, внутри метода - как раз и преобразуй "miguel:python" в зашифрованую строчку

            метод получится универсальным...
        */
    }

    public static void assertStatusCode(int statusCode) {
        assertEquals(statusCode, ToDoAppRESTfulCRUDTests.response.getStatus());
    }
    /*
        этот метод - явно не отсюда)
        ему место в ToDoAppRESTfulCRUDTests - раз мы обращаемся к его данным

        еще подумаем - нужен ли тебе такой метод)

        уже точно - ему тут не место)

    */
}
******************************************

http://joxi.ru/J2b1MVvs4N00pm

/*
    это не ресурсы, а контейнеры
    размести классы в пекедже containers

    набор классов - тоже вызывает вопросы
    разберемся ниже)
*/
******************************************

public class TasksContainer {


    ...

    public static void assertTasks(List<Task> expectedTasks) {
    public static List<Task> getAllTasks() {
    public static boolean assertTitles(String ... expectedTitle) {
    public static boolean assertDescriptions(String ... expectedDescription) {
    public static boolean assertIsDone(Boolean ... isDone) {
/*
    а вот и отгадка)

    будем исправлять)

    Single Responsibility principle
    у каждого класса есть своя ответственность
    у этого - парсинг с JSON
    и все)

    а вот эту функциональность - будем реализовывать в классе-ресурсе (аналог пейджа в Web UI тестировании)
    может, не именно так, как єта функциональность тут реализована, но эту же самую))
*/
**************************************************

public class Task {

 ...
    public static Entity createTaskEntity(Task task){
/*
    Это тоже не логика для класса, участвующего в сериализации/десериализации
*/

    public static List<Task> getInitialTasksList() {
/*
    а вот это как раз тестовые данные)
    тоже не место им тут)
*/

/*
    в каждом из этих классов - должны быть нужные поля + геттеры-сеттеры

    потом еще кое-что добавим

    но сейчас - только это

    остальное - ответственность других классов
*/
************************************************

/*
    да...
    написала только - что нужно поубирать
    а вот куда - пока не ясно...

    давай разбираться
*/
***************************************************
/*
    Вспомогательные методы разумно вынести в класс - аналог пейджа.
    Теперь у нас будет пекедж не pages, a resources.
    А правила работы с таким классом - аналогичные, как для класса-пейджа.
    Размещай класс в ветке src\main - как то, что можно переиспользовать

    Поскольку мы работаем только с тасками - в имена методов-действий можно не включать слово Tasks
    Имена методов-проверок - по-прежнему указываем точно

    Логично было бы назвать класс Tasks
    (если будем реализовывать как ресурс-модуль, а не ресурс-объект -
    в принципе - нет причин реализовывать как ресурс-объект, но тут
    можешь и сам решить это - может у тебя будут свои резоны)

    И в коде потом можно без статического импорта писать
            Response response = Tasks.create(URI, "give lesson");


    Это было бы и наглядно, и на будущее - на случай, если в тесте появятся запросы к другим ресурсам

        Tasks - неплохое имя
        но у нас есть класс Task
        и можно сказать - что мы используем один термин для достаточно разных понятий….

        Можно в таком случае для класса-ресурса использовать TasksApi или  TasksRest в таком случае…

        Но и Tasks - тоже неплохо…, но хуже)
        Можно пойти на эту уступку в принципе - код понятный, названия наглядные


    Еще одно лирическое отступление)
      На реальном проекте обычно создают класс-ресурс (объект или модуль) не сразу,
      когда начали писать тесты.
      А позже - когда появится необходимость переиспользовать
      вспомогательные методы в разных тест-классах.
      Или если одни более квалифицированные члены команды разрабатывают класс-ресурс,
      а другие - используют для разработки тестов.
      Ну или если так принято в данной команде)
      Т е - это делается обычно при необходимости.
      С пейджами - в общем аналогично.

      Мы сейчас реализуем проект сразус классом-ресурсом - чтобы разобраться, как это делать
*/
***************************************

/*
    Что же там у нас будет

    наш URI "http://localhost:5000/todo/api/v1.0/tasks" - как раз URI для ресурса tasks
    значит - тут и разместим
    на самом деле - и с URI можно еще кое-что улучшить
    но пока не будем распыляться - сделаем при следующих ревью

    практически для всех запросов к серверу нам нужно быть авторизированными
    поскольку только в рамках этого класса мы реализуем методы, которые будут делать такие запросы
    то тут и разместим метод, который будет делать авторизированный запрос для "miguel:python"
    воспользуйся уже реализованным универсальным методом, передав туда в качестве параметра "miguel:python"
    подумай, какой параметр должен быть у метода и что он должен возвращать

    а дальше - реализуем надор методов, работающих с ресурсом
    поскольку работаем с тасками - уже в именах методов-действий это не уточняем
    методы-действия
      - получить таски
      - создать таску (2 метода - только по заголовку или по всей информации)
      - обновить таску
      - удалить таску
    метод-проверка
      - проверить таски (нам нужен такой метод, который будет сверять ожидаемый результат с фактическим - для всех тасок)

  Есть интересный момент
      Встраивать ли проверку статуса в метод-действие
      Если встроить проверку response кода прямо в сам степ

      Дело в том, что если так делать - то мы спрячем тестовую логику внутрь степа, что не очень хорошо
      С другой стороны - эта проверка = само собой разумеющееся
      Эта проверка достаточно стандартная и всегда будет повторяться после вызова степа (если будет жить вне степа)
      А раз так - то почему бы и не спрятать?
      На самом деле - нет однозначного ответа на этот вопрос...
      Так - со спрятанной проверкой - код станет проще во всех тестах - но код станет до некоторой степени “магическим”
      Тут надо решать самостоятельно - как лучше )
      На твое усмотрение

      Еще момент
      В этих API тестах - самих тестов может быть не так уж и много -
      чтоб еще и заморачиваться над DRY кода...нужно и этот аспект учитывать
      Но если пойти этим путем = прятать в степе проверку статуса response, то код будет намного более удобным:)

      Этот момент также повлияет на то, что каждый из методов нам будет возвращать

*/
***************************************

  public class ToDoAppRESTfulCRUDTests {
  /*
    конвеншенсы ) (имя тест-метода заканчивается на Test)
    тексту CRUD - тоже не место в имени класса
    т к его придется править - если будешь расширять функциональность

    ToDoAppRESTfulTest
    RESTfulTest

    кстати, тут есть нюанс тоже - по кемел кейсу)
    https://google.github.io/styleguide/javaguide.html#s5.3-camel-case
    не RESTfulTest, а RestFulTest )

  */

  /*
    тут - чтоб далеко не ходить - стоит как раз и объявить тестовые данные - 2 таски по умолчанию

      С таким вынесением тестовых данных в переменные надо быть осторожным
      Важно - не надо тестовые данные объявлять в ресурс модуле
      (опять аналогия с пейджом - мы и там такого правила придерживались)
      Тут и правда есть смысл объявить эти данные
      Часто тестовые данные разумно оставлять в логике самих тестов -
      таким образом код тестов более очевидный. Пример - тудуэмвиси тесты
      Но - если эти данные вынесены в переменные - то их уже и используй при проверках

  */

    public  static Response response;
    /*
        мудрено получилось)
        сделать эту переменную статической, чтобы юзать ее в рамках другого класса...
        не....
        перебор)

        тебе не нужна в тест-классе такая переменная
        все методы по реализации запросов к серверу - будут жить в классе-ресурсе
        и в рамках каждого из методов просто создавай свой Response
    */
    Client client;
    /*
        ну...
        не вижу пока преимуществ в том, чтобы создавать перед тестом эту переменную и затем удалять

        в видео Яков ничего не говорил о том, что нужно закрывать (client.close();)

        пришлось таки мне поковыряться, чтоб сложить свое мнение - нужно ли
        Пока на 100% в своем мнении не уверена - делись контраргументами, линками и т д)

      Надо ли закрывать наш объект типа ClientBuilder
          Это а самом деле интересный вопрос - надо ли закрывать наш объект типа ClientBuilder, который мы создали)
          С одной стороны, надо - вот что написано в документации
          https://docs.oracle.com/javaee/7/tutorial/jaxrs-client001.htm
          и вот они примеры
          http://www.programcreek.com/java-api-examples/index.php?class=javax.ws.rs.client.Client&method=close

          С другой стороны - вот что
          http://grepcode.com/file/repo1.maven.org/maven2/javax.ws.rs/javax.ws.rs-api/2.0/javax/ws/rs/core/Response.java
          а именно - вот это http://joxi.ru/8An6LoZhqdNzyA
          вот тут еще почитай
          http://stackoverflow.com/questions/35265534/closing-connection-in-get-request-using-jersey-client-2-22-1

          тут есть примеры тест-методов
          http://www.hascode.com/2013/12/jax-rs-2-0-rest-client-features-by-example/

          плюс - эксперимент
          создай объект  ClientBuilder до начала тестов(всех)
          и пусть он используется для работы во всех тестах
          а после всех тестов - закрой ранее созданный объект
          тесты падают буквально при при попытке второго обращения к объекту ClientBuilder

          вывод - при нашем способе использования ClientBuilder - таки закрывать не нужно
          достаточно реализовать так, как показано в видео -
          для получения ответа на запрос используй  ClientBuilder.newClient().target(uri).request()


      вывод - переменная тебе не нужна (наверное))) - если у тебя нету контраргументов )

    */
    WebTarget target;
    /*
        а эту ты и вовсе не используешь - создал и все
    */
****************************************************


    @Before
    public void setUp() {
        client = ClientBuilder.newClient();
        target = client.target(TestData.URL);
    }

    @After
    public void tearDown() {
        requestTo(TestData.RESET_URL).delete();
        client.close();
    }
/*
    из вот этого кода - полезно делать ресет
    и я бы советовала его делать не после теста, а до
    т к не факт, что до начала работы у нас состояие = начальное
*/
***********************************************

    @Test
    public void testUnauthorizedTasksRead() {
        response = requestTo(TestData.URL).get();
        assertError("Unauthorized access");
        assertStatusCode(403);
    }
/*
    этот тест-метод - останется почти таким же

    за исключением того, что уйдут методы assertError и assertStatusCode
    и тут - просто используй их реализацию

    на самом деле, вот такой код
    assertEquals("Unauthorized access", response.readEntity(ErrorContainer.class).getError());
    не сказать, чтобы сильно сложнее

    если и делать такой метод, то точно не в ErrorContainer (Single Responsibility)
    и не в классе-ресурсе... - т к речь не о ресурсах
    и не в RestHelpers - т к все же не универсальная вещь, а привязанная к конкретным структурам данных,
    которые используются в нашем конкретном рест сервере
    а если вспомнить - что пока только в этом месте нам нужно анализировать ошибку
    так прекрасно можно обойтись строкой assertEquals("Unauthorized access", response.readEntity(ErrorContainer.class).getError());

    про проверку статуса
    сравни
    assertStatusCode(403);
    и
    assertEquals(403, response.getStatus());
    вопрос - стоит оно того, чтоб его в метод заворачивать и куда-то прятать?

*/
**********************************

    @Test
    public void testReadAll() {
        /*
            сомнительная польза от уточнения - что это ReadAll
            а не Read

            если бы были оба варианта - тогда да, польза бы была)
        */
        response = authorized(requestTo(TestData.URL)).get();
        /*
            тут код может быть проще
            List<Task> actualTasks = TasksApi.get();
            это если внутри метода класса-ресурса проверять статус (см строка 335 в этом ревью)

            если не хочется прятать, то сложнее
            Response response = TasksApi.get();
            List<Task>  = response.readEntity(TaskContainer.class).getTask();

            я бы спрятала)

        */
        List<Task> expected = getInitialTasksList();
        /*
            это наши тестовые данные, и мы их расположили уже по-другому (выше есть про это)
        */

        assertTasks(expected); // скрыта тестовая логика - нужно думать как реализовать
        /*
            а тут нам вполне подойдет assertEquals()
            который сравнит ожидаемый и действительный результат

            для начала - оба эти результата - типа List<Task>

              посмотри на информацию по этим линкам
              http://www.java2s.com/Tutorial/Java/0140__Collections/CheckingforEquality.htm
              http://www.javapractices.com/topic/TopicAction.do?Id=17
              http://javadevnotes.com/java-array-to-list-examples

              мы можем получить код проще
              public static void assertTasks(Task... expectedTasks) {
                      List<Task> actualTasks = get();
                      assertEquals(Arrays.asList(expectedTasks), actualTasks);
              }

              expectedTasks в данном случае - массив
              А если реализуешь метод toString для класса Task - и сообщение об ошибке будет более информативным.

        */
**************************************
public class Task {

    String description;
    boolean done;
    private String title;
    String uri;

    public Task() {

    }

    public Task(String title) {
        this.title = title;
    }

/*
    перед обсуждением метода testCreateWithTitle - самое время поговорить о конструкторах

    нужен еще один конструктор, в котором ты можешь прописать все поля без исключения
    нам нужен такой конструктор и для реализации второго testCreateWithFullInformation,
    и для использования в сравнении ожидаемого и фактического результатов

    и в случае, если в конструкторе ты что-то не прописываешь, задавай некое значение по умолчанию

    что-то типа такого
     public Task(String title) {
        this.title = title;
        this.description = "";
        this.done = false;
        this.uri = "";
    }

    цель - уйти от нуллов, они нам будут мешать при сравнениях тасок
    если это не понятно пока - так реализуй сравнение и как раз увидишь проблемы)

    подумай, как код конструкторов можно сделать более DRY

*/


    @Test
    public void testCreateTask() {
    /*
        определись с употреблением слова Task в именах методов
        у тебя то так, то так
    */
        List<Task> expectedTasks = getInitialTasksList();
        Task newTask = new Task("T 3");
        expectedTasks.add(newTask);
        /*
            вот эти манипуляции с expectedTasks - лучше не делать....
            чтобы не усложнять код
        */
        response = authorized(requestTo(TestData.URL)).post(createTaskEntity(newTask));

        assertTasks(expectedTasks);
        /*
            тут, и в остальных тест-методах можно применить такой способ работы
            наша цель - получить максимально читаемый код

            TasksApi.create("t3 title"); - тут мы создали таску с таким-то тайтлом, проверили статус,
            вернули созданную таску (при необходимости - можно что-то проанализировать)

            TasksApi.assertTasks(DEFAULT_TASKS[0], DEFAULT_TASKS[1], new Task("t3 title", "", false, TasksApi.uri + "/3"));
            методу передаем ожидаемый результат
            внутри метода - получаем список тасок и сверяем его
            DEFAULT_TASKS - массив со значениями по умолчанию

            заметь - при проверке передаем в качестве третьей таски - новую таску с ожидаемо заполненными полями

        */
        assertStatusCode(201);
    }

    @Test
    public void testCreateTaskWithoutParameters() {
        /*
            ну...
            если решил проверить на Bad request
            то стоит такие тесты написать для всех операций)
            причем вариантов там много - думаю, стоит это делать глядя на код рест сервера

            но...
            предлагаю таки этого не делать)

            первое, что нужно, чтобы рест-сервер корректно обрабатывал верные запросы)
            давай сосредоточимся на первом)

            предлагаю ограничиться двумя методами для теста создания таски
            testCreateWithFullInformation и testCreateWithTitle

        */
        TaskWithoutTitle task = new TaskWithoutTitle();
        response = authorized(requestTo(TestData.URL)).post(Entity.entity(task, MediaType.APPLICATION_JSON));

        assertError("Bad request");
        assertStatusCode(400);
    }

    @Test
    public void testUpdate() {
        List<Task> receivedTasks = getAllTasks();
        Task lastTask = receivedTasks.get(receivedTasks.size()-1);
        lastTask.setTitle(lastTask.getTitle() + " edited");
        lastTask.setDone(true);
        lastTask.setDescription(lastTask.getDescription() + " edited");
        /*
            вот эта логика реально лишняя

            у нас же гарантированно на начало теста - 2 такие-то таски
            ты знаешь какие
            все их данные зашиты в тестовых данных

            ты можешь создать
             Task task = new Task("t2 title", "t2 description", true, TasksApi.uri + "/2");

            и выполнить апдейт таски. ее URI мы уже задали

            а дальше - достаточно проверки с помощью все того же метода assertTasks
        */

 *******************************************
Есть костыль вроде создания класса таски без поля tile -
другого решения не нашел как проверить что это поле обязательное
/*
    предлагаю уйти от этого)
    выше писала
*/
(null подставляется когда использую с пустым конструктором)
/*
    ничего не мешает в конструкторе без параметров предзаполнить поля значениями по умолчанию = пустыми строками
*/

В некторых местах скрыта тестовая логика, еще буду думать как это исправить.
Так же в некоторых местах для верификации тасок использую сравнение всех атрибутов
вместо метода assertTasks (опять же это из-за того что скрыта тестовая логика)
/*
    надеюсь, описание позволит распутать это)

    ревью длинное и не простое)

    думаю, ты избежал бы многих проблем и вопросов,
    если бы это делал уже после работ по селениуму
    при написании фреймворка многое бы разобрали - общие принципы в том числе)

    но ты не расстраивайся.
    ок
    начали с этой работы
    значит - работы по селениуму будет потом делать легче)
    ведь часть общих принципов разберем тут
*/