Skip to content

Commit

Permalink
fix(pillarbox-monitoring): use capital case to align with spec
Browse files Browse the repository at this point in the history
This fix mainly conforms to the latest version of the specification, as some
values were not in capital case.

- srgssr middleware: rename tracker instance from `srgMonitoring` to
`pillarboxMonitoring`
- srgssr middleware: start a session directly by calling the `sessionStart`
function instead of sending an event
- pillarbox-monitoring: delete `pillarbox-monitoring/sessionstart` event
- pillarbox-monitoring: stringify error log
- srgssr middleware: add test cases for tracker initialization
  • Loading branch information
amtins committed Sep 27, 2024
1 parent 9e47528 commit bf306bd
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 20 deletions.
20 changes: 11 additions & 9 deletions src/middleware/srgssr.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,9 @@ class SrgSsr {
* @returns {Promise<import('./typedef').ComposedSrcMediaData>} - The composed source media data.
*/
static async getSrcMediaObj(player, srcObj) {
player.trigger('pillarbox-monitoring/sessionstart');
if (SrgSsr.pillarboxMonitoring(player)) {
SrgSsr.pillarboxMonitoring(player).sessionStart();
}

const { src: urn, ...srcOptions } = srcObj;
const mediaComposition = await SrgSsr.getMediaComposition(
Expand Down Expand Up @@ -601,30 +603,30 @@ class SrgSsr {
}

/**
* SRG monitoring singleton.
* PillarboxMonitoring monitoring singleton.
*
* @param {import('video.js/dist/types/player').default} player
*
* @returns {PillarboxMonitoring} instance of PillarboxMonitoring
*/
static srgMonitoring(player) {
if (player.options().trackers.srgMonitoring === false) return;
static pillarboxMonitoring(player) {
if (player.options().trackers.pillarboxMonitoring === false) return;

if (!player.options().trackers.srgMonitoring) {
const srgMonitoring = new PillarboxMonitoring(player, {
if (!player.options().trackers.pillarboxMonitoring) {
const pillarboxMonitoring = new PillarboxMonitoring(player, {
debug: player.debug(),
playerVersion: Pillarbox.VERSION.pillarbox,
playerName: 'Pillarbox',
});

player.options({
trackers: {
srgMonitoring,
pillarboxMonitoring,
},
});
}

return player.options().trackers.srgMonitoring;
return player.options().trackers.pillarboxMonitoring;
}

/**
Expand Down Expand Up @@ -665,7 +667,7 @@ class SrgSsr {
* @returns {Object}
*/
static middleware(player) {
SrgSsr.srgMonitoring(player);
SrgSsr.pillarboxMonitoring(player);
SrgSsr.cuechangeEventProxy(player);

return {
Expand Down
16 changes: 6 additions & 10 deletions src/trackers/PillarboxMonitoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class PillarboxMonitoring {
constructor(player, {
playerName = 'none',
playerVersion = 'none',
platform = 'web',
platform = 'Web',
schemaVersion = 1,
heartbeatInterval = 30_000,
beaconUrl = 'http://sse-broker-alb-1501344577.eu-central-1.elb.amazonaws.com/api/events'
Expand Down Expand Up @@ -114,7 +114,6 @@ class PillarboxMonitoring {

this.player.on('loadstart', this.loadStart);
this.player.on('loadeddata', this.loadedData);
this.player.on('pillarbox-monitoring/sessionstart', this.sessionStart);
this.player.on('playing', this.playbackStart);
this.player.on('pause', this.playbackStop);
this.player.on('error', this.error);
Expand All @@ -141,13 +140,11 @@ class PillarboxMonitoring {
* Binds the callback functions to the current instance.
*/
bindCallBacks() {

this.error = this.error.bind(this);
this.loadedData = this.loadedData.bind(this);
this.loadStart = this.loadStart.bind(this);
this.playbackStart = this.playbackStart.bind(this);
this.playbackStop = this.playbackStop.bind(this);
this.sessionStart = this.sessionStart.bind(this);
this.stalled = this.stalled.bind(this);
this.sessionStop = this.sessionStop.bind(this);
}
Expand Down Expand Up @@ -246,12 +243,12 @@ class PillarboxMonitoring {
}

this.sendEvent('ERROR', {
log: error.metadata || JSON
.stringify(pillarbox.log.history().slice(-15)),
log: JSON
.stringify(error.metadata || pillarbox.log.history().slice(-15)),

Check warning on line 247 in src/trackers/PillarboxMonitoring.js

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch
message: error.message,
name: error.code,
...playbackPosition,
severity: 'FATAL',
severity: 'Fatal',
url
});

Expand Down Expand Up @@ -470,7 +467,7 @@ class PillarboxMonitoring {
this.lastPlaybackDuration +=
PillarboxMonitoring.timestamp() - this.lastPlaybackStartTimestamp;

this.lastPlaybackStartTimestamp = undefined;
this.lastPlaybackStartTimestamp = 0;

Check warning on line 470 in src/trackers/PillarboxMonitoring.js

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered

Warning! Not covered statement
}

/**
Expand Down Expand Up @@ -553,7 +550,6 @@ class PillarboxMonitoring {
removeListeners() {
this.player.off('loadstart', this.loadStart);
this.player.off('loadeddata', this.loadedData);
this.player.off('pillarbox-monitoring/sessionstart', this.sessionStart);
this.player.off('playing', this.playbackStart);
this.player.off('pause', this.playbackStop);
this.player.off('error', this.error);
Expand Down Expand Up @@ -657,7 +653,7 @@ class PillarboxMonitoring {
this.sessionStartTimestamp = PillarboxMonitoring.timestamp();
// At this stage currentSource().src is the media identifier
// and not the playable source.
this.mediaId = this.player.currentSource().src;
this.mediaId = this.player.currentSource().src || undefined;

Check warning on line 656 in src/trackers/PillarboxMonitoring.js

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch
}

/**
Expand Down
38 changes: 38 additions & 0 deletions test/middleware/srgssr.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,44 @@ describe('SrgSsr', () => {
});
});

/**
*****************************************************************************
* pillarboxMonitoring *******************************************************
*****************************************************************************
*/
describe('pillarboxMonitoring', () => {
it('should not initialize the pillarboxMonitoring', () => {
player.options().trackers.pillarboxMonitoring = false;

const spyOnOptions = jest.spyOn(player, 'options');

SrgSsr.pillarboxMonitoring(player);

expect(player.options().trackers.pillarboxMonitoring).toBe(false);
expect(spyOnOptions).not.toHaveBeenLastCalledWith(expect.objectContaining({ trackers: { pillarboxMonitoring: expect.any(Object) }}));
});

it('should initialize the pillarboxMonitoring', () => {
player.options().trackers.pillarboxMonitoring = undefined;

const spyOnOptions = jest.spyOn(player, 'options');

SrgSsr.pillarboxMonitoring(player);

expect(spyOnOptions).toHaveBeenNthCalledWith(4, expect.objectContaining({ trackers: { pillarboxMonitoring: expect.any(Object) }}));
});

it('should not reinitialize the pillarboxMonitoring', () => {
player.options().trackers.pillarboxMonitoring = {};

const spyOnOptions = jest.spyOn(player, 'options');

SrgSsr.pillarboxMonitoring(player);

expect(spyOnOptions).not.toHaveBeenLastCalledWith(expect.objectContaining({ trackers: { pillarboxMonitoring: expect.any(Object) }}));
});
});

/**
*****************************************************************************
* updatePoster **************************************************************
Expand Down
2 changes: 1 addition & 1 deletion test/trackers/pillarbox-monitoring.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ describe('PillarboxMonitoring', () => {

monitoring.removeListeners();

expect(spyOnOff).toHaveBeenCalledTimes(8);
expect(spyOnOff).toHaveBeenCalledTimes(7);
expect(spyOnRemoveEventListener).toHaveBeenCalledTimes(1);
});
});
Expand Down

0 comments on commit bf306bd

Please sign in to comment.