Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javers integration not working properly for Entities with Long IDs #246

Closed
OmarHawk opened this issue Apr 4, 2024 · 3 comments
Closed

Comments

@OmarHawk
Copy link
Contributor

OmarHawk commented Apr 4, 2024

Hi,

For Entities with Long IDs this view does not work after restarting the application (empty Javers cache) and already having these entries in database:
image
It creates a request that produces an internal server error (500)

It took me quite some time to figure out the root cause for this issue and it has something to do with how Javers processes IDs and stores these in the jv_global_id table and then how it processes request for data in that table and how this blueprint makes use of that.

image
As you can see here, String IDs (like UUIDs) are quoted within the database table. Long ones are not quoted. That is happening completely in the background, i.e. nothing, the blueprint here can influence.

The problem is now with requests for showing changelog details.
The method is this one:

* fetches a previous version for for an entity class and id
*
* @return
*/
@RequestMapping(value = "/audits/entity/changes/version/previous",
method = RequestMethod.GET,
produces = MediaType.APPLICATION_JSON_VALUE)
@PreAuthorize("hasRole(\"" + AuthoritiesConstants.ADMIN + "\")")
public ResponseEntity<EntityAuditEvent> getPrevVersion(@RequestParam(value = "qualifiedName") String qualifiedName,
@RequestParam(value = "entityId") String entityId,
@RequestParam(value = "commitVersion") Long commitVersion)
throws ClassNotFoundException {
Class entityTypeToFetch = Class.forName("<%=packageName%>.domain." + qualifiedName);
QueryBuilder jqlQuery = QueryBuilder.byInstanceId(entityId, entityTypeToFetch)
.limit(1)
.withVersion(commitVersion - 1);
EntityAuditEvent prev = EntityAuditEvent.fromJaversSnapshot(javers.findSnapshots(jqlQuery.build()).get(0));
return new ResponseEntity<>(prev, HttpStatus.OK);
}

This one gets entityId as a String type, no matter, what the original entity's ID type was and passes that String on to Javers. Javers in turn passes this Object type further on, and converts it somehwhere internally to a JSON String to pass that further on into a prepared statement. So to the prepared statement, also Long / String is being passed for execution, which also contains the mentioned quotes in the String case, but since we do pass everything as String, also for the Long-ID type entities. A query log demonstrates the problem:

The red one is the query for a Long-ID based entity, while the geen one is the UUID-ID (String) based entity:image

Therefore, nothing is being found and we run into an java.lang.IndexOutOfBoundsException on this line in the get(0):

EntityAuditEvent prev = EntityAuditEvent.fromJaversSnapshot(javers.findSnapshots(jqlQuery.build()).get(0));

(!) Please note in can only be reproduced when clearing the Javers internal cache for Global IDs to enforce loading it from database, by i.e. restarting the application. The internal cache itself is sort of resistent against this issue.


Possible solution: Pass in the ID in the proper type (Long/String) for the respective entity

@OmarHawk OmarHawk changed the title Javers integration not working properly for Entities Long IDs Javers integration not working properly for Entities with Long IDs Apr 4, 2024
@OmarHawk
Copy link
Contributor Author

OmarHawk commented Apr 4, 2024

For my local installation I made a quick & dirty fix that patches the generated code in the mentioned method like this:

        Object typedId;
        try {
            typedId = Long.valueOf(entityId);
        } catch (NumberFormatException e) {
            typedId = entityId;
        }

        QueryBuilder jqlQuery = QueryBuilder
            .byInstanceId(typedId, entityTypeToFetch)
            .limit(1)
            .withVersion(commitVersion - 1);

But I think, this can be done much cleaner probably

@OmarHawk
Copy link
Contributor Author

I think you fixed this with #241 @mshima ?

@mshima
Copy link
Member

mshima commented Apr 16, 2024

I think you fixed this with #241 @mshima ?

👍

@mshima mshima closed this as completed Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants