Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Google Blockly] strip user code #57903

Merged
merged 2 commits into from
Apr 10, 2024
Merged

[Google Blockly] strip user code #57903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c26e44
[Google Blockly] strip user code
mikeharv Apr 9, 2024
c311b32
revert use GetWorkspaceCode to get student JavaScript
mikeharv Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 6 additions & 39 deletions apps/src/blockly/cdoBlocklyWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,14 @@ import {
setHasIncompatibleSources,
} from '@cdo/apps/redux/blockly';
import * as blockUtils from '../block_utils';
import {handleCodeGenerationFailure} from './utils';
import {
INFINITE_LOOP_TRAP,
LOOP_HIGHLIGHT,
handleCodeGenerationFailure,
strip,
} from './utils';
import {MetricEvent} from '@cdo/apps/lib/metrics/events';

const INFINITE_LOOP_TRAP =
' executionInfo.checkTimeout(); if (executionInfo.isTerminated()){return;}\n';

const LOOP_HIGHLIGHT = 'loopHighlight();\n';
const LOOP_HIGHLIGHT_RE = new RegExp(
LOOP_HIGHLIGHT.replace(/\(.*\)/, '\\(.*\\)') + '\\s*',
'g'
);

/**
* Wrapper class for https://github.com/code-dot-org/blockly
* This wrapper will facilitate migrating from CDO Blockly to Google Blockly
Expand Down Expand Up @@ -53,35 +49,6 @@ const BlocklyWrapper = function (blocklyInstance) {
};
};

function escapeRegExp(str) {
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}

/**
* Extract the user's code as raw JavaScript.
* @param {string} code Generated code.
* @return {string} The code without serial numbers and timeout checks.
*/
function strip(code) {
return (
code
// Strip out serial numbers.
.replace(/(,\s*)?'block_id_\d+'\)/g, ')')
// Remove timeouts.
.replace(new RegExp(escapeRegExp(INFINITE_LOOP_TRAP), 'g'), '')
// Strip out loop highlight
.replace(LOOP_HIGHLIGHT_RE, '')
// Strip out class namespaces.
.replace(/(StudioApp|Maze|Turtle)\./g, '')
// Strip out particular helper functions.
.replace(/^function (colour_random)[\s\S]*?^}/gm, '')
// Collapse consecutive blank lines.
.replace(/\n\n+/gm, '\n\n')
// Trim.
.replace(/^\s+|\s+$/g, '')
);
}

/**
* Given a type string for a field input, returns an appropriate change handler function
* for that type, which customizes the input field and provides validation on blur.
Expand Down
11 changes: 7 additions & 4 deletions apps/src/blockly/googleBlocklyWrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ import {
import {initializeScrollbarPair} from './addons/cdoScrollbar';
import {getStore} from '@cdo/apps/redux';
import {setFailedToGenerateCode} from '@cdo/apps/redux/blockly';
import {handleCodeGenerationFailure} from './utils';
import {
INFINITE_LOOP_TRAP,
LOOP_HIGHLIGHT,
handleCodeGenerationFailure,
strip,
} from './utils';
import {MetricEvent} from '@cdo/apps/lib/metrics/events';
import {
BlocklyWrapperType,
Expand All @@ -104,11 +109,8 @@ const options = {
const plugin = new CrossTabCopyPaste();
plugin.init(options);

const INFINITE_LOOP_TRAP =
' executionInfo.checkTimeout(); if (executionInfo.isTerminated()){return;}\n';
const MAX_GET_CODE_RETRIES = 2;
const RETRY_GET_CODE_INTERVAL_MS = 500;
const LOOP_HIGHLIGHT = 'loopHighlight();\n';

/**
* Wrapper class for https://github.com/google/blockly
Expand Down Expand Up @@ -224,6 +226,7 @@ function initializeBlocklyWrapper(blocklyInstance: GoogleBlocklyInstance) {
if (hiddenWorkspace) {
workspaceCode += Blockly.JavaScript.workspaceToCode(hiddenWorkspace);
}
workspaceCode = strip(workspaceCode);
getStore().dispatch(setFailedToGenerateCode(false));
} catch (e) {
if (retryCount < MAX_GET_CODE_RETRIES) {
Expand Down
37 changes: 37 additions & 0 deletions apps/src/blockly/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,40 @@ export function getBaseName(themeName: Themes) {
return themeName.replace(DARK_THEME_SUFFIX, '');
}
}
export const INFINITE_LOOP_TRAP =
' executionInfo.checkTimeout(); if (executionInfo.isTerminated()){return;}\n';

export const LOOP_HIGHLIGHT = 'loopHighlight();\n';
const LOOP_HIGHLIGHT_RE = new RegExp(
LOOP_HIGHLIGHT.replace(/\(.*\)/, '\\(.*\\)') + '\\s*',
'g'
);

function escapeRegExp(str: string) {
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}

/**
* Extract the user's code as raw JavaScript.
* @param {string} code Generated code.
* @return {string} The code without serial numbers and timeout checks.
*/
export function strip(code: string) {
return (
code
// Strip out serial numbers.
.replace(/(,\s*)?'block_id_[^']+'\)/g, ')')
// Remove timeouts.
.replace(new RegExp(escapeRegExp(INFINITE_LOOP_TRAP), 'g'), '')
// Strip out loop highlight
.replace(LOOP_HIGHLIGHT_RE, '')
// Strip out class namespaces.
.replace(/(StudioApp|Maze|Turtle)\./g, '')
// Strip out particular helper functions.
.replace(/^function (colour_random)[\s\S]*?^}/gm, '')
// Collapse consecutive blank lines.
.replace(/\n\n+/gm, '\n\n')
// Trim.
.replace(/^\s+|\s+$/g, '')
);
}
8 changes: 7 additions & 1 deletion apps/src/craft/agent/craft.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,13 @@ export default class Craft {
});

