Bug: Using session.list() with @EmbeddedId causes bad cache hits

Currently, when a session is populating its cache, it does so by extracting
they "key" of an entity, and comparing it to the "key" as read from database
columns. However, this logic only reads a single column, thus, `@EmbeddedId`
keys will falsely match in the cache, resulting in a call to `session.list()`
returning the same cached Entity multiple times instead of reading each row.

While this can be fixed by modifying the `getKey` method of `metadata.EntityInfo`,
this is not a complete solution, because the cache first calls `normalize` on the
`Variant` which is used as a key. `normalize` is not aware of objects, it has
special cases for `long` and `int`, and everything else uses `toString`, meaning
that all `Object` instances (unless they implement `toString`) will have the same
cache key. This is resolved by modifying `normalize` to simply return a `Variant`
containing an object as-is, which relies on `opEquals` being implemented.

This means that, if an `@EmbeddedId` property refers to an `@Embeddable` entity
that does NOT implement `opEquals`, then it will never be considered a cache-hit.
This limitation was added to the documentation and README, so that users wishing
for the performance benefit of the cache remember to implement `opEquals`.

- session.d: `Variant normalize(Variant v)` is used to form session cache keys,
  it was modified to support object keys used by `@EmbeddedId`.
- metadata.d: `EntityInfo.getKey(DataSetReader r, int startColumn)` has been
  renamed to `getKeyFromColumns` to disambiguate it from another overload.
  Added logic to read multiple columns for an `@EmbeddedId` key.
- annotations.d: This is the only file using CRLF line endings, standardize it.

Author: vnayar

README.md

@@ -194,6 +194,11 @@ To represent this using HibernateD, the following code would be used:
 class InvoiceId {
     string vendorNo;
     string invoiceNo;
+
+    // Be sure to implement this to benefit from session caching.
+    bool opEquals(const InvoiceId o) const @safe {
+        return vendorNo == o.vendorNo && invoiceNo == o.invoiceNo;
+    }
 }
 
 @Table("invoices")
@@ -203,10 +208,12 @@ class Invoice {
 }
 ```
 
-**Note**: At the time of writing, there are two important limitations.
+**Note**: At the time of writing, there are important limitations.
 1. The function `DBInfo.updateDbSchema(Connection conn, bool dropTables, bool createTables)`
    does not generate schemas with compound keys.
 2. The Hibernate annotation `@JoinColumns` (plural) has not yet been implemented, thus,
    the `@ManyToOne` and `@ManyToMany` relations are not usable for classes using an
    `@EmbeddedId`.
+3. The `@Embedded` class referenced via an `@EmbeddedId` property should implement `opEquals`
+   in order to gain performance benefits from session caching.
 These features will be added in future updates.

hdtest/source/embeddedidtest.d

@@ -1,5 +1,8 @@
 module embeddedidtest;
 
+
+import std.algorithm : any;
+
 import hibernated.core;
 
 import testrunner : Test;
@@ -12,6 +15,16 @@ class InvoiceId {
     string vendorNo;
     // Vendors independently pick an invoiceNo, which may overlap with other vendors.
     string invoiceNo;
+
+    // Allows session caching to function correctly.
+    bool opEquals(const InvoiceId o) const @safe {
+        return vendorNo == o.vendorNo && invoiceNo == o.invoiceNo;
+    }
+
+    // Useful for debugging.
+    override string toString() const @safe {
+       return vendorNo ~ ":" ~ invoiceNo;
+    }
 }
 
 @Entity
@@ -40,13 +53,21 @@ class EmbeddedIdTest : HibernateTest {
         invoice.invoiceId.invoiceNo = "L1005-2328";
         invoice.currency = "EUR";
         invoice.amountE4 = 120_3400;
-
         InvoiceId c1Id = sess.save(invoice).get!InvoiceId;
         assert(c1Id.vendorNo == "ABC123" && c1Id.invoiceNo == "L1005-2328");
+
+        Invoice invoice2 = new Invoice();
+        invoice2.invoiceId = new InvoiceId();
+        invoice2.invoiceId.vendorNo = "ABC123";
+        invoice2.invoiceId.invoiceNo = "L1005-2329";
+        invoice2.currency = "EUR";
+        invoice2.amountE4 = 80_1200;
+        InvoiceId c2Id = sess.save(invoice2).get!InvoiceId;
+        assert(c2Id.vendorNo == "ABC123" && c2Id.invoiceNo == "L1005-2329");
     }
 
-    @Test("embeddedid.read.query")
-    void readQueryTest() {
+    @Test("embeddedid.read.query.uniqueResult")
+    void readQueryUniqueTest() {
         Session sess = sessionFactory.openSession();
         scope(exit) sess.close();
 
@@ -61,6 +82,20 @@ class EmbeddedIdTest : HibernateTest {
         assert(i1.amountE4 == 120_3400);
     }
 
+    @Test("embeddedid.read.query.list")
+    void readQueryListTest() {
+        Session sess = sessionFactory.openSession();
+        scope(exit) sess.close();
+
+        // A query that only partially covers the primary key.
+        auto r2 = sess.createQuery("FROM Invoice WHERE invoiceId.vendorNo = :VendorNo")
+                .setParameter("VendorNo", "ABC123");
+        Invoice[] i2List = r2.list!Invoice();
+        assert(i2List.length == 2);
+        assert(i2List.any!((Invoice inv) => inv.invoiceId.vendorNo == "ABC123" && inv.invoiceId.invoiceNo == "L1005-2328"));
+        assert(i2List.any!(inv => inv.invoiceId.vendorNo == "ABC123" && inv.invoiceId.invoiceNo == "L1005-2329"));
+    }
+
     @Test("embeddedid.read.get")
     void readGetTest() {
         Session sess = sessionFactory.openSession();

hdtest/source/hibernatetest.d

@@ -11,10 +11,10 @@ import testrunner : Test, BeforeClass, AfterClass;
  */
 struct ConnectionParams {
     string host;
-	ushort port;
+    ushort port;
     string database;
-	string user;
-	string pass;
+    string user;
+    string pass;
 }
 
 /**