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

Middleware Architecture Implementation #132

Merged
merged 25 commits into from
Jan 29, 2019
Merged

Middleware Architecture Implementation #132

merged 25 commits into from
Jan 29, 2019

Conversation

muthurathinam
Copy link
Contributor

@muthurathinam muthurathinam commented Jan 11, 2019 •
edited
Loading

Changes:

  1. Added Support for initializing client with default middleware chain and custom middleware chain
  2. Added AuthProvider Implementation for MSAL.js
  3. Added and organized unit test cases
  4. Reduced output js file size by adding tslib dependency
  5. Added documentation for middleware usage, auth provider usage, tasks, content etc
  6. Added License File
  7. Modified Readme.md file
  8. Added License detail in all files
  9. Added jsDoc comment in all source files
  10. Removed unnecessary public methods buildFullURL, parserPath in GraphRequest and changed some members in LargeFileUploadTask from public to protected
  11. Added new rich samples interface for browser
  12. Updated node samples

Fixes Bugs:

  1. PageIterator does not work on empty collections #129
  2. getStream without callback #128
  3. avoid polyfill fetch and promise #113
  4. getting access token for a service without a user #112
  5. Support for "Retry-After" header? #42

NOTE: Failure of Travis-ci is because of dependency of a UT is in ES6 which is not supported by older versions of Node (Refer: Travis Error)

Reviewers Note: Please ignore the changes in files under lib folder, because they are generated ones.

muthurathinam-m and others added 19 commits December 3, 2018 16:25
@muthurathinam-m
Middleware complete code with unit tests
55d64ef 
Breaking changes proposed:
1. AuthProvider should be an instance of AuthenticationProvider
2. Removing unnecessary public methods like buildFullURL, parsePath
3. Removed Callback support for requests
4. Options that is been passed to Client.init is changed
5. Removed redundant support for OData query params (without $ support)
6. Changed access specifier for large file upload task members to protected
7. Renamed common.ts to Common.ts
Other Changes proposed:
1. Refactored unit test cases
2. Added License information in each file
@muthurathinam
Updating LICENSE
cdff293
@muthurathinam
Update LICENSE
9ee871e
@muthurathinam-m
1. Adding MSAL Authentication provider
73045dd 
2. Removing combined minified graph-js-sdk-core.js
3. Cleaning up Common.ts file
4. Added UT's for Auth provider
@muthurathinam-m
Adapting middleware chain initialization change without breaking current
ffb1b3b 
flow
@muthurathinam-m
Adding missed files with previous commit
b9a42fd
@muthurathinam-m
Added callback support for not breaking the current SDK
718631f
@muthurathinam-m
Changes:
1d31914 
1. Moved user interaction needed UT's under spec/development
2. Added tasks, content, middleware UT's to default UT
3. Exported necessary interfaces for development
4. Moved interfaces in IOptions to separate files like IAuthProvider and
IAuthProviderCallback
@muthurathinam-m
Changes:
fff5730 
1. Adapted web and core output js files which follows any depedency
model
2. Renamed DefaultAuthenticationProvider to CustomAuthenticationProvider
3. Cleaned browserify related codes
4. Added check for availability of Promise and fetch
@muthurathinam-m
New Docs added for library usage guidance
20c103b
@muthurathinam-m
Docs for PageIterator
b3c525b
@muthurathinam-m
Added complete docs CustomMiddleware, CustomAuthProvider, GettingRawR…
d0b58dc 
…esponse. Added Graph request and error handlers
@muthurathinam-m
1. Adding tslib to reduce the size of the output file
39a8d7b 
2. Two MSALAuthenticationProviders one for output js and one for npm
package
@muthurathinam-m
Moving msal to dev dependency
606b8ed
@muthurathinam-m
Moving middleware test folder under development
5e0a40e
@muthurathinam-m
Replacing Array.includes code with Array.indexOf
1f5a84f
@muthurathinam
Merge pull request #131 from microsoftgraph/AuthProvider
b8135ee 
Auth provider Implementation
@muthurathinam
Update THIRD PARTY NOTICES
042b7df
@muthurathinam-m
Bumping up the version
988502c
MIchaelMainer
MIchaelMainer reviewed Jan 22, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
MIchaelMainer
MIchaelMainer reviewed Jan 22, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
MIchaelMainer
MIchaelMainer suggested changes Jan 22, 2019
View reviewed changes
Copy link
Contributor

@MIchaelMainer MIchaelMainer left a comment

Choose a reason for hiding this comment

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

I think we made some mistakes in our design review with regards to the middleware pipeline and maintaining a concept of chained outer and inner handlers. At least that is the impression I'm getting from the examples. We need to provide the updated context back with each returned promise. That context must pass through with the response from the final httpMessageHandler.

pankajpalOlk
pankajpalOlk reviewed Jan 28, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
pankajpalOlk
pankajpalOlk reviewed Jan 28, 2019
View reviewed changes
src/Client.ts Outdated Show resolved Hide resolved
pankajpalOlk
pankajpalOlk reviewed Jan 28, 2019
View reviewed changes
src/GraphRequest.ts Outdated Show resolved Hide resolved
deepak2016
deepak2016 previously approved these changes Jan 28, 2019
View reviewed changes
pankajpalOlk
pankajpalOlk previously approved these changes Jan 28, 2019
View reviewed changes
@muthurathinam muthurathinam dismissed stale reviews from pankajpalOlk and deepak2016 via 93342d2 January 28, 2019 08:14
@muthurathinam muthurathinam merged commit 1522954 into dev Jan 29, 2019
This was referenced Jan 29, 2019
Closed
Closed
@muthurathinam muthurathinam deleted the Middleware branch January 30, 2019 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pankajpalOlk pankajpalOlk pankajpalOlk approved these changes

@MIchaelMainer MIchaelMainer MIchaelMainer approved these changes

@deepak2016 deepak2016 deepak2016 approved these changes

@darrelmiller darrelmiller darrelmiller approved these changes

@dadheech115 dadheech115 Awaiting requested review from dadheech115

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@muthurathinam @darrelmiller @deepak2016 @MIchaelMainer @pankajpalOlk @muthurathinam-m