-
Notifications
You must be signed in to change notification settings - Fork 4
cyrillic symbols fix for search, emails and attachment #10
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
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # src/java/edu/stanford/muse/email/EmailFetcherThread.java # src/java/edu/stanford/muse/webapp/EmailRenderer.java
|
also improved stability for standalone.jar on computers with low memory (4gb ram - works perfectly) |
| content = new String(b, FORCED_ENCODING); | ||
| } else | ||
| content = (String) p.getContent(); | ||
| content = new String(b, type.substring(type.indexOf("charset=") + "charset=".length())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gleb, thanks for this fix. One comment -- I'm not sure if we can assume charset will always be at the end of the string? I think we should make it robust by tokenizing the contentType string first on ";", trimming the token and then looking for token.substring("charset=").length()
(See Content-Type field spec: https://tools.ietf.org/html/rfc2045#page-12)
| // so we process in batches | ||
| //TODO: Ideally, should cap on buffer size rather than on number of messages. | ||
| final int BATCH = 10000; | ||
| int nMessagesperbathc = 10000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of fixing batch size based on 4GB/8GB limits.
But what should the number be? I think we can easily support 1000 for 4GB, 2000 for 8GB.
Have you experience otherwise?
There is a small disadvantage to small batch sizes, which is that the archive has to be closed and opened multiple times.
Minor typo in var name: bathc -> batch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i tested on different computers approx 10+ times, and it seems ok.
| @@ -0,0 +1,102 @@ | |||
| package edu.stanford.muse.email.json; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gleb, can you pls explain what this email.json package is for?
| // page/message | ||
| page.append("<a rel=\"page" + d.hashCode() + "\" title=\"" + attachment.filename + "\" class=\"" + (highlight?"highlight":"") + "\" href=\"" + attachmentURL + "\">"); | ||
| page.append(leader + "href=\"" + attachmentURL + "\" src=\"" + thumbnailURL + "\"></img>\n"); | ||
| page.append(leader + "href=\"" + attachmentURL + "\" download src=\"" + thumbnailURL + "\"></img>\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason to add the download attribute?
| Pair<String, String> p = JSPHelper.getNameAndURL((InternetAddress) a, addressBook); | ||
| String url = p.getSecond(); | ||
| String str = ia.toString(); | ||
| String str = ia.getPersonal() == null ? ia.getAddress() : ia.getPersonal() + "<" + ia.getAddress() + ">"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is this not what ia.toString() already returns? (I'm not sure)
- should ia.getPersonal() be checked for empty string as well as null (with Util.nullOrEmpty(ia.getPersonal())?
fix for better utf-8 support. (tested on cyrillic)