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.

Sign up for GitHub

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

Jump to bottom

feat!: make Twilio not singleton #675

Closed
wants to merge 40 commits into from
Closed

feat!: make Twilio not singleton #675

wants to merge 40 commits into from

Conversation

SimonOsnes
Copy link

Fixes #650

Major version update.
Twilio class is no longer static singleton. Make it possible to have multiple instances of Twilio within the same program. Implement new TwilioApi.
Also update tests and example code.

Add few testcases to RequestValidatorTest.java as per issue #645

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

emilmyresten and others added 30 commits March 4, 2022 11:35
@emilmyresten
docs: add unfinished report to repo (#2)
f742ad7
@emilmyresten
docs: change report to correct one (#2)
5d0ee1f
@emilmyresten
Branch/emil into main (#6)
5ec80d1 
* feat: twilio implements twilioapi, changed usage examples

* feat: change initialization of all tests (#3)

* refactor: moved static block in twilio.java (#5)

* refactor: changed this to static methods (#5)

* feat: twilio class and interface final (#1 #5)

* refactor: change synchronized back to original version (#3)
@emilmyresten
merge: pr #14, creator now takes object reference instead of static m…
33d7dc3 
…ethod (#7)

* git: update gitignore

* feat: updated creator abstract methods to take Twilio object (#7)

* fix: update examples to use twilio object as argument in create-method (#7)
@emilmyresten
merge: PR #15 update usage of .create() to .create(tw) in all tests
ab2861a 
* test: replace .create() with .create(tw) to conform to new usage (#7)

Fixes #7
@SimonOsnes
merge: PR #16, feat: update Updater to take in Twilio (#13)
41c4ff9 
Fixes #13
@emilmyresten
merge: PR #18: fix bug in .setUsername method (#17)
9f0651f 
* fix: bug in setUsername method (#17)
@emilmyresten
test: add multitenancy test and cleanup testclass (#19)
e9f1b4e
@SimonOsnes
merge: PR #21, test: change all occurences of .update() to .update(tw…
3e40e09 
…) in tests (#21)

* test: update new method call .update() to .update(tw) (#13)
@ViktorAryd
feat: update Fetcher method update() to update(Twilio) #11
c40893c
@ViktorAryd
test: change tests to use updated method call update(Twilio) #11
6192b3a
@SimonOsnes
merge: PR #20 add multitenancy test and cleanup TwilioTest.java (#19)
ff07500
@SimonOsnes
merge: PR #22
f9f5dc0 
feat: update Fetcher class methods to take Twilio object and changes tests accordingly (#11)
@emilmyresten
UML diagram added to report-group8.md (#2)
b5d101a
@emilmyresten
docs: add UML-before refactoring to report (#2)
06715bf
@joakim-olsson
feat: updates Deleter to take in non-singleton twilio instance (#8)
dd1eb0c
@joakim-olsson
test: update new method call signature to take in instance of twilio (#8
158fd06 
)
@joakim-olsson
docs: adds essence to repo report (#2)
a05cf77 
docs: adds essence to repo report (#2)
@joakim-olsson
merge: PR #24: update Delete to take in Twilio (#8)
3559638
@jovaanchan
feat: update methods in Reader and CallReaderExample to take in Twili…
ecb5421 
…o tw object. (#11)
@jovaanchan
test: change read tests to use Twilio instance (#11)
8af7acc
@emilmyresten
fix: renamed two testmethods that had been renamed faulty in refactor (…
bc0d329 
…#13)
@SimonOsnes
test: remove @mocked for twilio class in tests (#23)
b90e615
@SimonOsnes
chore: add logs before and after patch
f38ff24
@SimonOsnes
docs: update javadocs where method params changed (#25)
75b368c
@emilmyresten
docs: git diff, overall experience, emils time(#2)
de96212
@SimonOsnes
docs: update README.md to not use .init()
ae8a731
@SimonOsnes
merge: solving merge conflict
739f3eb
@joakim-olsson
docs: adds design pattern discussion to repo report (#2)
64db532
@joakim-olsson
test: adds additional RequestValidator test cases
e7c3192
@SimonOsnes SimonOsnes closed this Nov 14, 2022
@Earth-Turtle
Copy link

Is there a reason this pull request was closed? Allowing the Twilio client to be an object instead of static singleton makes it easier to mock and manage initialization.

In particular, trying to create a reusable object by using the TwilioRestClient.Builder, you have to supply the username and password immediately on creating the builder, where it can no longer be changed once the rest client is built. This clashes with the ability to call Twilio.setUsername() on the static instance whenever needed.

@SimonOsnes
Copy link
Author

@Earth-Turtle

This PR was made as a school project and when after we submitted it, we did not receive any feedback or anything for like 8 months before I closed it. If you want to re-open it and continue the work that we started you are more than welcome to do it. We won't be doing anything more to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Java library should not use singletons
6 participants
@SimonOsnes @Earth-Turtle @emilmyresten @ViktorAryd @joakim-olsson @jovaanchan