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

Setting deeper nested JSON objects as a String #2937

Closed
dmaier-redislabs opened this issue Mar 9, 2022 · 8 comments · Fixed by #3005
Closed

Setting deeper nested JSON objects as a String #2937

dmaier-redislabs opened this issue Mar 9, 2022 · 8 comments · Fixed by #3005
Labels
Milestone

Comments

@dmaier-redislabs
Copy link
Contributor

This is more a question / thought for further enhancement:

The current client.jsonSet(key, Path.ROOT_PATH, object) assumes a PoJo as the object. I would like to have an additional method that is client.jsonSet(key, Path.ROOT_PATH, string) that bypasses the JSON serialization. There is a workaround that doesn't feel intuitive that is to use client.jsonSet(key, Path2.ROOT_PATH, object).

@sazzad16
Copy link
Collaborator

@dmaier-redislabs Let's check following example from our test suit:

client.jsonSet("str", ROOT_PATH, "strong");
assertEquals("strong", client.jsonGet("str"));

It clearly sets a plain String by jsonSet method using Path.ROOT_PATH. Not only that, it is able to read a plain String as well. I am really not sure what is it that you are missing and/or asking for.

@dmaier-redislabs
Copy link
Contributor Author

Guess I didn't describe it well enough. I am not talking about plain strings. I am talking about JSON strings. So a JSON object in its String representation. If I remember correctly this is how RedisJSON works natively. You pass a path and a string that represents the JSON, e.g. JSON.SET test $ '{ "value" : "strong"}.

So imagine something like the following:

strong = "{ \"value\" : \"strong\"}"
client.jsonSet("jsonStr", ROOT_PATH, strong);
assertEquals(strong, client.jsonGet("jsonStr"));

This would currently by design fail because of the built-in serialization/de-serialization, which uses the default Gson builder. I would like to use a custom serializer/de-serializer on the raw JSON strings, but this is not totally intuitive with the current implementation. As said, it's more a developer experience/usability thing.

@dmaier-redislabs dmaier-redislabs changed the title Setting JSON as a String Setting deeper nested JSON objects as a String Mar 11, 2022
@sazzad16
Copy link
Collaborator

@dmaier-redislabs As you have already said, client.jsonSet(key, Path2.ROOT_PATH, object) handles that. What part of it you don't feel intuitive?

@dmaier-redislabs
Copy link
Contributor Author

Controlling how the passed over object is handled via the path parameter is semantically not intuitive. A path has semantically nothing to do with how object is serialized or not.

@sazzad16
Copy link
Collaborator

Controlling how the passed over object is handled via the path parameter is semantically not intuitive. A path has semantically nothing to do with how object is serialized or not.

Is this statement after trying both RedisJSON V1 and RedisJSON V2 at the the same time?

Paths do make difference on how json commands are handled.

FYI, Path represents the json path of RedisJSON v1 and Path2 represents the json path of RedisJSON v2.

@dmaier-redislabs
Copy link
Contributor Author

dmaier-redislabs commented Mar 14, 2022

Yeah, I got this now. Maybe I missed it in the documentation or it wasn't totally obvious. Most of the stuff here is more about usability.

It could be just me, but I personally don't like this Path vs. Path2 class approach versioning purposes. Here what I experienced (usability-wise):

  1. I want to set some json, I am doing an auto-completion for client.json* in my IDE
  2. The first block of commands accepts a key and a Path as the arguments. Makes sense.
  3. Let's search for a signature that accepts a String. It's not there, but it allows passing an Object
  4. Tried it out and realized that this ends up as a String that is wrapped by a JsonElement
  5. I looked at the source code and realized that there is some GSON serialization happening when passing a String
  6. I didn't pay any attention of the RedisJSON version that is used behind the scenes, and it's a fair question what should have caught that attention?
  7. Looking deeper into the code showed me then that there is a method signature that fulfills my need.

@sazzad16
Copy link
Collaborator

I feel you. We are all waiting for help in documentation and/or tutorial.

There are a few bigger questions as well. Like:

  1. Should one client support both versions of RedisJSON? Because, there are always going to be some confusions if both versions are being supported at the same time (at least in Java).
  2. Who are the target users of a client? Regular users who are okay with getting a json object. Or the advanced users who wants direct deserialization to their own object? If we target both, what would be method signature for extra methods? Because in Java we can't have two method(..., String regularString, ...) and method(..., String jsonString, ...).

@dmaier-redislabs
Copy link
Contributor Author

dmaier-redislabs commented Mar 14, 2022

Here my 2 pence:

  1. I think that there needs to be a way for the client to figure the version out and then behave accordingly. I think that we should make this as friction-free as possible for developers. The client could maintain some context dependent on which RedisJSON is running on the target side. This would also allow you to keep the same interfaces (on the side of the cient) if something behind the scenes changes (e.g., RESP version, format of the path, ...)
  2. From my point of view, the target users of Jedis are both, regular and advanced users. I see it more as a lower-level client library without sophisticated object-mapping baked-in. It would be nice to be able to easily 'plug' such object mappings/serializers 'in' when needed.

Let me please provide some context and explain what I did that inspired this ticket:

  • I defined my data model first logically. This logical model is reflected via classes. My model has Persons. One person can be a friend of another person. So the Person class has a member 'friends' that has the type Set.
  • Now there is typically a difference between a logical data model (the one the class diagram represents) and the physical data model (the one that is stored in the database). With the default JSON serializer I would end up storing each referenced person embedded to another person, but this is not what I want to store. Instead I would like to embed an array of user id-s for storage purposes.
  • There are indeed multiple ways to achieve this. Either I wrap my Person class which represents the physical data model, or I deal with it at (de-)serialization time of Person objects. I decided for the second option because I was going to implement more a Repository pattern instead of a DAO one.

So my example app will be able to access the friends of a person as Person directly (because the repo returns a person), but behind the scenes the repo deals with a physical model that is realized with the help of a custom (De-)Serializer.

Bottom line: I think that it is not uncommon that developers might want to decide in a flexible way how data is stored vs. how it is logically accessed. In that case the built-in JSON (de-)serialization might not be good enough.

@sazzad16 sazzad16 linked a pull request May 28, 2022 that will close this issue
@sazzad16 sazzad16 added this to the 4.3.0 milestone May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants