Practicing Java servlets, MySQL and JSON - follow-up

Question

What's new

I have incorporated all the points of Dmytro Maslenko's answer. My code seems a little bit tidier.

Code

AddPersonController.java

package net.coderodde.web.db.fun.controllers;

import java.io.IOException;
import java.io.PrintWriter;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import net.coderodde.web.db.fun.model.FunnyPerson;

/**
 * This controller is responsible for creating new persons.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Aug 8, 2017)
 */
@WebServlet(name = "AddPersonController", urlPatterns = {"/add_person"})
public class AddPersonController extends HttpServlet {

    /**
     * Handles the HTTP <code>GET</code> method.
     *
     * @param request  the servlet request.
     * @param response the servlet response.
     * @throws ServletException if a servlet-specific error occurs.
     * @throws IOException if an I/O error occurs.
     */
    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        response.setContentType("text/html;charset=UTF-8");
        try (PrintWriter out = response.getWriter()) {
            out.println("Please use the POST method!");
        }
    }

    /**
     * Handles the HTTP <code>POST</code> method.
     *
     * @param request  the servlet request.
     * @param response the servlet response.
     * @throws ServletException if a servlet-specific error occurs.
     * @throws IOException if an I/O error occurs.
     */
    @Override
    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        response.setContentType("text/html;charset=UTF-8");

        try (PrintWriter out = response.getWriter()) {
            String firstName = request.getParameter("first_name");
            String lastName = request.getParameter("last_name");
            String email = request.getParameter("email");

            if (firstName.isEmpty()) {
                out.println("The first name is empty.");
                return;
            }

            if (lastName.isEmpty()) {
                out.println("The last name is empty.");
                return;
            }

            if (email.isEmpty()) {
                out.println("The email is empty.");
                return;
            }

            FunnyPerson person = new FunnyPerson();
            person.setFirstName(firstName);
            person.setLastName(lastName);
            person.setEmail(email);

            try {
                DataAccessObject.instance().addPerson(person);
                out.println("Person " + person + " created!");
            } catch (RuntimeException ex) {
                out.println("Error: " + ex.getCause().getMessage());
            }
        }
    }

    /**
     * Returns a short description of the servlet.
     *
     * @return a String containing servlet description
     */
    @Override
    public String getServletInfo() {
        return "This servlet adds new persons to the database.";
    }
}

CreateDatabaseController.java

package net.coderodde.web.db.fun.controllers;

import java.io.IOException;
import java.io.PrintWriter;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

/**
 * This controller creates the database if it is not yet created.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Aug 8, 2017)
 */
@WebServlet(name = "CreateDatabaseController", urlPatterns = {"/create"})
public class CreateDatabaseController extends HttpServlet {


    /**
     * If not yet created, this request creates the database and the table.
     *
     * @param request  the servlet request.
     * @param response the servlet response.
     * @throws ServletException if a servlet-specific error occurs.
     * @throws IOException if an I/O error occurs.
     */
    protected void processRequest(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        response.setContentType("text/html;charset=UTF-8");

        try (PrintWriter out = response.getWriter()) {
            try {
                DataAccessObject.instance().createDatabase();
                out.println("Database created!");
            } catch (RuntimeException ex) {
                out.println("Error: " + ex.getCause().getMessage());
            }
        }
    }

    /**
     * Handles the HTTP <code>GET</code> method.
     *
     * @param request  the servlet request.
     * @param response the servlet response.
     * @throws ServletException if a servlet-specific error occurs.
     * @throws IOException if an I/O error occurs.
     */
    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        processRequest(request, response);
    }

    /**
     * Handles the HTTP <code>POST</code> method.
     *
     * @param request  the servlet request.
     * @param response the servlet response.
     * @throws ServletException if a servlet-specific error occurs.
     * @throws IOException if an I/O error occurs.
     */
    @Override
    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        processRequest(request, response);
    }

    /**
     * Returns a short description of the servlet.
     *
     * @return a String containing servlet description.
     */
    @Override
    public String getServletInfo() {
        return "Creates the database and the table.";
    }
}

