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

Manage CCDA data transmission to service #5658

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

sjpadgett
Copy link
Sponsor Member

  • CCM chunk send data to service
  • add data stacking buffer to service
  • honor EOT(file separator character) in service
  • replace deprecated js substr with substring

Fixes #5657

Short description of what this resolves:

Allows sending ccda content xml array for content larger than 128kb. Theoretically this will allow any array size.
Co-Authored with @adunsulag

- CCM chunk send data to service
- add data stacking buffer to service
- honor EOT(file separator character) in service
- repace deprecated js substr with substring
const delimiterIndex = this.buffer.indexOf(this.delimiter);
if (delimiterIndex !== -1) {
const bufferMsg = this.buffer.slice(0, delimiterIndex);
this.buffer = this.buffer.replace(bufferMsg + this.delimiter, "");
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't you get the same effect by just doing
this.buffer = "";
Not sure why you need the this.buffer.replace here.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current use case this may be true. But if I wanted to read the buffer before delimit, I don't want to destroy stack.
This made sense to me. Do you see a potential issue?

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you've already grabbed the bufferMsg in line 46. Line 47, if I'm reading it correctly just sets this.buffer to be an empty string. I didn't see why you need the replace operation when you can just assign this.buffer = ""; on line 47.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just because I'm not currently validating data doesn't mean I won't in future. The intent is to be able to read the current stack at anytime but not destroy stack until the end of file is received.
Being lazy, I probably should have added a new function to scan buffer to avoid confusion.
I could add if you feel strongly

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Say I want to fetch by different delimiter like new line and process received data line by line. I could just return out of receive to a function that pulls from the stack line by line by resetting delimiter from eof to new line i.e FIFO.
Even though this is not the way it is used currently doesn't mean it won't be in future either here or a different workflow using service.

…make clear how fetchBuffer is supposed to work.
Copy link
Sponsor Member

@adunsulag adunsulag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@sjpadgett sjpadgett merged commit 6e4effd into openemr:master Aug 3, 2022
@sjpadgett sjpadgett deleted the ccda-fix branch August 3, 2022 16:49
@sjpadgett
Copy link
Sponsor Member Author

Will pick to v7 branch for patch

sjpadgett added a commit that referenced this pull request Aug 3, 2022
* Manage CCDA data transmission to service
- CCM chunk send data to service
- add data stacking buffer to service
- honor EOT(file separator character) in service
- repace deprecated js substr with substring

* - add reading from stack by a delimiter method for future use. Helps make clear how fetchBuffer is supposed to work.

(cherry picked from commit 6e4effd)
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

Successfully merging this pull request may close these issues.

CCDA generation fails for large documents
2 participants