Merge pull request #220 from NativeScript/plamen5kov/fix_get_set_null_field

Plamen5kov/fix get set null field

Author: Plamen5kov

src/jni/ArgConverter.cpp

@@ -77,15 +77,13 @@ Local<Array> ArgConverter::ConvertJavaArgsToJsArgs(jobjectArray args)
 		Type argTypeID = (Type)JType::IntValue(env, argTypeIDObj);
 
 		Local<Value> jsArg;
-		Local<String> v8String;
 		switch (argTypeID)
 		{
 			case Type::Boolean:
 				jsArg = Boolean::New(isolate, JType::BooleanValue(env, arg));
 				break;
 			case Type::Char:
-				v8String = jcharToV8String(JType::CharValue(env, arg));
-				jsArg = v8String;
+				jsArg =jcharToV8String(JType::CharValue(env, arg));
 				break;
 			case Type::Byte:
 				jsArg = Number::New(isolate, JType::ByteValue(env, arg));
@@ -106,8 +104,7 @@ Local<Array> ArgConverter::ConvertJavaArgsToJsArgs(jobjectArray args)
 				jsArg = Number::New(isolate, JType::DoubleValue(env, arg));
 				break;
 			case Type::String:
-				v8String = jstringToV8String((jstring)arg);
-				jsArg = v8String;
+				jsArg = jstringToV8String((jstring)arg);
 				break;
 			case Type::JsObject:
 			{
@@ -156,11 +153,11 @@ std::string ArgConverter::jstringToString(jstring value)
 	return s;
 }
 
-Local<String> ArgConverter::jstringToV8String(jstring value)
+Local<Value> ArgConverter::jstringToV8String(jstring value)
 {
 	if	(value == nullptr)
 	{
-		return Local<String>();
+		return Null(Isolate::GetCurrent());
 	}
 
 	JEnv env;

src/jni/ArgConverter.h

@@ -27,7 +27,7 @@ namespace tns
 
 		static bool TryConvertToJavaLong(const v8::Local<v8::Value>& value, jlong& javaLong);
 
-		static v8::Local<v8::String> jstringToV8String(jstring value);
+		static v8::Local<v8::Value> jstringToV8String(jstring value);
 
 		static std::string jstringToString(jstring value);
 

src/jni/FieldAccessor.cpp

@@ -194,36 +194,26 @@ Local<Value> FieldAccessor::GetJavaField(const Local<Object>& target, FieldCallb
 	}
 	else
 	{
-		bool isString = fieldTypeName == "java/lang/String";
+		jobject result;
 
-		if (isString)
+		if (isStatic)
 		{
-			JniLocalRef result;
-			if (isStatic)
-			{
-				result = env.GetStaticObjectField(clazz, fieldId);
-			}
-			else
-			{
-				result = env.GetObjectField(targetJavaObject, fieldId);
-			}
-
-			auto resultV8String = ArgConverter::jstringToV8String(result);
-			fieldResult = handleScope.Escape(resultV8String);
+			result = env.GetStaticObjectField(clazz, fieldId);
 		}
 		else
 		{
-			jobject result;
-			if (isStatic)
+			result = env.GetObjectField(targetJavaObject, fieldId);
+		}
+
+		if(result != nullptr) {
+
+			bool isString = fieldTypeName == "java/lang/String";
+			if (isString)
 			{
-				result = env.GetStaticObjectField(clazz, fieldId);
+				auto resultV8Value = ArgConverter::jstringToV8String((jstring)result);
+				fieldResult = handleScope.Escape(resultV8Value);
 			}
 			else
-			{
-				result = env.GetObjectField(targetJavaObject, fieldId);
-			}
-
-			if (result != nullptr)
 			{
 				int javaObjectID = objectManager->GetOrCreateObjectId(result);
 				auto objectResult = objectManager->GetJsObjectByJavaObject(javaObjectID);
@@ -233,14 +223,13 @@ Local<Value> FieldAccessor::GetJavaField(const Local<Object>& target, FieldCallb
 					objectResult = objectManager->CreateJSWrapper(javaObjectID, fieldTypeName, result);
 				}
 
-				env.DeleteLocalRef(result);
-
 				fieldResult = handleScope.Escape(objectResult);
 			}
-			else
-			{
-				fieldResult = Null(isolate);
-			}
+			env.DeleteLocalRef(result);
+		}
+		else
+		{
+			fieldResult = handleScope.Escape(Null(isolate));
 		}
 	}
 	return fieldResult;
@@ -417,34 +406,33 @@ void FieldAccessor::SetJavaField(const Local<Object>& target, const Local<Value>
 	else
 	{
 		bool isString = fieldTypeName == "java/lang/String";
-		if (isString)
-		{
-			//TODO: validate valie is a string;
-			String::Utf8Value stringValue(value->ToString());
-			JniLocalRef value(env.NewStringUTF(*stringValue));
+		jobject result = nullptr;
 
-			if (isStatic)
+		if(!value->IsNull()) {
+			if (isString)
 			{
-				env.SetStaticObjectField(clazz, fieldId, value);
+				//TODO: validate valie is a string;
+				result = ConvertToJavaString(value);
 			}
 			else
 			{
-				env.SetObjectField(targetJavaObject, fieldId, value);
+				auto objectWithHiddenID = value->ToObject();
+				result =objectManager->GetJavaObjectByJsObject(objectWithHiddenID);
 			}
 		}
+
+		if (isStatic)
+		{
+			env.SetStaticObjectField(clazz, fieldId, result);
+		}
 		else
 		{
-			auto objectWithHiddenID = value->ToObject();
-			jweak javaObject = objectManager->GetJavaObjectByJsObject(objectWithHiddenID);
+			env.SetObjectField(targetJavaObject, fieldId, result);
+		}
 
-			if (isStatic)
-			{
-				env.SetStaticObjectField(clazz, fieldId, javaObject);
-			}
-			else
-			{
-				env.SetObjectField(targetJavaObject, fieldId, javaObject);
-			}
+		if (isString)
+		{
+			env.DeleteLocalRef(result);
 		}
 	}
 }

test-app/assets/app/mainpage.js

@@ -15,7 +15,7 @@ require("./tests/testsForTypescript");
 require("./tests/testGC");
 require("./tests/testsMemoryManagement");
 require("./tests/testIfAbleToRunExternalFile");
-require("./tests/finalFieldsSetTests");
+require("./tests/testFieldGetSet");
 require("./tests/extendedClassesTests");
 require("./tests/extendClassNameTests");
 require("./tests/testJniReferenceLeak");

test-app/assets/app/tests/finalFieldsSetTests.js test-app/assets/app/tests/testFieldGetSet.js

@@ -14,4 +14,14 @@ describe("Tests final fields set", function () {
 
 		expect(exceptionCaught).toBe(true);
 	});
+	
+	it("When setting a field with null it should return null object", function () {
+		
+		var dc = new com.tns.tests.DummyClass();
+		
+		dc.nameField = null;
+		var s = dc.nameField;
+		
+		expect(s).toBe(null);
+	});
 });