odpi / egeria-connector-xtdb

Pluggable repository for Egeria, using XTDB (formerly "Crux") as the back-end to natively support historical metadata.
https://odpi.github.io/egeria-docs/connectors/repository/xtdb/
Apache License 2.0
15 stars 7 forks source link

Implement support for classifications on entity proxies #443

Closed davidradl closed 1 year ago

davidradl commented 1 year ago

As per https://github.com/odpi/egeria/issues/7153

Also the implementation of the new classifyEntity method the @cmgrote added to the XTDB repository connector is not correct. The purpose of these three new methods (and getHomeClassifications) is to allow a repository to store a classification independently of its entity using the entityProxy as a stub to connect it to. The repository needs to look to see if it has the entity and if so, it can use the old method to add/update/delete the classification. If the entity is not stored then it should add the entity proxy to the store with the classification. When the entity finally turns up in the repository via an event typically, the entity proxy is upgraded to an entity, with care that the home classifications are not lost.

cmgrote commented 1 year ago

I suspect this is already fixed in snapshot versions of the connector going back as far as September, per https://github.com/odpi/egeria-connector-xtdb/issues/390 — just haven't done a release of the connector with it included yet 🙈

mandy-chessell commented 1 year ago

@cmgrote Is there a PR? The code is not showing in main. There is only a dummy implementation of the new classifyEntity method that is not performing the right behaviour

mandy-chessell commented 1 year ago

My apologies - the code is there - I was looking at the wrong version ...

cmgrote commented 1 year ago

No worries — here's the PR for reference: https://github.com/odpi/egeria-connector-xtdb/pull/400

The logic of these transaction functions is a bit confusing to follow, but broadly goes like this:

  1. At connector startup, the connector injects (Clojure) transaction functions into the storage layer (XTDB) — just placeholder code that we can run at any point later.
  2. The XtdbOMRSMetadataCollection methods ultimately call a (Java) transaction function using the <function>.transact() method.
  3. These static .transact() methods prepare all the inputs necessary for the (Clojure) transaction function to run and then invoke the Clojure transaction function (Transaction.builder().invokeFunction(...)).
    • The logic of the function is defined in Clojure at the top of each Java transaction function (as a static FN string). Immediately after (fn [... is where you'll see the arguments that the Clojure function requires — this is what the static .transact() method is preparing and passing in step (3).
  4. The .invokeFunction() method actually passes over to XTDB to run the (injected at (1)) Clojure transaction function, passing through the arguments prepared.
  5. The Clojure logic then executes: in the case of these classifyMethods, that includes steps like setting a specific transaction point (for consistency), retrieving any existing entity by its guid, calculating any required updates, and then writing any updates back to XTDB. All of these steps happen atomically thanks to the transaction point being used for consistency.
  6. Typically the "calculating any required updates" step is where the Clojure then passes back to the Java transaction function and calls the constructor of the Java class. These constructors are then where most of the logic exists to validate what we've pulled back from the previous steps in Clojure (the result of retrieving any existing entity), apply various validations, revise the object, etc — all in-memory — and then ultimately store the result back into a member named xtdbDoc. (Or throw an Exception if any error is found during the validations.)
  7. This member xtdbDoc is then usually returned back to the Clojure code.
  8. And the Clojure can then pick-up the results of (6) and write them back to XTDB.
cmgrote commented 1 year ago

I think this is now addressed — but if not, please re-open and let me know what I missed 🙈