Sprawdzenie czy baza zwraca wartości

Siemka, od dwóch dni próbuję uporać się ze sprawdzeniem, czy w bazie danych znajduje się już dany użytkownik.
Na ten moment kod odpowiadający za to wygląda tak:

    public boolean checkUserAlreadyExistsInDb(String id, String server) {
        try {
            if (statement.isClosed()) {
                openStatement();
            }

            ResultSet rs = statement.executeQuery("SELECT * FROM `" + dbTableName + "` WHERE `id` LIKE '" + id.trim() + "' AND `flags` = 'abc' AND `server` = '" + server.trim() + "'");

            if (rs.next()) {
                return true;
            }
        } catch (SQLException e) {
            log.error(e.getMessage());
        }

        return false;
    }

Problem polega na tym, że sprawdzenie zwraca ZAWSZE false, nawet kiedy dany użytkownik (ID w postaci Stringa) znajduje się już w bazie.
Próbowałem dla testu wrzucić pętlę while zliczającą ile razy użytkownik pojawia się w bazie, lecz także zawsze wychodzi 0.

Jakieś pomysły/propozycje? :/

1. Używaj bind variables, bo inaczej będą z Ciebie śmiać w najlepszym wypadku, a w gorszym będziesz miał aplikację podatną na "sql injection"
2. Możesz użyć zliczania: select count(0) as ilosc ... wówczas zawsze dostaniesz rekord, w którym wystąpi 0 lub >0 (obecnie jak nie znajduje użytkownika to co niby ma Ci zwrócić?)

eydjca:
3) Jesteś pewien że nie dostajesz wyjątku, który logujesz i po sprawie? (I dlatego false..) ?

yarel napisał(a):

1. Używaj bind variables, bo inaczej będą z Ciebie śmiać w najlepszym wypadku, a w gorszym będziesz miał aplikację podatną na "sql injection"
2. Możesz użyć zliczania: select count(0) as ilosc ... wówczas zawsze dostaniesz rekord, w którym wystąpi 0 lub >0 (obecnie jak nie znajduje użytkownika to co niby ma Ci zwrócić?)

eydjca:
3) Jesteś pewien że nie dostajesz wyjątku, który logujesz i po sprawie? (I dlatego false..) ?

1/ Dobrze wiedzieć, trafna uwaga ^^
2/ I prawdopodobnie tutaj właśnie pojawiał się problem.
3/ O tyle było to dziwne, że nic w logach nie było.

Na ten już wszystko działa, poniżej wrzucam poprawny kod.

    public boolean checkUserAlreadyExistsInDb(String id, String server) {
        PreparedStatement preparedStatement;

        try {
            String sql = "SELECT COUNT(*) as COUNT_TMP FROM " + dbName + "." + dbTableName + " WHERE `identity` LIKE '" + id + "' AND `flags` LIKE 'abc' AND `serwer` = '" + server + "'";
            int count = 0;

            connection.setAutoCommit(false);
            preparedStatement = connection.prepareStatement(sql);

            ResultSet resultSet = preparedStatement.executeQuery();

            if (resultSet.next()) {
                count = resultSet.getInt("COUNT_TMP");
            }

            preparedStatement.close();
            connection.setAutoCommit(true);

            return count >= 1;
        } catch (SQLException e) {
            log.error("Nie udało się sprawdzić, czy użytkownik istnieje w bazie danych (" + e.getMessage() + ")");
        }

        return false;
    }

Jeżeli powinienem coś tutaj jeszcze poprawić, proszę dać znać :D
Wydaje mi się, że ten autoCommit niepotrzebny, ale raczej nie przeszkadza w niczym i nikomu ;p

1. ustawianie autocommita - jaki jest cel? Jak nie wiesz, to po co ustawiasz? W tym przypadku jest bez sensu.
2. Nie zamykasz "result seta" - masz taki konstrukt jak "try-with-resources", doczytaj co to takiego i poszukaj jak może prosto wyglądać zwalnianie "prepared statement" / "result seta".
3. Ustawianie int count=0 możesz wyciągnąć przed blok try, wówczas będziesz miał metodę tylko z jednym punktem wyjścia : return count >= 1.
4. Jak Ci się wywali na operacji bazodanowej, to uznasz, że użytkownika nie ma w bazie, co niekoniecznie musi być zgodne ze stanem faktycznym. Nie wiem czy masz jakiś pomysł na obsługę takich sytuacji w aplikacji.

Ten punkt warto przemyśleć. W szczególności jak chciałbyś podejść do rozwiązania:

• obiektowo (rzucasz wyjątek i gdzieś go tam wyżej obsługujesz),
• funkcyjnie (zwracasz jakiegoś Either<Error,Result> i Twój kod ma szansę stać się zwięzły),
• proceduralnie (zwracasz obiekt i obskakujesz go ifami).

Najlepiej doczytaj o każdym z podejść, przećwicz i zobacz jak Ci się sprawdza.

Jeżeli powinienem coś tutaj jeszcze poprawić, proszę dać znać

Nadal masz tam sqlinjection przecież. Użyłeś niby preparedStatement ale nie bindujesz zmiennych tylko dalej kleisz stringi na pałe. Wyślij jako id do tej swojej funkcji wartość dupa' or '1'='1 -- hacked i zobacz że funkcja nagle zacznie zwracać same true. A można zrobić i więcej, można by w ogóle zdumpować ci całą bazę tą funkcją, ale trochę by to potrwało bo nie mamy żadnego echa, więc trzeba by wyciągać dane timingiem, wrzucać do query sleep i patrzeć czy się wykonuje.

Poza tym logika tej funkcji w ogóle jest też zrypana. Czemu masz tam like na tym id a nie =? Tu nawet nie trzeba być hackerem, wysyłasz id % i nagle zawsze dostaniesz true, o ile w bazie jest jakikolwiek rekord.