Remove creating global `_status_catalog` variable

e2e/jail/jail_rpc_test.go

@@ -272,6 +272,8 @@ func (s *JailRPCTestSuite) TestJailVMPersistence() {
 			 }
 		  });
 		}
+
+		_status_catalog;
 	`)
 	s.NotContains(parseResult, "error", "further will fail if initial parsing failed")
 

e2e/jail/jail_test.go

@@ -56,9 +56,9 @@ func (s *JailTestSuite) TestInitWithoutBaseJS() {
 	s.EqualError(err, "cell '"+testChatID+"' not found")
 	s.Nil(cell)
 
-	// create VM (w/o properly initializing base JS script)
+	response := s.Jail.CreateAndInitCell(testChatID)
-	err = errors.New("ReferenceError: '_status_catalog' is not defined")
+	s.Equal(jail.EmptyResponse, response)
-	s.Equal(errorWrapper(err), s.Jail.CreateAndInitCell(testChatID, ``))
+
 	err = errors.New("ReferenceError: 'call' is not defined")
 	s.Equal(errorWrapper(err), s.Jail.Call(testChatID, `["commands", "testCommand"]`, `{"val": 12}`))
 
@@ -77,40 +77,34 @@ func (s *JailTestSuite) TestInitWithBaseJS() {
 
 	// now no error should occur
 	response := s.Jail.CreateAndInitCell(testChatID)
-	expectedResponse := `{"result": {"commands":{},"responses":{}}}`
+	s.Equal(jail.EmptyResponse, response)
-	s.Equal(expectedResponse, response)
 
 	// make sure that Call succeeds even w/o running node
 	response = s.Jail.Call(testChatID, `["commands", "testCommand"]`, `{"val": 12}`)
-	expectedResponse = `{"result": 144}`
+	expectedResponse := `{"result": 144}`
 	s.Equal(expectedResponse, response)
 }
 
-// @TODO(adam): finally, this test should pass as checking existence of `_status_catalog`
+func (s *JailTestSuite) TestCreateAndInitCell() {
-// should be done in status-react.
+	// If no custom JS provided -- the default response is returned
-func (s *JailTestSuite) TestCreateAndInitCellWithoutStatusCatalog() {
+	response := s.Jail.CreateAndInitCell("newChat1")
-	response := s.Jail.CreateAndInitCell(testChatID)
+	expectedResponse := jail.EmptyResponse
-	s.Equal(`{"error":"ReferenceError: '_status_catalog' is not defined"}`, response)
+	s.Equal(expectedResponse, response)
-}
 
-// @TODO(adam): remove extra JS when checking `_status_catalog` is move to status-react.
+	// If any custom JS provided -- the result of the last op is returned
-func (s *JailTestSuite) TestMultipleInitError() {
+	response = s.Jail.CreateAndInitCell("newChat2", "var a = 2", "a")
-	response := s.Jail.CreateAndInitCell(testChatID, `var _status_catalog = {}`)
+	expectedResponse = `{"result": 2}`
-	s.Equal(`{"result": {}}`, response)
+	s.Equal(expectedResponse, response)
 
-	// Shouldn't cause an error and reinitialize existing
+	// Reinitialization preserves the JS environment, so the 'test' variable exists
-	response = s.Jail.CreateAndInitCell(testChatID)
+	// even though we didn't initialize it here (in the second room).
-	s.Equal(`{"result": {}}`, response)
+	response = s.Jail.CreateAndInitCell("newChat2", "a")
-}
+	expectedResponse = `{"result": 2}`
+	s.Equal(expectedResponse, response)
 
-// @TODO(adam): remove extra JS when checking `_status_catalog` is moved to status-react.
+	// But this variable doesn't leak into other rooms.
-func (s *JailTestSuite) TestCreateAndInitCellResponse() {
+	response = s.Jail.CreateAndInitCell("newChat1", "a")
-	extraCode := `
+	expectedResponse = `{"error":"ReferenceError: 'a' is not defined"}`
-	var _status_catalog = {
-		foo: 'bar'
-	};`
-	response := s.Jail.CreateAndInitCell("newChat", extraCode)
-	expectedResponse := `{"result": {"foo":"bar"}}`
 	s.Equal(expectedResponse, response)
 }
 

geth/jail/handlers_test.go

@@ -52,7 +52,7 @@ func (s *HandlersTestSuite) TestWeb3SendHandlerSuccess() {
 
 	jail := New(&testRPCClientProvider{client})
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	// web3.eth.syncing is an arbitrary web3 sync RPC call.
@@ -66,7 +66,7 @@ func (s *HandlersTestSuite) TestWeb3SendHandlerSuccess() {
 func (s *HandlersTestSuite) TestWeb3SendHandlerFailure() {
 	jail := New(nil)
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	_, err = cell.Run("web3.eth.syncing")
@@ -79,7 +79,7 @@ func (s *HandlersTestSuite) TestWeb3SendAsyncHandlerSuccess() {
 
 	jail := New(&testRPCClientProvider{client})
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	errc := make(chan string)
@@ -104,7 +104,7 @@ func (s *HandlersTestSuite) TestWeb3SendAsyncHandlerWithoutCallbackSuccess() {
 
 	jail := New(&testRPCClientProvider{client})
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	_, err = cell.Run(`web3.eth.getSyncing()`)
@@ -119,7 +119,7 @@ func (s *HandlersTestSuite) TestWeb3SendAsyncHandlerWithoutCallbackSuccess() {
 func (s *HandlersTestSuite) TestWeb3SendAsyncHandlerFailure() {
 	jail := New(nil)
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	errc := make(chan otto.Value)
@@ -148,7 +148,7 @@ func (s *HandlersTestSuite) TestWeb3IsConnectedHandler() {
 
 	jail := New(&testRPCClientProvider{client})
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	// When result is true.
@@ -170,7 +170,7 @@ func (s *HandlersTestSuite) TestWeb3IsConnectedHandler() {
 func (s *HandlersTestSuite) TestSendSignalHandler() {
 	jail := New(nil)
 
-	cell, err := jail.createAndInitCell("cell1")
+	cell, _, err := jail.createAndInitCell("cell1")
 	s.NoError(err)
 
 	signal.SetDefaultNodeNotificationHandler(func(jsonEvent string) {

geth/jail/jail.go

@@ -22,6 +22,9 @@ const (
 			return new Bignumber(val);
 		}
 	`
