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

Lua cjson library #558

Closed
ivan-kripakov-m10 opened this issue Nov 8, 2024 · 11 comments · Fixed by #561
Closed

Lua cjson library #558

ivan-kripakov-m10 opened this issue Nov 8, 2024 · 11 comments · Fixed by #561

Comments

@ivan-kripakov-m10
Copy link

ivan-kripakov-m10 commented Nov 8, 2024

hi!
There are some pre-imported libraries available in the Redis Lua runtime context that are not available in the Jedis mock Lua runtime context (source)

The following standard Lua libraries are available to use:

The String Manipulation (string) library
The Table Manipulation (table) library
The Mathematical Functions (math) library
The Operating System Facilities (os) library

In addition, the following external libraries are loaded and accessible to scripts:

The struct library
The cjson library
The cmsgpack library
The bitop library

As I understand, the Lua context is configured here:

globals.set("unpack", globals.load("return table.unpack(...)").checkfunction());
globals.set("redis", globals.load(REDIS_LUA).call().checktable());
globals.set("KEYS", embedLuaListToValue(args.subList(0, keysNum)));
globals.set("ARGV", embedLuaListToValue(args.subList(keysNum, args.size())));
globals.set("_mock", CoerceJavaToLua.coerce(new LuaRedisCallback(state)));

And in this method implementation:

public static Globals standardGlobals() {
    Globals globals = new Globals();
    globals.load(new JseBaseLib());
    globals.load(new PackageLib());
    globals.load(new Bit32Lib());
    globals.load(new TableLib());
    globals.load(new StringLib());
    globals.load(new CoroutineLib());
    globals.load(new JseMathLib());
    globals.load(new JseIoLib());
    globals.load(new JseOsLib());
    globals.load(new LuajavaLib());
    LoadState.install(globals);
    LuaC.install(globals);
    return globals;
}

So, only external libs are unsupported at the moment.
I'm ready to add them, but I want to discuss the approach first

I would like to present the one I've found for cjson as an example:

  1. Extend the org.luaj.vm2.lib.TwoArgFunction:
class LuaCjsonLib extends TwoArgFunction {

    @Override
    public LuaValue call(LuaValue modname, LuaValue env) {
        LuaTable cjson = new LuaTable();
        cjson.set("encode", new encode());
        cjson.set("decode", new decode());
        env.set("cjson", cjson);
        env.get("package").get("loaded").set("cjson", cjson);
        return cjson;
    }

    static class encode extends OneArgFunction {

        @Override
        public Varargs invoke(LuaValue arg) {
            // details omitted
        }
    }

    static class decode extends OneArgFunction {
        @Override
        public Varargs invoke(LuaValue arg) {
            // details omitted
        }
    }
}
  1. Configure via:
globals.set("cjson", globals.load(new LuaCjsonLib()));

While this approach can work, it would require to map all possible org.luaj.vm2.LuaValue inheritors to Java types, which seems to me a little bit complicated, but possible and good enough.

Are there any other approaches I should consider?

@inponomarev
Copy link
Collaborator

Hi @ivan-kripakov-m10 , thanks for your interest in JedisMock! Yeah I was waiting for somebody to come and complain about missing cjson or bitop, and now this day has come :-)

it would require to map all possible org.luaj.vm2.LuaValue inheritors to Java types,

I did not immerse deeply into the problem but it looks wrong to me. Isn't it something we could avoid, as in the end what we get back into Redis is just a combination of string, number, table, and boolean?

Have a closer look at how globals.set("redis", globals.load(REDIS_LUA).call()); works -- what it does is just loading a text file which provides redis.call, redis.pcall, redis.sha1hex and other stuff directly in the form of Lua script. Can we do something similar in order to import cjson? Of course this involves adding cjson Lua source code to JediMock directly.

@ivan-kripakov-m10
Copy link
Author

ivan-kripakov-m10 commented Nov 9, 2024

Got it! It appears that the Redis cjson library is not available as a Lua file: https://github.com/redis/redis/blob/fdeb97629ef8964a5d9040328ee63734884ac874/deps/lua/src/lua_cjson.c#L1346

