search

berhir / SignalR.Modules

Modular SignalR with a shared connection using a C# Source Generator
MIT License
35 stars 7 forks source link

Attributes are not included in the result generating #2

Closed bretbas closed 2 years ago

bretbas commented 3 years ago

I generated several some hubs with your package, and i don't invoke any methods on the hub from front-end:

ERROR Error: Failed to invoke 'DisputeHub_GetMessages' due to an error on the server. HubException: Method does not exist.

bretbas commented 3 years ago

I found out why this is happening. The point is that your generator does not add attributes that were set on real methods. I have several methods, and i override all method names:

[Authorize(IdentityPolicy.User)]
        [HubMethodName("GetMessages")]
        public async Task<IList<DisputeMessageResponse>> GetMessagesAsync(int disputeId, int skip = 0, int take = 10)
        {
            var messages = await _disputeMessageManager.GetMessagesAsync(disputeId, skip, take);
            return _mapper.Map<IList<DisputeMessageResponse>>(messages);
        }

So, I have two attribute here - Authorize and HubMethodName. This attributes not be included in the result generating.

berhir commented 3 years ago

You are right, this is on my to-do list, but I didn't have the time yet to implement it.

bretbas commented 3 years ago

I try to implement it myself and make pull request

berhir commented 3 years ago

I took a look today and it's much simpler than I thought. I already have a working solution and can merge it tomorrow.

bretbas commented 3 years ago

I found another error Снимок : Here you use .Name property, but need full type with namespace!

bretbas commented 3 years ago

So, i fixed them. Pull request already created. Please check

berhir commented 3 years ago

thank you @bretbas for your PR. I noticed you made some changes that should be discussed separately. And your solution is more complex than needed. I wanted to implement it the same way as you, but then I noticed that it's enough to use the ToString() method of the attribute. Please take a look at my PR #4. Currently I support only method attributes, but I think class attributes should be supported as well.

bretbas commented 3 years ago

Please check your PR on the attributes with named arguments. For example: [Authorize(Policy = IdentityPolicy.Admin)]

berhir commented 3 years ago

Yes, I tested it and it works

bretbas commented 3 years ago

Good, thanks.

So, can you try to develop SignalR.Modules.Client for javascript, flutter and other clients ?

berhir commented 3 years ago

I merged my PR now, please check if it works for you. There is currently one limitation: if you use the HubMethodName attribute, the name gets not prefixed with the module hub name. I am unsure if the prefix should be added or not.

Regarding other clients, I have currently no plans to add any. Maybe a JavaScript client, but as I said, I don't have any plans yet. If you want to add another client, PRs are very welcome.

bretbas commented 3 years ago

ok, thanks!

berhir commented 3 years ago

@bretbas Can we close this issue?