Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add annotation @ExpectMaxUpdatedColumn #2

Open
wants to merge 2 commits into
base: master
from

Conversation

@ablanchard
Copy link
Contributor

commented Jun 17, 2019

Motivation

Users might want to ensure that a method to not update more columns that required. By putting @ExpectMaxUpdatedColumn(3) quickperf wil assert that no more than 3 columns have been updated.

Implementation

Context

We want to count columns passed in the update SQL query. We will count the parameter setted in the set clause of the query. We suppose also that with are using PreparedStatement to execute this update. This is what hibernate do.

Counting

How to return 2 in this example UPDATE book SET isbn = ?, title = ? WHERE id = ?
To count updated columns, we cannot rely on query.getParametersList().get(0) because this list will contains all 3 parameters of the query.
Instead we can count the occurrences of the character ? which represent a parameter. We must count those occurrences only on the substring SET isbn = ?, title = ? to exclude ? placeholders used elsewhere in the query.

Testing

It would be great to unit test the method SqlExecutions#countUpdatedColumn with different string query. But the module sql-annotations do not have yet a testing package neither a testing framework.
So @jeanbisutti what do you think about adding one ?

Not implemented

According to postgres documentation of Update statement (https://www.postgresql.org/docs/9.1/sql-update.html) :

[ WITH [ RECURSIVE ] with_query [, ...] ]
UPDATE [ ONLY ] table [ * ] [ [ AS ] alias ]
    SET { column = { expression | DEFAULT } |
          ( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) } [, ...]
    [ FROM from_list ]
    [ WHERE condition | WHERE CURRENT OF cursor_name ]
    [ RETURNING * | output_expression [ [ AS ] output_name ] [, ...] ]

Update can also contains FROM or RETURNING. We should consider those keywords also to extract the substring on which we do the count.

@jeanbisutti

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2019

Many thanks for your contribution!

With the current implementation, we find that the following tests don't fail but they should. In these cases, the update queries don't contain bind parameters.

    @Test
    @ExpectMaxUpdatedColumn(1)
    public void execute_update_with_two_columns_updated() {
        EntityManager em = emf.createEntityManager();
        em.getTransaction().begin();
        String sql = "UPDATE book SET isbn ='12EEE', title = 'Book title'";
        Query nativeQuery = em.createNativeQuery(sql);
        nativeQuery.executeUpdate();
        em.getTransaction().commit();
    }
    @Test
    @ExpectMaxUpdatedColumn(1)
    public void execute_update_with_two_columns_updated() {
        EntityManager em = emf.createEntityManager();
        em.getTransaction().begin();
       String sql = "UPDATE book SET isbn ='12,EEE', title = 'Book title'";
        Query nativeQuery = em.createNativeQuery(sql);
        nativeQuery.executeUpdate();
        em.getTransaction().commit();
    }
    @Test
    @ExpectMaxUpdatedColumn(1)
    public void execute_update_with_two_columns_updated() {
        EntityManager em = emf.createEntityManager();
        em.getTransaction().begin();
        SString sql = "UPDATE book SET isbn ='12EEE', title = ' '', '";
        Query nativeQuery = em.createNativeQuery(sql);
        nativeQuery.executeUpdate();
        em.getTransaction().commit();
    }

Could you please write automatic tests for the previous use cases and update the implementation to have the expected behavior?

The number of updated columns may be found considering the number of commas not between two quotes. You may transform the sql String into a char array and iterate on this array to do this.

We prefer fast (memory database) integration tests to unit tests because integration tests check the whole implementation. In some specific cases, some unit tests could be interesting. For example, in MaxUpdatedColumnsPerMeasureExtractor we could mock SqlExecutions and write tests for update syntaxes specific to PostgreSQL (not asked in the frame of this pull request but if you are interested you could submit another pull request, we would be happy of that :))

One more time, thanks a lot for your contribution.

@jeanbisutti

This comment has been minimized.

Copy link
Collaborator

commented Jul 7, 2019