+	// EmptyResponse is returned when cell is successfully created and initialized
+	// but no additional JS was provided to the initialization method.
+	EmptyResponse = `{"result": ""}`
 )
 
 var (
@@ -137,37 +140,37 @@ func (j *Jail) initCell(cell *Cell) error {
 }
 
 // CreateAndInitCell creates and initializes a new Cell.
-func (j *Jail) createAndInitCell(chatID string, code ...string) (*Cell, error) {
+func (j *Jail) createAndInitCell(chatID string, extraCode ...string) (*Cell, string, error) {
 	cell, err := j.obtainCell(chatID, false)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
 	if err := j.initCell(cell); err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
-	// Run custom user code
+	response := EmptyResponse
-	for _, js := range code {
+
-		_, err := cell.Run(js)
+	if len(extraCode) > 0 {
+		result, err := cell.Run(strings.Join(extraCode, ";"))
 		if err != nil {
-			return nil, err
+			return nil, "", err
 		}
+		response = newJailResultResponse(result)
 	}
 
-	return cell, nil
+	return cell, response, nil
 }
 
-// CreateAndInitCell creates and initializes new Cell. Additionally,
+// CreateAndInitCell creates and initializes new Cell.
-// it creates a `catalog` variable in the VM.
-// It returns the response as a JSON string.
 func (j *Jail) CreateAndInitCell(chatID string, code ...string) string {
-	cell, err := j.createAndInitCell(chatID, code...)
+	_, result, err := j.createAndInitCell(chatID, code...)
 	if err != nil {
 		return newJailErrorResponse(err)
 	}
 
-	return j.makeCatalogVariable(cell)
+	return result
 }
 
 // Parse creates a new jail cell context, with the given chatID as identifier.
@@ -175,9 +178,10 @@ func (j *Jail) CreateAndInitCell(chatID string, code ...string) string {
 // DEPRECATED in favour of CreateAndInitCell.
 func (j *Jail) Parse(chatID, code string) string {
 	cell, err := j.cell(chatID)
+	result := EmptyResponse
 	if err != nil {
 		// cell does not exist, so create and init it
-		cell, err = j.createAndInitCell(chatID, code)
+		cell, result, err = j.createAndInitCell(chatID, code)
 	} else {
 		// cell already exists, so just reinit it
 		err = j.initCell(cell)
@@ -191,25 +195,7 @@ func (j *Jail) Parse(chatID, code string) string {
 		return newJailErrorResponse(err)
 	}
 
-	return j.makeCatalogVariable(cell)
+	return result
-}
-
-// makeCatalogVariable provides `catalog` as a global variable.
-// TODO(divan): this can and should be implemented outside of jail,
-// on a clojure side. Moving this into separate method to nuke it later
-// easier.
-func (j *Jail) makeCatalogVariable(cell *Cell) string {
-	_, err := cell.Run(`var catalog = JSON.stringify(_status_catalog)`)
-	if err != nil {
-		return newJailErrorResponse(err)
-	}
-
-	value, err := cell.Get("catalog")
-	if err != nil {
-		return newJailErrorResponse(err)
-	}
-
-	return newJailResultResponse(value)
 }
 
 func (j *Jail) cell(chatID string) (*Cell, error) {

geth/jail/jail_test.go

@@ -101,29 +101,16 @@ func (s *JailTestSuite) TestJailCall() {
 	s.Equal(`{"result": undefined}`, result)
 }
 
-func (s *JailTestSuite) TestMakeCatalogVariable() {
-	cell, err := s.Jail.obtainCell("cell1", false)
-	s.NoError(err)
-
-	// no `_status_catalog` variable
-	response := s.Jail.makeCatalogVariable(cell)
-	s.Equal(`{"error":"ReferenceError: '_status_catalog' is not defined"}`, response)
-
-	// with `_status_catalog` variable
-	_, err = cell.Run(`var _status_catalog = { test: true }`)
-	s.NoError(err)
-	response = s.Jail.makeCatalogVariable(cell)
-	s.Equal(`{"result": {"test":true}}`, response)
-}
-
 func (s *JailTestSuite) TestCreateAndInitCell() {
-	cell, err := s.Jail.createAndInitCell(
+	cell, result, err := s.Jail.createAndInitCell(
 		"cell1",
 		`var testCreateAndInitCell1 = true`,
 		`var testCreateAndInitCell2 = true`,
+		`testCreateAndInitCell2`,
 	)
 	s.NoError(err)
 	s.NotNil(cell)
+	s.Equal(`{"result": true}`, result)
 
 	value, err := cell.Get("testCreateAndInitCell1")
 	s.NoError(err)
@@ -135,18 +122,23 @@ func (s *JailTestSuite) TestCreateAndInitCell() {
 }
 
 func (s *JailTestSuite) TestPublicCreateAndInitCell() {
-	response := s.Jail.CreateAndInitCell("cell1", `var _status_catalog = { test: true }`)
+	response := s.Jail.CreateAndInitCell("cell1")
-	s.Equal(`{"result": {"test":true}}`, response)
+	s.Equal(EmptyResponse, response)
+}
+
+func (s *JailTestSuite) TestPublicCreateAndInitCellWithJS() {
+	response := s.Jail.CreateAndInitCell("cell1", "var a = 2", "a")
+	s.Equal(`{"result": 2}`, response)
 }
 
 func (s *JailTestSuite) TestPublicCreateAndInitCellConsecutive() {
-	response1 := s.Jail.CreateAndInitCell("cell1", `var _status_catalog = { test: true }`)
+	response1 := s.Jail.CreateAndInitCell("cell1", `var _status_catalog = { test: true }; JSON.stringify(_status_catalog);`)
 	s.Contains(response1, "test")
 	cell1, err := s.Jail.Cell("cell1")
 	s.NoError(err)
 
 	// Create it again
-	response2 := s.Jail.CreateAndInitCell("cell1", `var _status_catalog = { test: true, foo: 5 }`)
+	response2 := s.Jail.CreateAndInitCell("cell1", `var _status_catalog = { test: true, foo: 5 }; JSON.stringify(_status_catalog);`)
 	s.Contains(response2, "test", "foo")
 	cell2, err := s.Jail.Cell("cell1")
 	s.NoError(err)

lib/utils.go

@@ -40,6 +40,10 @@ import (
 )
 
 const zeroHash = "0x0000000000000000000000000000000000000000000000000000000000000000"
+const initJS = `
+	var _status_catalog = {
+		foo: 'bar'
+	};`
 
 var testChainDir string
 var nodeConfigJSON string
@@ -1281,14 +1285,8 @@ func testJailInitInvalid(t *testing.T) bool {
 
 func testJailParseInvalid(t *testing.T) bool {
 	// Arrange.
-	initCode := `
+	InitJail(C.CString(initJS))
-	var _status_catalog = {
-		foo: 'bar'
-	};
-	`
-
 	// Act.
-	InitJail(C.CString(initCode))
 	extraInvalidCode := `
 	var extraFunc = function (x) {
 	  return x * x;
@@ -1305,71 +1303,43 @@ func testJailParseInvalid(t *testing.T) bool {
 }
 
 func testJailInit(t *testing.T) bool {
-	initCode := `
+	InitJail(C.CString(initJS))
-	var _status_catalog = {
-		foo: 'bar'
-	};
-	`
-	InitJail(C.CString(initCode))
 
-	extraCode := `
+	chatID := C.CString("CHAT_ID_INIT_TEST")
-	var extraFunc = function (x) {
-	  return x * x;
-	};
-	`
-	rawResponse := CreateAndInitCell(C.CString("CHAT_ID_INIT_TEST"), C.CString(extraCode))
-	parsedResponse := C.GoString(rawResponse)
 
-	expectedResponse := `{"result": {"foo":"bar"}}`
+	// Cell initialization return the result of the last JS operation provided to it.
+	response := CreateAndInitCell(chatID, C.CString(`var extraFunc = function (x) { return x * x; }; extraFunc(2);`))
+	require.Equal(t, `{"result": 4}`, C.GoString(response), "Unexpected response from jail.CreateAndInitCell()")
 
-	if !reflect.DeepEqual(expectedResponse, parsedResponse) {
+	// Commands from the jail initialization are available in any of the created cells.
-		t.Error("expected output not returned from jail.CreateAndInitCell()")
+	response = ExecuteJS(chatID, C.CString(`JSON.stringify({ result: _status_catalog });`))
-		return false
+	require.Equal(t, `{"result":{"foo":"bar"}}`, C.GoString(response), "Environment from `InitJail` is not available in the created cell")
-	}
-
-	t.Logf("jail inited and parsed: %s", parsedResponse)
 
 	return true
 }
 
 func testJailParseDeprecated(t *testing.T) bool {
-	initCode := `
+	InitJail(C.CString(initJS))
-		var _status_catalog = {
-			foo: 'bar'
-		};
-	`
-	InitJail(C.CString(initCode))
 
 	extraCode := `
 		var extraFunc = function (x) {
 			return x * x;
 		};
+
+		extraFunc(2);
 	`
-	rawResponse := Parse(C.CString("CHAT_ID_PARSE_TEST"), C.CString(extraCode))
+	chatID := C.CString("CHAT_ID_PARSE_TEST")
-	parsedResponse := C.GoString(rawResponse)
+
-	expectedResponse := `{"result": {"foo":"bar"}}`
+	response := Parse(chatID, C.CString(extraCode))
-	if !reflect.DeepEqual(expectedResponse, parsedResponse) {
+	require.Equal(t, `{"result": 4}`, C.GoString(response))
-		t.Error("expected output not returned from Parse()")
-		return false
-	}
 
 	// cell already exists but Parse should not complain
-	rawResponse = Parse(C.CString("CHAT_ID_PARSE_TEST"), C.CString(extraCode))
+	response = Parse(chatID, C.CString(extraCode))
-	parsedResponse = C.GoString(rawResponse)
+	require.Equal(t, `{"result": 4}`, C.GoString(response))
-	expectedResponse = `{"result": {"foo":"bar"}}`
-	if !reflect.DeepEqual(expectedResponse, parsedResponse) {
-		t.Error("expected output not returned from Parse()")
-		return false
-	}
 
 	// test extraCode
-	rawResponse = ExecuteJS(C.CString("CHAT_ID_PARSE_TEST"), C.CString(`extraFunc(2)`))
+	response = ExecuteJS(chatID, C.CString(`extraFunc(10)`))
-	parsedResponse = C.GoString(rawResponse)
+	require.Equal(t, `100`, C.GoString(response))
-	expectedResponse = `4`
-	if !reflect.DeepEqual(expectedResponse, parsedResponse) {
-		t.Error("expected output not returned from ExecuteJS()")
-		return false
-	}
 
 	return true
 }