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

Imagecashletter REST service X9 conversion enhancement #259

Open
smithbk opened this issue Sep 19, 2022 · 4 comments
Open

Imagecashletter REST service X9 conversion enhancement #259

smithbk opened this issue Sep 19, 2022 · 4 comments

Comments

@smithbk
Copy link
Contributor

smithbk commented Sep 19, 2022

What were you trying to do?
The current REST APIs support an object store model for transforming X9 files to JSON and vice-versa. While this approach has it purposes, it does expose potential security and scalability concerns in specific use cases. Our suggestion and proposed contribution is to extend the existing REST API to support in-memory transformations on-demand. In particular, a single REST call would take one format and return the other format in the response, rather than requiring multiple calls to upload, return, and delete representations.

What did you expect to see?
Propose two additional REST APIs similar to:
a) to_json - which reads an X9 file and returns JSON
b) to_x9 - which reads JSON and returns an X9 file

What are we proposing?
We have a working implementation which wraps the existing library. We are proposing that we port this code to the existing REST service.

@atonks2
Copy link
Collaborator

atonks2 commented Sep 27, 2022

If I'm understanding you correctly, the general goal is to be able to convert between formats without persisting anything in memory?

I haven't dug into this at a code level yet, but I wonder if we could just add one endpoint and leverage the standard Content-Type and Accept headers for this. text/plain indicates x9, application/json for JSON.

@smithbk
Copy link
Contributor Author

smithbk commented Sep 27, 2022

@atonks2 Correct, that is the general goal. Sure, a single endpoint is fine and using the HTTP headers to infer the conversion source and destination formats makes sense, but I think application/octet-stream is what I've seen used for binary data rather than text/plain (unless I'm missing something). Also, since additional metadata may be needed to describe a format, I would still recommend allowing a "from" and "to" section in the body of the POST. For example, X9 can be EBCDIC or ASCII. And we may need an option in the future to filter out the image data in the JSON, e.g.

{
   "from": {
       "x9_format": "EBCDIC" | "ASCII"
   },
   "to": {
       "exclude": ["imageViewData"]
   }
}

BTW, can you tell me what the "hacktoberfest" label on this issue means?

@atonks2
Copy link
Collaborator

atonks2 commented Sep 27, 2022

I suppose we could do that. I suggested the headers because that's how we already handle this in our projects. The encoding (EBCDIC vs ASCII) is currently static (configured in the code):

h := r.Header.Get("Content-Type")
if strings.Contains(h, "application/json") {
file, err := imagecashletter.FileFromJSON(bs)
if err != nil {
err = logger.LogErrorf("error creating file from JSON: %v", err).Err()
moovhttp.Problem(w, err)
return
} else {
req = file
}
} else {
f, err := imagecashletter.NewReader(bytes.NewReader(bs), imagecashletter.ReadVariableLineLengthOption(), imagecashletter.ReadEbcdicEncodingOption()).Read()
if err != nil {
err = logger.LogErrorf("error reading image cache letter: %v", err).Err()
moovhttp.Problem(w, err)
return
} else {
req = &f
}
}

That said, I can see the benefits of making these behaviors configurable instead of limiting a running instance of ICL to only one encoding.

Hacktoberfest is an event put on by Digital Ocean to encourage people to get involved with Open Source projects. We add that label to certain issues in our repos to identify them as good candidates for people wishing to contribute as part of the event. See https://hacktoberfest.com/

@zayscue
Copy link

zayscue commented Oct 24, 2023

Any update on this issue?

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

No branches or pull requests

4 participants