In Redis, the cjson library is implemented using a similar approach to the one I described above.

UPD: I will try to figure out how to link this C source to JedisMock Lua Runtime Context.

@inponomarev
Copy link
Collaborator

Do I get this right that as cjson (as its name suggests) is a C library, in order to support it in Java world, we need to basically reimplement it?

@ivan-kripakov-m10
Copy link
Author

ivan-kripakov-m10 commented Nov 9, 2024

I'm not entirely sure.
What I do understand is that we have two options: we can either re-implement it or attempt to link this library via the Native API (which may be a smaller re-implementation) using the approach I've described.
However, one area I'm uncertain about is whether similar approach can be used with LuaJ.

Do I get this right that as cjson (as its name suggests) is a C library, in order to support it in Java world, we need to basically reimplement it?

@ivan-kripakov-m10
Copy link
Author

In other words, does LuaJ expose API to link the C source files to LuaJ Runtime Context?

However, one area I'm uncertain about is whether similar approach can be used with LuaJ.

And also there is another option: find some Lua json library and (possibly) modify it (if it will be incompatible with cjson in some cases).

So, generally, I see 5* options:

  1. Implement jjson using Java capabilities
  2. Implement jjson using Native API calls to cjson (in my opinion, is losing to the 1st option)
  3. Pick another Lua json library and add it to Lua Runtime Context via Globals#load
  4. Write JedisMock own Lua Json Lib (is losing to 3rd option)
  5. Figure out if it possible to link C source files to LuaJ Runtime Context*

5th option seems to be impossible if you take a look at Globals#load or Compiler#compile Java Docs:

Load the content form a reader as a text file. Must be lua source.

Interface for module that converts lua source text into a prototype.

From my perspective, this essentially narrows it down to two main options:

  1. Implement jjson directly in Java
  2. Use another Lua-based JSON library and add it to the Lua runtime context with Globals#load

Since you suggested linking a Lua library in your comment, I can look into some libraries and come back with a list:)

@inponomarev
Copy link
Collaborator

inponomarev commented Nov 9, 2024

Thanks for the thorough analysis, @ivan-kripakov-m10 !

I remember that I came to this problem when we implemented Lua support, so I decided to wait for someone who needs one of these libraries in the first place, so that we could properly prioritize the support of one of them. As individual libraries from this list (struct, cjson,cmsgpack, bitop) will require individual approach.

So let's focus on cjson now.

Ideally, it would be the best if we had a pure Lua implementation -- this would have required the least amount of work. If it's incompatible with Redis implementation in some details, we coud've fixed it, but in general JSON parsing should be more or less the same everywhere.

Or maybe we could find a Java implementation or quickly write one ourselves. All this requires research, thank you for your willingness to do this.

This was referenced Nov 9, 2024
@inponomarev
Copy link
Collaborator

inponomarev commented Nov 11, 2024

Hi @ivan-kripakov-m10 , thanks for the investigation. My thoughts so far:

  • As I see, both approaches have known limitations in regards of support of null values in lists/maps. I believe it's ok, Jedis-Mock project itself is nothing more than a "good enough" reimplementation of Redis full of "known limitations". So in my view, if your implementation

  • In terms of lines of code, Java-Based cjson lib #561 is a substantially smaller addition than Add Lua-based cjson support #560. Also, java-based solution involves Java coding only, thus it looks like preferrable for JedisMock, as I we are not Lua coders here :-)

@inponomarev inponomarev changed the title Lua Runtime libraries Lua cjson library Nov 11, 2024
@ivan-kripakov-m10
Copy link
Author

ivan-kripakov-m10 commented Nov 11, 2024

Hi @inponomarev!
Thank you for the review!

I also prefer the Java-based approach since I understand it better :)
I’ll try to find time to polish the PR within the next 1-2 days. I’ll also look into the Redis native tests, rewrite them in Java, and add them to the PR.

@inponomarev
Copy link
Collaborator

Thank you @ivan-kripakov-m10 for the PR, expect the new Maven release soon.

@ivan-kripakov-m10
Copy link
Author

thanks, @inponomarev!

@inponomarev
Copy link
Collaborator

Fixed in v1.1.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants