Associate remote participant w/ JitsiConference (_UPDATED)

The commit message of "Associate remote participant w/ JitsiConference
(_JOINED)" explains the motivation for this commit.

Practically, _JOINED and _LEFT combined with "Remove remote participants
who are no longer of interest" should alleviate the problem with
multiplying remote participants to an acceptable level of annoyance.

Technically though, a remote participant cannot be identified by an ID
only. The ID is (somewhat) "unique" in the context of a single
JitsiConference instance. So in order to not have to scratch our heads
over an obscure corner, racing case, it's better to always identify
remote participants by the pair id-conference. Unfortunately, that's a
bit of a high order given the existing source code. So I've implemented
the cases which are the easiest so that new source code written with
participantUpdated is more likely to identify a remote participant with
the pair id-conference.

Additionally, the commit "Reduce direct read access to the
features/base/participants redux state" brings more control back to the
functions of the feature base/participants so that one day we can (if we
choose to) do something like, for example:

If getParticipants is called with a conference, it returns the
participants from features/base/participants who are associated with the
specified conference. If no conference is specified in the function
call, then default to the conference which is the primary focus of the
app at the time of the function call. Added to the above, this should
allow us to further reduce the cases in which we're identifying remote
participants by id only and get us even closer to a more "predictable"
behavior in corner, racing cases.
This commit is contained in:
Lyubo Marinov 2018-05-22 16:47:43 -05:00
parent 37cd5bb5b9
commit 771d60f954
8 changed files with 75 additions and 21 deletions

View File

@ -1804,9 +1804,9 @@ export default {
APP.UI.participantConnectionStatusChanged(id);
});
room.on(JitsiConferenceEvents.DOMINANT_SPEAKER_CHANGED, id => {
APP.store.dispatch(dominantSpeakerChanged(id));
});
room.on(
JitsiConferenceEvents.DOMINANT_SPEAKER_CHANGED,
id => APP.store.dispatch(dominantSpeakerChanged(id, room)));
if (!interfaceConfig.filmStripOnly) {
room.on(JitsiConferenceEvents.CONNECTION_INTERRUPTED, () => {
@ -1883,6 +1883,7 @@ export default {
= displayName.substr(0, MAX_DISPLAY_NAME_LENGTH);
APP.store.dispatch(participantUpdated({
conference: room,
id,
name: formattedDisplayName
}));
@ -1916,6 +1917,7 @@ export default {
switch (name) {
case 'raisedHand':
APP.store.dispatch(participantUpdated({
conference: room,
id: participant.getId(),
raisedHand: newValue === 'true'
}));
@ -2015,6 +2017,7 @@ export default {
APP.UI.addListener(UIEvents.EMAIL_CHANGED, this.changeLocalEmail);
room.addCommandListener(this.commands.defaults.EMAIL, (data, from) => {
APP.store.dispatch(participantUpdated({
conference: room,
id: from,
email: data.value
}));
@ -2026,6 +2029,7 @@ export default {
(data, from) => {
APP.store.dispatch(
participantUpdated({
conference: room,
id: from,
avatarURL: data.value
}));
@ -2035,6 +2039,7 @@ export default {
(data, from) => {
APP.store.dispatch(
participantUpdated({
conference: room,
id: from,
avatarID: data.value
}));
@ -2579,6 +2584,12 @@ export default {
const localId = localParticipant.id;
APP.store.dispatch(participantUpdated({
// XXX Only the local participant is allowed to update without
// stating the JitsiConference instance (i.e. participant property
// `conference` for a remote participant) because the local
// participant is uniquely identified by the very fact that there is
// only one local participant.
id: localId,
local: true,
email: formattedEmail
@ -2606,6 +2617,12 @@ export default {
}
APP.store.dispatch(participantUpdated({
// XXX Only the local participant is allowed to update without
// stating the JitsiConference instance (i.e. participant property
// `conference` for a remote participant) because the local
// participant is uniquely identified by the very fact that there is
// only one local participant.
id,
local: true,
avatarURL: formattedUrl
@ -2662,6 +2679,12 @@ export default {
}
APP.store.dispatch(participantUpdated({
// XXX Only the local participant is allowed to update without
// stating the JitsiConference instance (i.e. participant property
// `conference` for a remote participant) because the local
// participant is uniquely identified by the very fact that there is
// only one local participant.
id,
local: true,
name: formattedNickname

View File

@ -125,13 +125,14 @@ function _addConferenceListeners(conference, dispatch) {
conference.on(
JitsiConferenceEvents.DISPLAY_NAME_CHANGED,
(id, displayName) => dispatch(participantUpdated({
conference,
id,
name: displayName.substr(0, MAX_DISPLAY_NAME_LENGTH)
})));
conference.on(
JitsiConferenceEvents.DOMINANT_SPEAKER_CHANGED,
(...args) => dispatch(dominantSpeakerChanged(...args)));
id => dispatch(dominantSpeakerChanged(id, conference)));
conference.on(
JitsiConferenceEvents.PARTICIPANT_CONN_STATUS_CHANGED,
@ -155,18 +156,21 @@ function _addConferenceListeners(conference, dispatch) {
conference.addCommandListener(
AVATAR_ID_COMMAND,
(data, id) => dispatch(participantUpdated({
conference,
id,
avatarID: data.value
})));
conference.addCommandListener(
AVATAR_URL_COMMAND,
(data, id) => dispatch(participantUpdated({
conference,
id,
avatarURL: data.value
})));
conference.addCommandListener(
EMAIL_COMMAND,
(data, id) => dispatch(participantUpdated({
conference,
id,
email: data.value
})));

View File

@ -143,7 +143,10 @@ function _overwriteLocalParticipant(
if ((avatarURL || email || name)
&& (localParticipant = getLocalParticipant(getState))) {
const newProperties: Object = { id: localParticipant.id };
const newProperties: Object = {
id: localParticipant.id,
local: true
};
if (avatarURL) {
newProperties.avatarURL = avatarURL;
@ -264,7 +267,10 @@ function _undoOverwriteLocalParticipant(
if ((avatarURL || name || email)
&& (localParticipant = getLocalParticipant(getState))) {
const newProperties: Object = { id: localParticipant.id };
const newProperties: Object = {
id: localParticipant.id,
local: true
};
if (avatarURL === localParticipant.avatarURL) {
newProperties.avatarURL = undefined;

View File

@ -22,17 +22,23 @@ import { getLocalParticipant } from './functions';
* Create an action for when dominant speaker changes.
*
* @param {string} id - Participant's ID.
* @param {JitsiConference} conference - The {@code JitsiConference} associated
* with the participant identified by the specified {@code id}. Only the local
* participant is allowed to not specify an associated {@code JitsiConference}
* instance.
* @returns {{
* type: DOMINANT_SPEAKER_CHANGED,
* participant: {
* conference: JitsiConference,
* id: string
* }
* }}
*/
export function dominantSpeakerChanged(id) {
export function dominantSpeakerChanged(id, conference) {
return {
type: DOMINANT_SPEAKER_CHANGED,
participant: {
conference,
id
}
};

View File

@ -127,9 +127,7 @@ export function getLocalParticipant(stateful: Object | Function) {
* @private
* @returns {(Participant|undefined)}
*/
export function getParticipantById(
stateful: Object | Function,
id: string) {
export function getParticipantById(stateful: Object | Function, id: string) {
const participants = _getAllParticipants(stateful);
return participants.find(p => p.id === id);

View File

@ -63,20 +63,19 @@ MiddlewareRegistry.register(store => next => action => {
case DOMINANT_SPEAKER_CHANGED: {
// Ensure the raised hand state is cleared for the dominant speaker.
const { conference, id } = action.participant;
const participant = getLocalParticipant(store.getState());
if (participant) {
const { id } = action.participant;
store.dispatch(participantUpdated({
participant
&& store.dispatch(participantUpdated({
conference,
id,
local: participant.id === id,
raisedHand: false
}));
}
typeof APP === 'object'
&& APP.UI.markDominantSpeaker(action.participant.id);
typeof APP === 'object' && APP.UI.markDominantSpeaker(id);
break;
}

View File

@ -32,12 +32,24 @@ import { LOCAL_PARTICIPANT_DEFAULT_ID, PARTICIPANT_ROLE } from './constants';
declare var APP: Object;
/**
* These properties should not be bulk assigned when updating a particular
* @see Participant.
* The participant properties which cannot be updated through
* {@link PARTICIPANT_UPDATED}. They either identify the participant or can only
* be modified through property-dedicated actions.
*
* @type {string[]}
*/
const PARTICIPANT_PROPS_TO_OMIT_WHEN_UPDATE
= [ 'dominantSpeaker', 'id', 'local', 'pinned' ];
const PARTICIPANT_PROPS_TO_OMIT_WHEN_UPDATE = [
// The following properties identify the participant:
'conference',
'id',
'local',
// The following properties can only be modified through property-dedicated
// actions:
'dominantSpeaker',
'pinned'
];
/**
* Listen for actions which add, remove, or update the set of participants in

View File

@ -505,6 +505,12 @@ class Toolbox extends Component<Props> {
const { _localParticipantID, _raisedHand } = this.props;
this.props.dispatch(participantUpdated({
// XXX Only the local participant is allowed to update without
// stating the JitsiConference instance (i.e. participant property
// `conference` for a remote participant) because the local
// participant is uniquely identified by the very fact that there is
// only one local participant.
id: _localParticipantID,
local: true,
raisedHand: !_raisedHand