Skip to content

Commit

Permalink
Upgrade libraries (oppia#13867)
Browse files Browse the repository at this point in the history
* Upgrade backend libs

* Upgrade frontend libs

* Fix backend libs

* Fix tests

* Fix tests

* Add newline

* Fix typescript errors

* Fix frontend errors

* Fix FE tests

* Regenerate requirements

* Regenerate requirements

* Fix fe coverage

* Fix fe tests

* Fix tests

* Upgrade libs

* Fix e2e tests
  • Loading branch information
vojtechjelinek authored Sep 23, 2021
1 parent 94ae3ad commit 17711ee
Show file tree
Hide file tree
Showing 33 changed files with 960 additions and 789 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*.swp
*.swo
*.bak
__pycache__/
gae_runtime/*
# Ignore the auto-generated protobuf python code.
proto_files/*.py
Expand Down
4 changes: 2 additions & 2 deletions core/controllers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import base64
import datetime
import functools
import hmac
import json
import logging
Expand All @@ -36,7 +37,6 @@
import python_utils
import utils

import backports.functools_lru_cache
import webapp2

from typing import Any, Dict, Optional # isort: skip
Expand All @@ -58,7 +58,7 @@
)


@backports.functools_lru_cache.lru_cache(maxsize=128)
@functools.lru_cache(maxsize=128)
def load_template(filename):
"""Return the HTML file contents at filepath.
Expand Down
5 changes: 2 additions & 3 deletions core/controllers/learner_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ class LearnerDashboardHandler(base.BaseHandler):
@acl_decorators.can_access_learner_dashboard
def get(self):
"""Handles GET requests."""
(
learner_progress, number_of_nonexistent_activities) = (
learner_progress_services.get_activity_progress(self.user_id))
(learner_progress, number_of_nonexistent_activities) = (
learner_progress_services.get_activity_progress(self.user_id))

completed_exp_summary_dicts = (
summary_services.get_displayable_exp_summary_dicts(
Expand Down
8 changes: 5 additions & 3 deletions core/domain/learner_progress_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -1815,8 +1815,7 @@ def get_activity_progress(user_id):
('TopicSummaryModel', topic_ids_to_learn),
('ExpSummaryModel', exploration_playlist_ids),
('CollectionSummaryModel', collection_playlist_ids),
('TopicSummaryModel', all_topic_ids),
('TopicSummaryModel', untracked_topic_ids)
('TopicSummaryModel', all_topic_ids)
]))

incomplete_exploration_models = activity_models[0]
Expand All @@ -1830,7 +1829,10 @@ def get_activity_progress(user_id):
exploration_playlist_models = activity_models[8]
collection_playlist_models = activity_models[9]
all_topic_models = activity_models[10]
untracked_topic_models = activity_models[11]
untracked_topic_models = [
topic_model for topic_model in activity_models[10]
if topic_model.id in untracked_topic_ids
]

incomplete_exp_summaries = (
[exp_fetchers.get_exploration_summary_from_model(model)
Expand Down
4 changes: 2 additions & 2 deletions core/platform/taskqueue/cloud_taskqueue_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def create_http_task(
task: Dict[str, Any] = {
# Specify the type of request.
'app_engine_http_request': {
'http_method': tasks_v2.types.target_pb2.HttpMethod.POST,
'http_method': tasks_v2.types.HttpMethod.POST,
'relative_uri': url,
}
}
Expand Down Expand Up @@ -111,7 +111,7 @@ def create_http_task(
# Note: retry=retry.Retry() means that the default retry arguments
# are used. It cannot be removed since then some failures that occur in
# Taskqueue API are not repeated.
response = CLIENT.create_task(parent, task, retry=retry.Retry())
response = CLIENT.create_task(parent=parent, task=task, retry=retry.Retry())

logging.info('Created task %s' % response.name)
return response
6 changes: 2 additions & 4 deletions core/platform/taskqueue/cloud_taskqueue_services_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ def mock_create_task(
task,
{
'app_engine_http_request': {
'http_method': (
tasks_v2.types.target_pb2.HttpMethod.POST),
'http_method': tasks_v2.types.HttpMethod.POST,
'relative_uri': dummy_url,
'headers': {
'Content-type': 'application/json'
Expand Down Expand Up @@ -124,8 +123,7 @@ def mock_create_task(
task,
{
'app_engine_http_request': {
'http_method': (
tasks_v2.types.target_pb2.HttpMethod.POST),
'http_method': tasks_v2.types.HttpMethod.POST,
'relative_uri': dummy_url,
'headers': {
'Content-type': 'application/json'
Expand Down
28 changes: 28 additions & 0 deletions core/templates/app-events/event-bus.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,32 @@ describe('Event Bus Group', () => {
eventbusGroup.emit(new CustomEvent('Event'));
eventbusGroup.unsubscribe();
}));

it('should throw uncaught errors that are not Error type', waitForAsync(
() => {
spyOn(Subject.prototype, 'subscribe').and.callFake(
(f) => {
// This throws "This expression is not callable. Not all constituents
// of type 'PartialObserver<any> | ((value: any) => void) |
// ((value: any) => void)' are callable. Type 'NextObserver<any>'
// has no call signatures". We need to suppress this error because of
// strict type checking.
// @ts-ignore
// The eslint error is suppressed since we need to test if
// just a string was thrown.
// eslint-disable-next-line oppia/no-to-throw
expect(() => f()).toThrow('Random Error');
return new Subscription();
}
);
eventbusGroup.on(CustomEvent as Newable<CustomEvent>, _ => {
// The eslint error is suppressed since we need to throw just a string
// for testing purposes.
// eslint-disable-next-line no-throw-literal
throw 'Random Error';
});
eventbusGroup.emit(new CustomEvent('Event'));
eventbusGroup.unsubscribe();
}
));
});
8 changes: 6 additions & 2 deletions core/templates/app-events/event-bus.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ export class EventBusService {
(event: T): void => {
try {
action.call(callbackContext, event);
} catch (error) {
this._errorHandler(error);
} catch (error: unknown) {
if (error instanceof Error) {
this._errorHandler(error as Error);
} else {
throw error;
}
}
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,28 @@ export class StatsReportingBackendApiService {
<StatsReportingUrlsKey> urlIdentifier], {
exploration_id: explorationId
});
} catch (e) {
let additionalInfo = (
'\nUndefined exploration id error debug logs:' +
'\nThe event being recorded: ' + urlIdentifier +
'\nExploration ID: ' + this.contextService.getExplorationId()
);
if (currentStateName) {
additionalInfo += (
'\nCurrent State name: ' + currentStateName);
} catch (e: unknown) {
if (e instanceof Error) {
let additionalInfo = (
'\nUndefined exploration id error debug logs:' +
'\nThe event being recorded: ' + urlIdentifier +
'\nExploration ID: ' + this.contextService.getExplorationId()
);
if (currentStateName) {
additionalInfo += (
'\nCurrent State name: ' + currentStateName);
}
if (nextExpId) {
additionalInfo += (
'\nRefresher exp id: ' + nextExpId);
}
if (previousStateName && nextStateName) {
additionalInfo += (
'\nOld State name: ' + previousStateName +
'\nNew State name: ' + nextStateName);
}
e.message += additionalInfo;
}
if (nextExpId) {
additionalInfo += (
'\nRefresher exp id: ' + nextExpId);
}
if (
previousStateName &&
nextStateName) {
additionalInfo += (
'\nOld State name: ' + previousStateName +
'\nNew State name: ' + nextStateName);
}
e.message += additionalInfo;
throw e;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,11 @@ describe('NumberWithUnitsObjectFactory', () => {
}).toThrowError(errors.INVALID_UNIT_CHARS);
expect(() => {
nwuof.fromRawInputString('2 m**2');
}).toThrowError('SyntaxError: Unexpected "*" in "m**2" at index 2');
}).toThrowError('Unexpected "*" in "m**2" at index 2');
expect(() => {
nwuof.fromRawInputString('2 kg / m^(2)');
}).toThrowError(
'SyntaxError: In "kg / m^(2)", "^" must be ' +
'followed by a floating-point number');
'In "kg / m^(2)", "^" must be followed by a floating-point number');
});

it('should create currency units', () => {
Expand Down
12 changes: 3 additions & 9 deletions core/templates/domain/objects/UnitsObjectFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ import { downgradeInjectable } from '@angular/upgrade/static';
import { Injectable } from '@angular/core';

import { createUnit, unit } from 'mathjs';
import { ObjectsDomainConstants } from
'domain/objects/objects-domain.constants';
import { Unit } from
'interactions/answer-defs';
import { ObjectsDomainConstants } from 'domain/objects/objects-domain.constants';
import { Unit } from 'interactions/answer-defs';

interface UnitsBackendDict {
units: Unit[];
Expand Down Expand Up @@ -254,11 +252,7 @@ export class UnitsObjectFactory {

var compatibleUnits = this.toMathjsCompatibleString(units);
if (compatibleUnits !== '') {
try {
unit(compatibleUnits);
} catch (err) {
throw new Error(err);
}
unit(compatibleUnits);
}
return new Units(this.fromStringToList(units));
}
Expand Down
8 changes: 3 additions & 5 deletions core/templates/domain/objects/UnitsObjectFactorySpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,12 @@ describe('UnitsObjectFactory', () => {
it('should throw errors for invalid units', () => {
expect(() => {
units.fromRawInputString('NK*kg');
}).toThrowError('SyntaxError: Unit "NK" not found.');
}).toThrowError('Unit "NK" not found.');
expect(() => {
units.fromRawInputString('per &kg$');
}).toThrowError(
'SyntaxError: Unexpected "&" in "dollar/ &kg" at index 8');
}).toThrowError('Unexpected "&" in "dollar/ &kg" at index 8');
expect(() => {
units.fromRawInputString('cent %mol$');
}).toThrowError(
'SyntaxError: Unit "dollarcent" not found.');
}).toThrowError('Unit "dollarcent" not found.');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* @fileoverview Service to check the status of dummy handler in backend.
*/

import { HttpClient } from '@angular/common/http';
import { HttpClient, HttpErrorResponse } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { downgradeInjectable } from '@angular/upgrade/static';

Expand Down Expand Up @@ -47,8 +47,8 @@ export class PlatformFeatureDummyBackendApiService {
await this.http.get(PlatformFeatureDomainConstants.DUMMY_HANDLER_URL)
.toPromise();
return true;
} catch (err) {
if (err.status === 404) {
} catch (err: unknown) {
if (err instanceof HttpErrorResponse && err.status === 404) {
return false;
} else {
throw err;
Expand Down
12 changes: 6 additions & 6 deletions core/templates/pages/login-page/login-page.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ export class LoginPageComponent implements OnInit {
let authSucceeded = false;
try {
authSucceeded = await this.authService.handleRedirectResultAsync();
} catch (error) {
this.onSignInError(error);
} catch (error: unknown) {
this.onSignInError(error as firebase.auth.Error);
return;
}

Expand All @@ -74,8 +74,8 @@ export class LoginPageComponent implements OnInit {

try {
await this.authService.signInWithRedirectAsync();
} catch (error) {
this.onSignInError(error);
} catch (error: unknown) {
this.onSignInError(error as firebase.auth.Error);
}
},
error => {
Expand All @@ -88,8 +88,8 @@ export class LoginPageComponent implements OnInit {

try {
await this.authService.signInWithEmail(email);
} catch (error) {
this.onSignInError(error);
} catch (error: unknown) {
this.onSignInError(error as firebase.auth.Error);
return;
}

Expand Down
46 changes: 46 additions & 0 deletions core/templates/services/assets-backend-api.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,29 @@ describe('Assets Backend API Service', () => {
expect(onFailure).toHaveBeenCalled();
}));

it('should handle rejection when saving a math SVG fails with non HTTP err',
fakeAsync(() => {
const onSuccess = jasmine.createSpy('onSuccess');
const onFailure = jasmine.createSpy('onFailure');

spyOn(
// This throws "Argument of type 'getImageUploadUrl' is not assignable
// to parameter of type 'keyof AssetsBackendApiService'. We need to
// suppress this error because of strict type checking.
// @ts-ignore
assetsBackendApiService, 'getImageUploadUrl'
).and.throwError(Error('token'));

assetsBackendApiService.saveMathExpresionImage(
imageBlob, 'new.svg', 'exploration', 'expid12345'
).then(onSuccess, onFailure);
flushMicrotasks();

expect(onSuccess).not.toHaveBeenCalled();
expect(onFailure).toHaveBeenCalled();
})
);

it('should handle rejection when saving a file fails', fakeAsync(() => {
const onSuccess = jasmine.createSpy('onSuccess');
const onFailure = jasmine.createSpy('onFailure');
Expand All @@ -233,6 +256,29 @@ describe('Assets Backend API Service', () => {
expect(onFailure).toHaveBeenCalled();
}));

it('should handle rejection when saving a file fails with non HTTP error',
fakeAsync(() => {
const onSuccess = jasmine.createSpy('onSuccess');
const onFailure = jasmine.createSpy('onFailure');

spyOn(
// This throws "Argument of type 'getImageUploadUrl' is not assignable
// to parameter of type 'keyof AssetsBackendApiService'. We need to
// suppress this error because of strict type checking.
// @ts-ignore
assetsBackendApiService, 'getAudioUploadUrl'
).and.throwError(Error('token'));

assetsBackendApiService.saveAudio(
'0', 'a.mp3', audioBlob
).then(onSuccess, onFailure);
flushMicrotasks();

expect(onSuccess).not.toHaveBeenCalled();
expect(onFailure).toHaveBeenCalled();
})
);

it('should successfully fetch and cache image', fakeAsync(() => {
const successHandler = jasmine.createSpy('success');
const failHandler = jasmine.createSpy('fail');
Expand Down
Loading

0 comments on commit 17711ee

Please sign in to comment.