I wrote the additional tests:

   @Test
    public void
    should_fail_if_no_bind_parameters_and_the_number_of_updated_columns_is_greater_than_the_expected_number() {

        // GIVEN
        Class<?> testClass = AClassAnnotatedWithExpectMaxUpdatedColumnFailingInCaseOfNoBindParameters.class;

        // WHEN
        PrintableResult printableResult = PrintableResult.testResult(testClass);

        // THEN
        SoftAssertions softAssertions = new SoftAssertions();

        softAssertions.assertThat(printableResult.failureCount()).isEqualTo(1);

        softAssertions.assertThat(printableResult.toString())
                      .contains("Maximum expected number of updated columns <1> but is <2>.")
                      .contains("UPDATE")
                      .contains("book")
        ;

        softAssertions.assertAll();

    }

    @Test
    public void
    should_fail_if_no_bind_parameters_and_comma_in_updated_column_and_the_number_of_updated_columns_is_greater_than_the_expected_number() {

        // GIVEN
        Class<?> testClass = AClassAnnotatedWithExpectMaxUpdatedColumnFailingInCaseOfNoBindParametersAndAQuoteInUpdatedColumn.class;

        // WHEN
        PrintableResult printableResult = PrintableResult.testResult(testClass);

        // THEN
        SoftAssertions softAssertions = new SoftAssertions();

        softAssertions.assertThat(printableResult.failureCount()).isEqualTo(1);

        softAssertions.assertThat(printableResult.toString())
                      .contains("Maximum expected number of updated columns <1> but is <2>.")
                      .contains("UPDATE")
                      .contains("book")
        ;

        softAssertions.assertAll();

    }

    @Test
    public void
    should_fail_if_no_bind_parameters_and_quote_in_updated_column_and_the_number_of_updated_columns_is_greater_than_the_expected_number() {

        // GIVEN
        Class<?> testClass = AClassAnnotatedWithExpectMaxUpdatedColumnFailingInCaseOfNoBindParametersAndAQuoteInUpdatedColumn.class;

        // WHEN
        PrintableResult printableResult = PrintableResult.testResult(testClass);

        // THEN
        SoftAssertions softAssertions = new SoftAssertions();

        softAssertions.assertThat(printableResult.failureCount()).isEqualTo(1);

        softAssertions.assertThat(printableResult.toString())
                      .contains("Maximum expected number of updated columns <1> but is <2>.")
                      .contains("UPDATE")
                      .contains("book")
        ;

        softAssertions.assertAll();

    }
/*
 * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
 * the License. You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
 * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
 * specific language governing permissions and limitations under the License.
 *
 * Copyright 2019-2019 the original author or authors.
 */

package org.quickperf.sql.update.column;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.quickperf.junit4.QuickPerfJUnitRunner;
import org.quickperf.sql.SqlTestBase;
import org.quickperf.sql.annotation.ExpectMaxUpdatedColumn;

import javax.persistence.EntityManager;
import javax.persistence.Query;

@RunWith(QuickPerfJUnitRunner.class)
public class AClassAnnotatedWithExpectMaxUpdatedColumnFailingInCaseOfNoBindParameters extends SqlTestBase {

    @Test
    @ExpectMaxUpdatedColumn(1)
    public void execute_update_with_two_columns_updated() {
        EntityManager em = emf.createEntityManager();
        em.getTransaction().begin();
        String sql = "UPDATE book SET isbn ='12EEE', title = 'Book title'";
        Query nativeQuery = em.createNativeQuery(sql);
        nativeQuery.executeUpdate();
        em.getTransaction().commit();
    }

}
/*
 * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
 * the License. You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
 * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
 * specific language governing permissions and limitations under the License.
 *
 * Copyright 2019-2019 the original author or authors.
 */

package org.quickperf.sql.update.column;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.quickperf.junit4.QuickPerfJUnitRunner;
import org.quickperf.sql.SqlTestBase;
import org.quickperf.sql.annotation.ExpectMaxUpdatedColumn;

import javax.persistence.EntityManager;
import javax.persistence.Query;

@RunWith(QuickPerfJUnitRunner.class)
public class AClassAnnotatedWithExpectMaxUpdatedColumnFailingInCaseOfNoBindParametersAndACommaInUpdatedColumn extends SqlTestBase {

    @Test
    @ExpectMaxUpdatedColumn(1)
    public void execute_update_with_two_columns_updated() {
        EntityManager em = emf.createEntityManager();
        em.getTransaction().begin();
        String sql = "UPDATE book SET isbn ='12,EEE', title = 'Book title'";
        Query nativeQuery = em.createNativeQuery(sql);
        nativeQuery.executeUpdate();
        em.getTransaction().commit();
    }

}
/*
 * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
 * the License. You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
 * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
 * specific language governing permissions and limitations under the License.
 *
 * Copyright 2019-2019 the original author or authors.
 */

package org.quickperf.sql.update.column;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.quickperf.junit4.QuickPerfJUnitRunner;
import org.quickperf.sql.SqlTestBase;
import org.quickperf.sql.annotation.ExpectMaxUpdatedColumn;

import javax.persistence.EntityManager;
import javax.persistence.Query;

@RunWith(QuickPerfJUnitRunner.class)
public class AClassAnnotatedWithExpectMaxUpdatedColumnFailingInCaseOfNoBindParametersAndAQuoteInUpdatedColumn extends SqlTestBase {

    @Test
    @ExpectMaxUpdatedColumn(1)
    public void execute_update_with_two_columns_updated() {
        EntityManager em = emf.createEntityManager();
        em.getTransaction().begin();
        String sql = "UPDATE book SET isbn ='12,EEE', title = 'Book title'";
        Query nativeQuery = em.createNativeQuery(sql);
        nativeQuery.executeUpdate();
        em.getTransaction().commit();
    }

}

@jeanbisutti jeanbisutti force-pushed the quick-perf:master branch from 6d2082d to b8b9a2d Jul 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.