DataAccessObject.java

package net.coderodde.web.db.fun.controllers;

import com.mysql.jdbc.jdbc2.optional.MysqlDataSource;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import net.coderodde.web.db.fun.model.FunnyPerson;

public final class DataAccessObject {

    /**
     * For validating the email addresses.
     */
    public static final Pattern VALID_EMAIL_ADDRESS_REGEX = 
    Pattern.compile("^[A-Z0-9._%+-]+@[A-Z0-9.-]+\\.[A-Z]{2,6}$",
                    Pattern.CASE_INSENSITIVE);

    /**
     * The SQL command for inserting a person.
     */
    private static final String INSERT_PERSON_SQL = 
            "INSERT INTO funny_persons (first_name, last_name, email) VALUES " +
            "(?, ?, ?);";

    /**
     * Creates a new database if not already created.
     */
    private static final String CREATE_DATABASE_SQL = 
            "CREATE DATABASE IF NOT EXISTS funny_db;";

    /**
     * Switches to 'funny_db'.
     */
    private static final String USE_DATABASE_SQL = "USE funny_db";

    /**
     * Creates the table if not already created.
     */
    private static final String CREATE_TABLE_SQL =
            "CREATE TABLE IF NOT EXISTS funny_persons (\n" +
                "id INT(6) UNSIGNED AUTO_INCREMENT PRIMARY KEY,\n" +
                "first_name VARCHAR(40) NOT NULL,\n" +
                "last_name VARCHAR(40) NOT NULL,\n" +
                "email VARCHAR(50) NOT NULL,\n" +
                "created TIMESTAMP);";

    /**
     * The SQL for selecting a user given his/her ID.
     */
    private static final String GET_USER_BY_ID_SQL = 
            "SELECT * FROM funny_persons WHERE id = ?;";

    private final MysqlDataSource mysqlDataSource;

    private DataAccessObject(MysqlDataSource mysqlDataSource) {
        this.mysqlDataSource = Objects.requireNonNull(
                mysqlDataSource, 
                "The MysqlDataSource is null.");
    }

    private static final DataAccessObject INSTANCE;

    static {
        MysqlDataSource mysqlDataSource = new MysqlDataSource();
        mysqlDataSource.setUser("root");
        mysqlDataSource.setPassword("your_password");
        mysqlDataSource.setURL("jdbc:mysql://localhost:3306/funny_db");
        INSTANCE = new DataAccessObject(mysqlDataSource);
    }

    public static DataAccessObject instance() {
        return INSTANCE;
    }

    /**
     * Adds a person to the database.
     * 
     * @param person the person to add.
     */
    public void addPerson(FunnyPerson person) {
        checkPerson(person);

        try (Connection connection = mysqlDataSource.getConnection()) {
            try (PreparedStatement statement = 
                    connection.prepareStatement(INSERT_PERSON_SQL)) {
                statement.setString(1, person.getFirstName().trim());
                statement.setString(2, person.getLastName().trim());
                statement.setString(3, person.getEmail().trim());
                statement.executeUpdate();
            }
        } catch (SQLException ex) {
            throw new RuntimeException(ex);
        }
    }

    /**
     * Creates the empty database and the table.
     */
    public void createDatabase() {
        try (Connection connection = mysqlDataSource.getConnection()) {
            try (Statement statement = connection.createStatement()) {
                statement.executeUpdate(CREATE_DATABASE_SQL);
                statement.executeUpdate(USE_DATABASE_SQL);
                statement.executeUpdate(CREATE_TABLE_SQL);
            }
        } catch (SQLException ex) {
            throw new RuntimeException(ex);
        }
    }

