ocilo / skype-http

Unofficial Skype API for Node.js via HTTP
https://ocilo.github.io/skype-http
MIT License
51 stars 24 forks source link

Type check conversations and events #87

Open demurgos opened 6 years ago

demurgos commented 6 years ago

This is a large PR introducing a change similar to the contact API: it moves the focus of the library on abstracting HTTP calls and guaranteeing the return values, and minimizing renaming or other normalizations.

The immediate benefit is that the skype events are now more rich: the API exposes the all the available data. One of the downsides is that there is no more conversationId on message events: you have a conversationLink instead and have to retrieve the id manually. Next PRs should adds helpers to facilitate this.

demurgos commented 6 years ago

@grigori-gru

As I see you're going to remove checking EventMessage.resourceType in the next version? original comment

I'm responding here for context.

The change around the events is that the API would only emit two kinds of events: error and event:

    this.messagesPoller = new MessagesPoller(this.io, this.context);
    this.messagesPoller.on("error", (err: Error) => this.emit("error", err));

(source)

Currently, there are 4 events: error, event, Text and RichText. The idea originally was to emit specific events so you could do api.on("Event/Call", ev => ...) to only handle calls for example. This never got fully implemented so you had to use the catch-all event event. The solution was to either add specific events on the API for each possible Skype event or just have the catch-all event (the current middle ground with a catch-all and 2 specific events is not optimal). Updating all the api.on("Text", ev => ...) and api.on("RichText", ev => ...) by api.on("event", ev => ...) shouldn't be a big change: I believe you already had to use the catch-all event anyway.

The bigger change is around the event object. error events still pass an Error instance, but the event event now passes a SkypeEvent object instead of an EventMessage. Or with code:

// Current version
api.on("event", (event: EventMessage) => {...});

// Version in this PR
api.on("event", (event: SkypeEvent) => {...});

This change means two things:

Now, how do you deal with these new Skype events? As you can see in the SkypeEvent definition, there are three main categories of events: EndpointPresenceEvent, UserPresenceEvent and NewMessageEvent. You can differentiate them by looking at their resourceType property (which is an EventResourceType enum value). UserPresenceEvent and EndpointPresenceEvent are related to the active sessions of your contacts. The first one represents online contacts regardless of their endpoint, the second on is about endpoints: a user can be online but connected with both his phone and computer at the same time for example (one user presence but two endpoints), these events are currently dropped. NewEventMessage is the closest to the current events, they hold a resource which are one of the possible MessageResource, which you can differentiate by looking at messageType (this corresponds to the current event.resource.type).

To clarify:

// Curent version
api.on("event", (event: EventMessage) => {
  // "ConversationUpdate" or "NewMessage"
  if (event.resourceType === "ConversationUpdate") {
    console.log("Received conversation update");
  } else { // "NewMessage"
    switch (event.resource.type) {
      case "RichText":
        console.log(event.resource.from); // A parsed user ID (MRI key)
        console.log(event.resource.content);
        console.log(event.resource.clientId); // Message ID assigned by the client
      default:
        // ...
    }
  }
});

// Version in this PR:
api.on("event", (event: SkypeEvent) => {
  switch (event.resourceType) {
    case EventResourceType.EndpointPresenceEvent:
      console.log("Connection to a new endpoint");
      break;
    case EventResourceType.UserPresenceEvent:
      console.log("New user connection");
      break;
    case EventResourceType.NewMessageEvent:
      switch (event.resource.messageType) {
        case "RichText":
          console.log(event.resource.from); // An absolute URL (a helper should be provided for this)
          console.log(event.resource.content);
          console.log(event.resource.clientMessageId); // Message ID assigned by the client (renamed)
        default:
          // ...
      }
      break;
  }
});

This PR needs at least the following changes:

grigori-gru commented 6 years ago

@demurgos Thanks for detailed explanation! I'd like to add some types to MessageResource. For example "ThreadActivity/AddMember" messagetype. How can I help you with it? Should I wait for closing this pullrequest or do smth else?

demurgos commented 6 years ago

I think the easiest way to work on this would be to merge this first and then but I don't think I'll have time to finish it immediately. It will probably be done by the end of February.

Having most of the message types figured out would be great: it will probably be the main focus of the next version.

I started to collect some examples of JSON messages. Until this PR is merged, could you post here some examples of events if you have them? (I usually swap some characters in identifiers and keys just in case).