Fix bug where query logic uses @OneToMany referenced entity's own ID …

[vnayar]

…rather than referencing entity's id.

Modify example1 to use differing ID names for Customer and User.

Closes #77.

[SingingBush]

thanks for the PR. Does the change source/hibernated/session.d fix the issue without needing a change to the examples? Ideally I'd like to see both variations used in the examples to prove that it works either way. I need to give the github actions a kick to make sure this PR triggers a build/test workflow

[vnayar]

thanks for the PR. Does the change source/hibernated/session.d fix the issue without needing a change to the examples? Ideally I'd like to see both variations used in the examples to prove that it works either way. I need to give the github actions a kick to make sure this PR triggers a build/test workflow

Indeed it does fix the issue and does not require any changes to the examples. I tested it both ways, and now both cases work. I'll take a moment to see if I can find a way to either have the example demonstrate both methods (I think it's good to have examples of the explicit case too), and barring that, I'll move my case to a unittest.

[vnayar]

I'm having trouble running the tests locally. Do you have any suggestions for the following?

$ dub test --config=SQLite
             Generating test runner configuration 'hibernated-test-SQLite' for 'SQLite' (staticLibrary).
    Starting Performing "unittest" build using /usr/bin/dmd for x86_64.
  Up-to-date ddbc 0.5.8: target for configuration [SQLite] is up to date.
    Building hibernated ~master: building configuration [hibernated-test-SQLite]
DDBC will use SQLite driver
source/hibernated/dialects/mysqldialect.d(105,1): Error: class `hibernated.dialect.Dialect` at source/hibernated/dialect.d(23,10) conflicts with enum `ddbc.core.Dialect` at ../../.dub/packages/ddbc-0.5.8/ddbc/source/ddbc/core.d(345,1)

[SingingBush]

I'm having trouble running the tests locally. Do you have any suggestions for the following?

$ dub test --config=SQLite
             Generating test runner configuration 'hibernated-test-SQLite' for 'SQLite' (staticLibrary).
    Starting Performing "unittest" build using /usr/bin/dmd for x86_64.
  Up-to-date ddbc 0.5.8: target for configuration [SQLite] is up to date.
    Building hibernated ~master: building configuration [hibernated-test-SQLite]
DDBC will use SQLite driver
source/hibernated/dialects/mysqldialect.d(105,1): Error: class `hibernated.dialect.Dialect` at source/hibernated/dialect.d(23,10) conflicts with enum `ddbc.core.Dialect` at ../../.dub/packages/ddbc-0.5.8/ddbc/source/ddbc/core.d(345,1)

Oh that's annoying. I added a Dialect enum to ddbc and tagged release today. Probably best to pin the previous version of ddbc for now. I will take a look tomorrow

[vnayar]

Oh that's annoying. I added a Dialect enum to ddbc and tagged release today. Probably best to pin the previous version of ddbc for now. I will take a look tomorrow

Ok great, thanks a bunch. I've tried several versions of DDBC, and 0.5.7 has a different error:

  Up-to-date ddbc 0.5.7: target for configuration [full] is up to date.
    Building hibernated 0.3.9: building configuration [hibernated-test-full]
DDBC will use SQLite driver
DDBC will use PGSQL driver
DDBC will use MySQL driver
DDBC will use ODBC driver
Hibernated will log using 'std.logger'.
source/hibernated/tests.d(280,39): Error: undefined identifier `SQLITE_TESTS_ENABLED`
source/hibernated/tests.d(280,63): Error: undefined identifier `MYSQL_TESTS_ENABLED`
source/hibernated/tests.d(280,86): Error: undefined identifier `PGSQL_TESTS_ENABLED`

0.5.6 has quite a few more errors, so I suspect the divergence only grows from there.

[SingingBush]

I've fixed master now and it's working with the latest DDBC. Can you rebase your branch and try again

[vnayar]

I've fixed master now and it's working with the latest DDBC. Can you rebase your branch and try again

I've done a pull of the latest master, rebased onto it, and modified the ddbc dependency to depend on "~master" instead of "~>0.5.8". This is what I currently see:

$ git rev-parse HEAD
d02157d11b1dbee35d2de7fdba9f1526a1535b7c

$ $ dub test
     Warning Selected package ddbc ~master does not match the dependency specification ~>0.5.8 in package hibernated. Need to "dub upgrade"?
             Generating test runner configuration 'hibernated-test-full' for 'full' (staticLibrary).
    Starting Performing "unittest" build using /usr/bin/dmd for x86_64.
  Up-to-date derelict-util 2.0.6: target for configuration [library] is up to date.
  Up-to-date derelict-pq 2.2.0: target for configuration [library] is up to date.
  Up-to-date mysql-native 3.1.0: target for configuration [library] is up to date.
  Up-to-date odbc 1.0.0: target for configuration [library] is up to date.
  Up-to-date ddbc ~master: target for configuration [full] is up to date.
    Building hibernated 0.3.8+commit.3.gd02157d: building configuration [hibernated-test-full]
DDBC will use SQLite driver
DDBC will use PGSQL driver
DDBC will use MySQL driver
DDBC will use ODBC driver
Hibernated will log using 'std.logger'.
source/hibernated/tests.d(280,39): Error: undefined identifier `SQLITE_TESTS_ENABLED`
source/hibernated/tests.d(280,63): Error: undefined identifier `MYSQL_TESTS_ENABLED`
source/hibernated/tests.d(280,86): Error: undefined identifier `PGSQL_TESTS_ENABLED`
Error /usr/bin/dmd failed with exit code 1.

Using 0.5.8 strictly has the same error.

[SingingBush]

I just realised you are running dub test. That's probably been in a bad way for a while as hibernated is generally developed by building the project then running it against the various supported databases (dub test is not run in the GitHub Action). See the github action for how the integration tests are run (eg: running against a Postgres docker image):

cd ./hdtest
dub build --config=PGSQL
./bin/hdtest --host=127.0.0.1 --port=$PORT --database=hdtest --user=testuser --password=passw0rd

I found the cause of the issue. Those values were defined in ddbc but the code was commented out back in October 2022. It wasn't clear that it was used for tests in Hibernated. For now perhaps it's worth setting them as you need locally with immutable bool SQLITE_TESTS_ENABLED = true;

The thought process is that unittest blocks within the codebase were being used for integration testing which isn't ideal. Moving forward any tests within the codebase should be proper unit tests with no reliance on a database. Any tests that require a database should be done in the separate project that can be used to test each db type.

[vnayar]

Thank you kindly for the instructions on how to run the tests.

While going through the code, I accidentally ran into a segfault induced by an infinite loop in PropertyInfo.opEquals. I attempted to modify the test cases to reproduce the case, but this proved to be more difficult than anticipated.

The code has changed since the version where I found the bug, but the logical statement reached was here:

    private FromClauseItem findRelation(FromClauseItem from, const PropertyInfo prop) {
        for (int i=0; i<query.from.length; i++) {
            FromClauseItem f = query.from[i];
            if (f.base == from && f.baseProperty == prop)
                return f;
            if (f.entity == prop.referencedEntity && from.base == f)
                return f;
        }
        return null;
    }

Specifically the part f.baseProperty == prop causes a call to opEquals, which in turn calls itself:

    bool opEquals(ref const PropertyInfo s) const {
        return this == s;
    }

I added a fix, but unfortunately I'm struggling to reproduce the code in an isolated case.

[SingingBush]

thanks vnayar. Would it be possible to add a test that covers the scenario