    /**
     * Gets a user by his/her ID.
     * 
     * @param id the ID of the user.
     * @return a {@code FunnyPerson} object or {@code null} if there is not such
     *         user.
     */
    public FunnyPerson getUserById(int id) {
        try (Connection connection = mysqlDataSource.getConnection()) {
            try (PreparedStatement statement = 
                    connection.prepareStatement(GET_USER_BY_ID_SQL)) {
                statement.setInt(1, id);

                try (ResultSet resultSet = statement.executeQuery()) {
                    if (!resultSet.next()) {
                        return null;
                    }

                    FunnyPerson person = new FunnyPerson();

                    person.setId(resultSet.getInt("id"));
                    person.setFirstName(resultSet.getString("first_name"));
                    person.setLastName(resultSet.getString("last_name"));
                    person.setEmail(resultSet.getString("email"));
                    person.setCreated(resultSet.getDate("created"));

                    return person;
                }
            }
        } catch (SQLException ex) {
            throw new RuntimeException(ex);
        }
    }

    private void checkPerson(FunnyPerson person) {
        Objects.requireNonNull(person, "The person is null.");
        Objects.requireNonNull(person.getFirstName(), 
                               "The first name is null.");

        Objects.requireNonNull(person.getLastName(), "The last name is null.");
        Objects.requireNonNull(person.getEmail(), "The email is null.");

        if (person.getFirstName().trim().isEmpty()) {
            throw new IllegalArgumentException("The first name is empty.");
        }

        if (person.getLastName().trim().isEmpty()) {
            throw new IllegalArgumentException("The last name is empty.");
        }

        if (!validate(person.getEmail().trim())) {
            throw new IllegalArgumentException("Invalid email address.");
        }
    }

    /**
     * Checks the email address.
     * 
     * @param email the email address to validate.
     * @return {@code true} if {@code email} is a valid email address.
     */
    private static boolean validate(String email ) {
        Matcher matcher = VALID_EMAIL_ADDRESS_REGEX .matcher(email );
        return matcher.find();
    }
}

ShowPersonController.java

package net.coderodde.web.db.fun.controllers;

import com.google.gson.Gson;
import java.io.IOException;
import java.io.PrintWriter;
import javax.servlet.ServletException;
import javax.servlet.annotation.WebServlet;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import net.coderodde.web.db.fun.model.FunnyPerson;

/**
 * This controller is responsible for viewing persons.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Aug 8, 2017)
 */
@WebServlet(name = "ShowPersonController", urlPatterns = {"/show/*"})
public class ShowPersonController extends HttpServlet {


    /**
     * Processes requests for both HTTP <code>GET</code> and <code>POST</code>
     * methods.
     *
     * @param request  the servlet request.
     * @param response the servlet response.
     * @throws ServletException if a servlet-specific error occurs.
     * @throws IOException if an I/O error occurs.
     */
    protected void processRequest(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        response.setContentType("text/html;charset=UTF-8");

        try (PrintWriter out = response.getWriter()) {
            String path = request.getPathInfo();

            if (path.startsWith("/")) {
                path = path.substring(1);
            }

            String[] tokens = path.split("/");

            if (tokens.length == 0) {
                out.println("Error: At least the user ID is required.");
                return;
            }

            String idString = tokens[0];
            int id = -1;

            try {
                id = Integer.parseInt(idString);
            } catch (NumberFormatException ex) {
                out.println("Error: " + idString + " is not an integer.");
                return;
            }

            FunnyPerson person = DataAccessObject.instance().getUserById(id);

            if (person == null) {
                out.println("Error: no person with ID = " + id + ".");
                return;
            }

            String matchFirstName = null;

            if (tokens.length == 2) {
                matchFirstName = tokens[1];
            }

            if (!person.getFirstName().equals(matchFirstName)) {
                response.sendRedirect("/DBWebFun/show/" + id + "/" + 
                                      person.getFirstName());
                return;
            }

            Gson gson = new Gson();
            out.println(gson.toJson(person));
        }
    }

