Error when someone tries to register with duplicate userId

[arushi tomar]

While registering a user, I want to check that if the userId is unique, then only it registers, otherwise it should give an error message, I have written the following code, to check if user exists, but it is still registering a new user with duplicate userId, what am I missing? Thanks.

public class RegisterService {

	public boolean register(User user) {
		Session session = HibernateUtil.getSession();
		if (isUserExists(user))
			return false;
		Transaction tx = null;
		try {
			tx = session.getTransaction();
			tx.begin();
			session.saveOrUpdate(user);
			tx.commit();
		} catch (Exception e) {
			if (tx != null) {
				tx.rollback();
			}
			e.printStackTrace();
		} finally {
			session.close();
		}
		return true;
	}

	public boolean isUserExists(User user) {
		Session session = HibernateUtil.getSession();
		boolean result = false;
		Transaction tx = null;
		try {
			tx = session.getTransaction();
			tx.begin();
			Query query = session.createQuery("from User where userId='" + user.getUserId() + "'");
			User u = (User) query.uniqueResult();
			tx.commit();
			if (u != null)
				result = true;
		} catch (Exception ex) {
			if (tx != null) {
				tx.rollback();
			}
		} finally {
		}
		return result;
	}
}

[Roel De Nijs]

If the code can save a duplicate userId, it seems your database table is missing a primary key constraint or unique constraint on the userId column.

And did you already debug your code to see what happens when a duplicate user is registered? When an exception occurs (e.g. a constraint violation exception) in the register() method, you would expect false as return value but you are currently returning true.

[arushi tomar]

Resolved.

I put constraint manually in the table, and did this:

public boolean register(User user) {
		Session session = HibernateUtil.getSession();
		if (isUserExists(user))
			return false;
		Transaction tx = null;
		try {
			tx = session.getTransaction();
			tx.begin();
			session.saveOrUpdate(user);

			tx.commit();
		} catch (ConstraintViolationException ex) {
			throw ex;
		} finally {
			session.close();
		}
		return true;
	}

and gave pop ups for both fail and success.
Thanks.

[arushi tomar]

Roel De Nijs wrote:If the code can save a duplicate userId, it seems your database table is missing a primary key constraint or unique constraint on the userId column.

And did you already debug your code to see what happens when a duplicate user is registered? When an exception occurs (e.g. a constraint violation exception) in the register() method, you would expect false as return value but you are currently returning true.

Thank you

[Roel De Nijs]

arushi tomar wrote:Resolved.

I put constraint manually in the table, and did this:

Still confused why the code to create a new user is executed while the isUserExists() method is returning true

[arushi tomar]

Roel De Nijs wrote:

arushi tomar wrote:Resolved.

I put constraint manually in the table, and did this:

Still confused why the code to create a new user is executed while the isUserExists() method is returning true

Yeah, even I am, that shouldn't happen. :-/

[Roel De Nijs]

arushi tomar wrote:Yeah, even I am, that shouldn't happen. :-/

That brings us back to my original question: did you already debug your code to see what happens when a duplicate user is registered? Does the isUserExists() method returns true (as you would expect)? Or maybe it returns false for some reason?

[arushi tomar]

Roel De Nijs wrote:

arushi tomar wrote:Yeah, even I am, that shouldn't happen. :-/

That brings us back to my original question: did you already debug your code to see what happens when a duplicate user is registered? Does the isUserExists() method returns true (as you would expect)? Or maybe it returns false for some reason?

It returned true but still entered the part of saving user, I resolved it before I could check the problem in deep though. I will check it once again after I am done with the other work, and update the solution here.
Thanks :-)

[xsunil kumar]

Interesting .... Please let us know, whey it was going to save even if isUserExists() method returns true.

[Dave Tolls]

Since you have to handle the situation where a userId exists during the insert operation anyway, there's little point in the isUserExist method.
I'd just get rid of it and let the db tell you of the ConstraintViolation (which you're handling) when you try the insert.