moov-io / ach

ACH implements a reader, writer, and validator for Automated Clearing House (ACH) files. The HTTP server is available in a Docker image and the Go package is available.
https://moov-io.github.io/ach/
Apache License 2.0
452 stars 150 forks source link

feat: try out golang.org/x/mobile for generating Java JNI bindings #1327

Closed adamdecaf closed 5 months ago

adamdecaf commented 9 months ago

https://docs.google.com/document/d/1y9hStonl9wpj-5VM-xWrSTuEJFUAxGOXOhxvAs7GZHE/edit

See: https://blog.dogan.io/2015/08/15/java-jni-jnr-go/ See: https://pkg.go.dev/golang.org/x/mobile/cmd/gobind Related: https://github.com/golang/go/issues/34885

Remaining Items

Notes

codecov-commenter commented 9 months ago

Codecov Report

Merging #1327 (6b92979) into master (5957ca4) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 6b92979 differs from pull request most recent head 10e244c. Consider uploading reports for the commit 10e244c to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1327 +/- ## ======================================= Coverage 89.02% 89.02% ======================================= Files 75 75 Lines 7689 7689 ======================================= Hits 6845 6845 Misses 502 502 Partials 342 342 ```
midepeter commented 6 months ago

can i get to hop on this @adamdecaf ?

adamdecaf commented 6 months ago

Yea go for it!

midepeter commented 6 months ago

@adamdecaf

I am trying to get a clear description of this very PR. What we are trying to achieve here it to use golang.org/x/mobile to generate Java JNI bindings so they will be able to use ach library exported functions from java android applications right.

Also, I just ran the two gobind commands you had in the initial commit that you places in the makefile

  1. the first gobind gobind -lang=java -outdir ./java/ github.com/moov-io/ach/internal/... worked
  2. there were some errors on the second one gobind -lang=java -outdir ./java/ github.com/moov-io/ach/ and this is because we are having three returned types for the function signatures .SegmentFile .NextEntry.

Was this the main problem you faced while working on it. I am just trying to get the state of the PR and what needs to be achieved.

adamdecaf commented 6 months ago

Yes that's pretty much where I ended up as well. Java is a popular language so if we could offer native support (without having to rewrite Go to Java) that would help a lot of potential users of the library.

We probably have to write some middleware to convert functions like SegmentFile so they're compatible with Java / JNI. The Java interface would have a slightly different API than Go, but would be largely compatible.

midepeter commented 6 months ago

Yes,

that’s the constraint. Not sure I understand how exactly the middleware should be built. If you can shed more light to that

But the temporary thing I did was to duplicate the functions (SegmentFileJNI and NextEntryJNI) and now embed the return values in a single struct to align with gobind restrictions. It worked and generated successfully but the problem is that there is no way to ignore the normal function or suppress the errors from the normal functions during the bindings.

adamdecaf commented 6 months ago

I think we will want to create a new package which calls functions from moov-io/ach but returns a struct of the multiple values like you did. That way we generate a single package and avoid trying to generate known bad code.

adamdecaf commented 6 months ago
package javainterop

type EntryDetail ach.EntryDetail // reference every type

type File ach.File 

type SegmentFileResponse struct {
    CreditEntryFile *File
    DebitEntryFile *File 
}
func (f *File) SegmentFile(_ *SegmentFileConfiguration) (SegmentFileResponse, error)

Ideally I'd like to generate this or at least compare the API surface between moov-io/ach and the javainterop package so we can ensure every function is ported over.

midepeter commented 6 months ago

Alright

I will work on that.