// Run user generated code, calling appCodeOrgAPI
const code = Blockly.getWorkspaceCode();
let code = '';
let codeBlocks = Blockly.mainBlockSpace.getTopBlocks(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be worthwhile to move this code into a wrapper function so we don't have to repeat it? Does the reference to studioApp().initializationBlocks mean we can't move this into the wrapper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially... although the changes in these craft files are really just a straight revert of 3c8902b

There's quite a few variations of the logic here in the various lab files, so I think it would take some thought to figure out exactly what we want to abstract and move to the wrapper.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 makes sense

if (studioApp().initializationBlocks) {
codeBlocks = studioApp().initializationBlocks.concat(codeBlocks);
}

code = Blockly.Generator.blocksToCode('JavaScript', codeBlocks);
CustomMarshalingInterpreter.evalWith(code, {
moveForward: function (blockID) {
appCodeOrgAPI.moveForward(
Expand Down
3 changes: 2 additions & 1 deletion apps/src/craft/aquatic/craft.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,8 @@ Craft.executeUserCode = function () {
};

// Run user code.
code += Blockly.getWorkspaceCode();
let codeBlocks = Blockly.mainBlockSpace.getTopBlocks(true);
code += Blockly.Generator.blocksToCode('JavaScript', codeBlocks);
interpreter = CustomMarshalingInterpreter.evalWith(
code,
{
Expand Down
6 changes: 5 additions & 1 deletion apps/src/craft/designer/craft.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,11 @@ Craft.executeUserCode = function () {
var appCodeOrgAPI = Craft.gameController.codeOrgAPI;
appCodeOrgAPI.startCommandCollection();
// Run user generated code, calling appCodeOrgAPI
const code = Blockly.getWorkspaceCode();
let codeBlocks = Blockly.mainBlockSpace.getTopBlocks(true);
if (studioApp().initializationBlocks) {
codeBlocks = studioApp().initializationBlocks.concat(codeBlocks);
}
const code = Blockly.Generator.blocksToCode('JavaScript', codeBlocks);

const evalApiMethods = {
moveForward: function (blockID) {
Expand Down
8 changes: 7 additions & 1 deletion apps/src/craft/simple/craft.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,13 @@ Craft.executeUserCode = function () {
});

// Run user generated code, calling appCodeOrgAPI
const code = Blockly.getWorkspaceCode();
let code = '';
let codeBlocks = Blockly.mainBlockSpace.getTopBlocks(true);
if (studioApp().initializationBlocks) {
codeBlocks = studioApp().initializationBlocks.concat(codeBlocks);
}

code = Blockly.Generator.blocksToCode('JavaScript', codeBlocks);

CustomMarshalingInterpreter.evalWith(
code,
Expand Down