    /**
     * Handles the HTTP <code>GET</code> method.
     *
     * @param request servlet request
     * @param response servlet response
     * @throws ServletException if a servlet-specific error occurs
     * @throws IOException if an I/O error occurs
     */
    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        processRequest(request, response);
    }

    /**
     * Handles the HTTP <code>POST</code> method.
     *
     * @param request servlet request
     * @param response servlet response
     * @throws ServletException if a servlet-specific error occurs
     * @throws IOException if an I/O error occurs
     */
    @Override
    protected void doPost(HttpServletRequest request, HttpServletResponse response)
            throws ServletException, IOException {
        processRequest(request, response);
    }

    /**
     * Returns a short description of the servlet.
     *
     * @return a String containing servlet description
     */
    @Override
    public String getServletInfo() {
        return "Shows the user info via ID/first_name";
    }
}

The entire Maven project is here.

Critique request

Please tell me anything that comes to mind.

Antot · Accepted Answer · 2017-08-10 16:14:19Z

AddPersonController

content type value should be extracted into a constant or taken from an existing list in a related lib (but I'm not sure there is such list of constants in Servlet API).
doGet outputs Please use the POST method! with successful response code. It looks much more like 405 Method not allowed and would better return this response code.
For error output, the response code should be set correctly. Currently, even in case of errors, all requests will look like 200 OK.
doPost method is too long. It does three things: extraction and validation of the args; 2) initialization of person; 3) saving the person. This method should be split.
avoid try blocks nesting.
there are cases of repetitive output of error messages across different classes. It would be a good idea to extract out.println("Error: " + ex.getCause().getMessage()) into a shortcut method.

CreateDatabaseController

both doPost and doGet are bad choice for the action to create a database. Objects creation corresponds to POST verb, so doGet should be removed.

ShowPersonController

processRequest is just too long, please split it in logical parts.
the case is inverse here for doGet and doPost: this controller is used to show persons information, not to create entries. This is the verb GET. So doPost is to remove.

Dmytro Maslenko · Accepted Answer · 2017-08-10 18:25:44Z

+1 for Antot's answer. Some more from my side.

checkPerson method:

private void checkPerson(FunnyPerson person) {
    Objects.requireNonNull(person, "The person is null.");
    Objects.requireNonNull(person.getFirstName(), 
                           "The first name is null.");

    Objects.requireNonNull(person.getLastName(), "The last name is null.");
    Objects.requireNonNull(person.getEmail(), "The email is null.");

    if (person.getFirstName().trim().isEmpty()) {
        throw new IllegalArgumentException("The first name is empty.");
    }

    if (person.getLastName().trim().isEmpty()) {
        throw new IllegalArgumentException("The last name is empty.");
    }

    if (!validate(person.getEmail().trim())) {
        throw new IllegalArgumentException("Invalid email address.");
    }
}

can be simplified by using Apache commons lang Validate:

private void checkPerson(FunnyPerson person) {
    Validate.notNull(person, "The person is null.");
    Validate.notBlank(person.getFirstName(), "The first name is null or blank.");
    Validate.notBlank(person.getLastName(), "The last name is null or blank.");
    Validate.notBlank(person.getEmail(), "The email is null or blank.");
}

In processRequest() you have an the end:

Gson gson = new Gson();
out.println(gson.toJson(person));

Move up this logging and put right after getting of person.

About logging. Avoid println, try to reuse at least embedded Java logging but better look at third party loggers like slf4j, logback, etc.

instead of relying on Apache Commons I'd recommend to rely on JPA and related APis, which would also vastly clean up the DAOs by abstracting the SQL statements away — Vogel612, Commented Aug 11, 2017 at 13:17

Stack Exchange Network

Practicing Java servlets, MySQL and JSON - follow-up

What's new

Code

Critique request

2 Answers 2

AddPersonController

CreateDatabaseController

ShowPersonController

Your Answer

Not the answer you're looking for? Browse other questions tagged
java
mysql
json
controller
servlets
or ask your own question.

Linked

Hot Network Questions

Practicing Java servlets, MySQL and JSON - follow-up

What's new

Code

Critique request

2 Answers 2

AddPersonController

CreateDatabaseController

ShowPersonController

Your Answer

Sign up or log in

Post as a guest

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

Linked

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
java
mysql
json
controller
servlets
or ask your own question.