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

Transport layer #225

Open
wants to merge 23 commits into
base: CLEN-2074
Choose a base branch
from
Open

Transport layer #225

wants to merge 23 commits into from

Conversation

mohitpubnub
Copy link
Contributor

@mohitpubnub mohitpubnub commented Sep 23, 2024

feat(transport layer): Added new implementation for transport middleware.

Tasks Checklist:

  • new execution flow implemented for http request.
  • Update File Upload.
  • Update Subscribe.

Copy link
Contributor

@jakub-grzesiowski jakub-grzesiowski left a comment

Choose a reason for hiding this comment

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

Initial review, mostly pedantry

src/Api/PubnubApi/ClientNetworkStatus.cs Outdated Show resolved Hide resolved
src/Api/PubnubApi/ClientNetworkStatus.cs Outdated Show resolved Hide resolved
src/Api/PubnubApi/EndPoint/Files/SendFileOperation.cs Outdated Show resolved Hide resolved
}
else if (enumValue.ToLowerInvariant() == "uuid_custom")
{
ret = "uuid.custom";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I've seen this conversion in another file, can this method be put once into a util?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's required conversation between C# request object and Rest api query parameter key values, So if something is going to change, we need to update this conversation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean if we can move MapEnumValueToEndpoint method to a static util since it's repeated at endpoints but doesn't seem to have significant variation in how it translates the values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.
Agree! moving it to a common place...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored! at bbd62a4

good catch!

src/Api/PubnubApi/EndPoint/PubSub/SubscribeManager2.cs Outdated Show resolved Hide resolved
jakub-grzesiowski and others added 3 commits October 7, 2024 13:35
* temporarily remove one assert from TestSubscribeDecryptionOnNonEncryptedMessage

* re-enable unit tests on branch

* disable stub test

* remove channels argument exception from UrlRequestBuilder

* temporarily disable tests that were causing the CI to hang

* update key-set used by tests

* Fix broken Encryption tests

* Disable global HereNow tests (for now)

* re-enable access manager dependant tests

* Revert "re-enable access manager dependant tests"

This reverts commit dff644b.

* Test increased timeouts in ThenManageMembershipShouldReturnEventInfo

* Revert "Test increased timeouts in ThenManageMembershipShouldReturnEventInfo"

This reverts commit fc129da.

* Fix potentially faulty async calls in EncryptionTests

* Increase verbosity for CI debugging

* test increasing --logger verbosity

* add a NUnit ConsoleTraceListener

* add a simple debug writeline to test logging

* Update .pubnub.yml for .Net version support (#223)

* fix encryption tests PAM issue

* Fix more of file tests

* add missing test teardowns, replace v2 PAM access grants with v3 ones in tests that got a PAM 403

* change to PAM v3 in WhenObjectChannelMetadata

* failing test debug assert

* add temp debug method for PubnubCommon values

* Change PAM v2 to v3 in more tests, more failing test debug

* update .yml test keys ids

* add debug to failing ci test

* debug on file tests

* tests yml sdk keys change

* remove old test debugs

* Remove deprecated build targets

* Fix missing UWP includes

---------

Co-authored-by: Mohit Tejani <60129002+mohitpubnub@users.noreply.github.com>
* Added support for Patch requests in client library
@mohitpubnub
Copy link
Contributor Author

mohitpubnub commented Oct 9, 2024

@jakub-grzesiowski Please make a note that We have to support HTTP PATCH request for AppContext Apis.
I've added it in contract.

I'm referring this method-

* fix current userId memenership issue for AppContext Api
added request timeout mechanism,
fixes to adhere to existing response generation mechanism by applying request state values as per response
@mohitpubnub
Copy link
Contributor Author

mohitpubnub commented Oct 15, 2024

  • Fix file upload tests
  • Remove commented code
  • Add debug logs in catch clause

@jakub-grzesiowski

mohitpubnub and others added 11 commits October 15, 2024 13:41
…utility directory, moved unnecessary log messages from internet utility method
…er information sent through default http client component
…r handling in default http client library,

removed code for UrlProcess request from PubNubCore class,
fixed issues with generate file url
Code Cleanup for Pubnub, PubnubCoreBase, PNConfiguration class,
Code Cleanup - removed dead codes in various files and removed old PubnubHttpClient footprints.
@mohitpubnub
Copy link
Contributor Author

@jakub-grzesiowski changes at 70011b8 is about serialisation failure in net 6 versions. Because any of conditional compilation clauses not having higher .net version value!!:(

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.

2 participants