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

Added if condition for hasAttachments #6

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

aarahmanqa
Copy link
Contributor

No description provided.

Copy link
Owner

@shivam1608 shivam1608 left a comment

Choose a reason for hiding this comment

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

Edit this and the pull request to issue #8 with the required changes for now

long aSize = Long.parseLong(object.get("size").toString());
String aDownloadUrl = object.get("downloadUrl").toString();
attachments.add(new Attachment(aId , aFilename , aContentType , aDisposition, aTransferEncoding ,aRelated ,aSize , aDownloadUrl ,bearerToken));
if(hasAttachments) {
Copy link
Owner

Choose a reason for hiding this comment

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

 if(hasAttachments) {
            JSONArray attachmentArray = (JSONArray) parser.parse(json.get("attachments").toString());
            Object[] aArray = attachmentArray.toArray();

            for (Object attachmentObject : aArray) {
                JSONObject object = (JSONObject) attachmentObject;
                 ... 
                attachments.add(new Attachment(aId, aFilename, aContentType, aDisposition, aTransferEncoding, aRelated, aSize, aDownloadUrl, bearerToken));
            }
        }

here it would be better if you do it like this .. instead of wrapping it in whole if statement you can have the array as empty or filled.
it would not loop with 0 elements and will make the code more readable

          JSONArray attachmentArray = (JSONArray) parser.parse(json.get("attachments").toString());
          Object[] aArray = new Object[0];
          if(hasAttachments) {
                 aArray = attachmentArray.toArray();
          }
          
          for (Object attachmentObject : aArray) {
              JSONObject object = (JSONObject) attachmentObject;
               ... 
              attachments.add(new Attachment(aId, aFilename, aContentType, aDisposition, aTransferEncoding, aRelated, aSize, aDownloadUrl, bearerToken));
          }

@aarahmanqa
Copy link
Contributor Author

@shivam1608 , the issue is exactly at
json.get("attachments")
attachments field is null leading to NPE. In the suggested change, we would still get NPE.

@shivam1608
Copy link
Owner

shivam1608 commented Oct 30, 2023

@aarahmanqa sorry my bad I was using the web interface to edit I forgot to copy the json.get() Inside of if statement you can work around that I think you get what I am trying to say

@aarahmanqa
Copy link
Contributor Author

@shivam1608, I have modified it as you had suggested. Kindly check and let me know if any deviation.

Copy link
Owner

@shivam1608 shivam1608 left a comment

Choose a reason for hiding this comment

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

Nice work 👍. @aarahmanqa thanks for contribution

@shivam1608 shivam1608 merged commit 2645137 into shivam1608:main Oct 30, 2023
@shivam1608
Copy link
Owner

Merged 👍will be included in next stable version.

@shivam1608 shivam1608 added the bug Something isn't working label Oct 30, 2023
@jagadeesh1035
Copy link

This is sort of an important and critical bug fix and it is needed to released.

shivam1608 added a commit that referenced this pull request Jun 14, 2024
* Update README.md

* Update README.md

* Update README.md

* Update README.md

* ignore eclipse project file

* bugfix in example(generating random domain)

Domains.getRandomDomain() returns an Domain object.
It does not override toString(), so
"thisismychoice@"+Domains.getRandomDomain() will be like
thisismychoice@me.shivzee.util.Domain@7fb4f2a9
This cause following Exception :
javax.security.auth.login.LoginException: Something went wrong while
creating account! Try Againjavax.security.auth.login.LoginException:
Account Already Exists! Error 422
	at me.shivzee.util.JMailBuilder.createAndLogin(JMailBuilder.java:94)

To fix this, I called getDomainName() to get real domain name, and it
worked fine. :)

* add .gitignore for eclipse

Sorry, since I'm in army, all  I can use in here is just Eclipse
Portable XD

* FunctionalInterface annotation because a lambda can be WorkCallback

* add Synchronous methods

* name Threads

* change version number

* fixed pr3

Added Reusability of Code & Updated Java Docs

* Updated Version

* Update README.md

* Added EventListener

* Fix:Patch Request

* Added Checks

* Added: EventListener

* Fix: Naming Convention

* Removed Old Docs

* Added EventListener

* Update: Docs

* Update: EventListener

* Update: Java Docs

* Update: Version

* Add: JMailTM.jar

* Update README.md

* Added: onAccountUpdate Event

* Update: Bind to Logger

* Fixed: Documentation

* Updated: Documentation

* Updated: Version

* Updated: Version

* 1. Added status code 429 Exception
2. Added created/updated time in ZonedDateTime format

* Updated as per comment

* Updated: Version

* Added: Examples Directory

* Added: Login with Token

* Updated: JavaDocs

* Update README.md

* Added if condition for hasAttachments (#6)

* Added if condition for hasAttachments

* As per review comments, modified the if block to minimal

---------

Co-authored-by: aarahman <aarahman@tekion.com>

* Null checks on fields (#7)

* Added if condition for hasAttachments

* Added safeEval() so that if any field is null calling toString will not cause issue

* Changed the primitive data type to Wrapper type in POJO classes

---------

Co-authored-by: aarahman <aarahman@tekion.com>

* Update readme tags

---------

Co-authored-by: awidesky <awidesky@naver.com>
Co-authored-by: VigneshwaranT <vigneshwarant@tekion.com>
Co-authored-by: Ahamed Abdul Rahman <ahamedabdulrahman@gmail.com>
Co-authored-by: aarahman <aarahman@tekion.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants