2
\$\begingroup\$

I've come up with a way to externalize and read in my .sql files instead of having them in-code as strings. Note that I'm keeping the .sql file in the same package as the DAO (NOT separated into a separate resources folder).

I'm curious what others think about this methodology.

The sqlStatement is inject on class load (which because this is managed by Spring, on application start). It will call the static method SqlReader.getStatement(), passing in the file name.

Data Access

@Repository
public class TestDAO {

    @Value("#{sqlReader.getStatement('com/mycompany/selectStatement.sql')}")
    private String selectStatement;

The SqlReader is a Utility class located in another package.

public class SqlReader {

    public static String getStatement(final String file) {

        try {
            return FileUtils.readFileToString(new ClassPathResource(file).getFile(), StandardCharsets.UTF_8);
        } catch (IOException exception) {
            Logger.error("IOException reading file [" + file + "].", exception);
            throw new IllegalStateException("Error reading for file [" + file + "]", exception);
        }
    }
}

Updated: updated the error handling. also, made SqlReader a Spring component, which cleans up the @Value expression by removing the necessity to lookup the SqlReader by type (Spring can find it by it's bean name now).

\$\endgroup\$
1
\$\begingroup\$

The idea to extract SQL queries from the code into in a separate resource is good in general. Its main advantage is that when there is a non-substantial change at the database side, it might be enough to change the resource file, without recompiling the code.

However, it slightly reduces the readability: the query is in a separate file, you do not have it immediately visible near the instructions that process the ResultSet. That can become confusing if the query is passed to a PreparedStatement with a number of parameters, but everything finally depends on each case. This discussion can easily end up with arguing whether inline SQL is better than using an ORM :)

For the solution that you submitted, I can observe the following:

  • each *.sql file can contain only one query. If there are a lot of queries used by objects within a package, this can easily pollute the folder and you'll need to be careful when choosing a naming convention for the files, otherwise one will be easily lost among them. selectStatement.sql is just an example and real resources should have names like selectBookByAuthor.sql etc.

  • a single string with plain and directly usable SQL is replaced with two values which are not obvious to change and maintain if necessary: the class name (com.mycompany.SqlReader) and the file name (com/mycompany/selectStatement.sql), because both are packed in the same string expression. Would you rely on your IDE or make all changes manually for each occurrence when these two values are changing?

  • the UnsupportedOperationException is not the best choice for this case. Logically, the operation is supported, but something gone wrong here and the expected predefined statement could not be read. The application is thus in an invalid state: the .sql was not included in the build sequence or a typo in the file name... I think that IllegalStateException is much better here. It also should be moved inside the catch block and chained with the IOE (throw new IllegalStateException(ioexception);). You should probably consider adding to the catch block other exception types, if any, that can result from calling FileUtils.readFileToString.

Another Approach

I can suggest another approach that has more or less the same result of externalizing SQL queries, but can be achieved without the dedicated SqlReader class, only by using the features of Spring.

The fields representing SQL queries can be initialized from property file sources:

@Value("${sql.select.products}")
private String selectProductsQuery;

The properties file must contain the key sql.select.products.

The annotation @PropertySource is used to point to the properties file to look in:

@PropertySource("classpath:/queries.properties")
public class TestDAO { ... }

In queries.properties you just specify the queries to use:

sql.select.products=SELECT p.id, p.name ... FROM product p...

The advantages:

  • it's all managed by Spring, once the configuration is set up properly.

  • You have all the queries in the same file.

\$\endgroup\$
  • \$\begingroup\$ it was my intention to make each .sql file it's own statement -kind of goings with the pattern of making each DAO method just one database call as well (combining multiple methods/calls in a Transaction). And I put them specifically in the same package as the rest of the logic that would call it. \$\endgroup\$ – Tony Card Sep 12 '16 at 13:31
  • \$\begingroup\$ I totally agree with you -the two strings used for injection can get long and potential breaking point if things change. I'd really like to pursue the potential of a separate annotation/aspect that would inject statement without having to specify the SqlReader.getStatement() (hide the call in the aspect itself). \$\endgroup\$ – Tony Card Sep 12 '16 at 13:31
  • \$\begingroup\$ You're absolutely right, UnsupportedOperationException is not really appropriate. Nor is swallowing the IOException -I'll definitely have to rework that. \$\endgroup\$ – Tony Card Sep 12 '16 at 13:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.