fnc12 / Mitsoko

10 stars 1 forks source link

Separation of example files in separate folder #7

Closed DonaldTrump88 closed 7 years ago

DonaldTrump88 commented 7 years ago

I want to use the library in another application with different communication method. Is it possible to separate library code and example code(connected with template) in two separate folders? I separated Mitsoko main folder files which are example related. The files are

Language.cpp
Language.hpp
Presenter.hpp
View.cpp
View.hpp
ViperGod.cpp
ViperGod.hpp

The common library files in Mitsoko folder are

AndroidUtil
example //the example folder
iOSutil
Url
//Remaining Mitsoko files
fnc12 commented 7 years ago

You're asking about feature which wasn't included in design of the lib. This repo is separated from template cause developers must have ability to update existing project when the lib updates. That's why this repo is a subrepo of the example. One thing cannot be updates with git - objc and java files. But I think I'll make a script for it. So if you want to make it a separate lib maybe you should make a fork especially for this goal. I can commit to you and we can share updates.

DonaldTrump88 commented 7 years ago

Thanks for your reply. Mitsoko is kind of new design for me. I feel you made some great design. Nvertheless I am still learning about it. Before commenting on anything further, I need to have some working model.

BTW, I could compile the library code without example files. My next target is to do network calls as you explained in https://github.com/fnc12/embeddedRest/issues/4

fnc12 commented 7 years ago

Ok cool. Thanks. So you don't want to add java HTTP bindings? BTW I found today http client based on boost::asio, crypto and openssl https://github.com/reo7sp/tgbot-cpp/blob/master/src/net/HttpClient.cpp .

DonaldTrump88 commented 7 years ago

:) I want to use HttpURLConnection from MitSoko. The call sequence will be App(Java) > C++ library > Mitsoko > Java.

There are couple of problems with asio and TLS approch.

  1. There are no official buids on mobile platforms. Most of ways are unofficial ported version.
  2. As I posted yesterday, Android prefers statically linked SSL or calling it using java classes.
  3. In case of statically SSL binding, app size increases and I have to maintain it.
  4. Building and maintaing SSL libraries is huge problem due to unknown app user, the portable libressl(my preferred) is available after linux kernel 3.17(released in Feb 2015).
  5. Most of asio libraries come with boost, which I do not want. There is standalone asio also.
  6. For asio there is no good wrapper.

If I decide for asio + TLS then I have build asio, asio C++ wrapper and SSL. Most of asio wrappers are not ready to use.

To avoid so many steps and following android advice, I decided to give Mitsoko a try. Lets see.

My only requirement is doing rest calls(mostly sync with timout) to server. For this simple requirement is not worth to do so many steps with TLS.

fnc12 commented 7 years ago

Ok. BTW CrystaX has built-in boost

DonaldTrump88 commented 7 years ago

I have noticed that you used same JVM received fro NI.cpp. Does it cause problem to application like thread blocking etc.?

fnc12 commented 7 years ago

You talking about JNIEnv* value? It doesn't lock anything and it stores in a static map in java::lang::JNI class. It is removed in its destructor. This is how JNIEnv* has always actual value in C++

DonaldTrump88 commented 7 years ago

Yes, I was talking about JNIEnv pointer.

DonaldTrump88 commented 7 years ago

I set JNIEnv using following code.

java::lang::JNI e(g_pJNIEnv);

std::string str =   "https://api.vk.com/method/database.getCountries";
jbyteArray array = g_pJNIEnv->NewByteArray(str.size());
g_pJNIEnv->SetByteArrayRegion(array, 0, str.size(), (const jbyte*)str.c_str());
//works till here.

//does not work below.
Request request;
//  https://api.vk.com/method/database.getCountries?lang=en
request.url("https://api.vk.com/method/database.getCountries", {
    { "lang", "en" },
});

I get segmentation fault in following code.
auto java::lang::String::create(std::string str)->String{
    if(auto env=JNI::Env()){
        jbyteArray array = env->NewByteArray(str.size());
        env->SetByteArrayRegion(array, 0, str.size(), (const jbyte*)str.c_str());
        jstring strEncode = env->NewStringUTF("UTF-8");
//        jclass cls = env->FindClass(signature().c_str());
        auto cls = java::lang::Class::find<String>();
        jmethodID ctor = env->GetMethodID(cls, "<init>", "([BLjava/lang/String;)V");
        jstring object = (jstring) env->NewObject(cls, ctor, array, strEncode);
        return object;
    }else{
        return nullptr;
    }
}

jbyteArray NewByteArray(jsize length)
{ return functions->NewByteArray(this, length); }

I check pointer first case and second is same. 06-25 18:16:13.518 9494-9494/ A/libc: Fatal signal 6 (SIGABRT), code -6 in tid 9494 ()

                                                    [ 06-25 18:16:13.519   292:  292 E/         ]
                                                    ptrace attach failed: Operation not permitted

0xb8151268 0xb8151268 0xb8151268

fnc12 commented 7 years ago

What's your goal? Create jstring? And please show declaration of g_pJNIEnv. Thanks

DonaldTrump88 commented 7 years ago

I want to set current JNIEnv to the map. So I can call this request example.

#include <jni.h>
JNIEnv *g_pJNIEnv = NULL;

JNIEXPORT jshort JNICALL Java_tech_Interface_Command
  (JNIEnv * env, jclass IntCls, jshort Command, jstring Ip, jobject RetObj)
{
    (void)IntCls;
    jshort RetVal = 0;

    g_pJNIEnv = env;

//call to usage of g_pJNIEnv

}
DonaldTrump88 commented 7 years ago

I am not calling anything from this method. Is it relevant in this case?

mitsoko-template-master\proj.android\app\src\main\jni\NI.cpp
    jint JNI_OnLoad(JavaVM *vm, void *reserved){
        // Get JNI Env for all function calls
        JNIEnv* env;
        if (vm->GetEnv((void **) &env, JNI_VERSION_1_6) != JNI_OK) {
            cout<<"GetEnv failed."<<endl;
            return -1;
        }

        java::lang::JNI e(env);
        Mitsoko::Dispatch::initMainThreadHandler();
        std::cout.rdbuf(new androidbuf);
        return JNI_VERSION_1_6;
    }
fnc12 commented 7 years ago

Don't save JNIEnv* in global var - JNIEnv* is local and is invalidated in the end of native function call - this is how JVM works. JVM is hell and I tried to get this hell to work well just like objc runtime. You don't need to save JNIEnv* explicitly - it is done in this line: java::lang::JNI e(env);. Please check out java::lang::JNI constructor and destructor. And this line exists in every native call - JNIEnv* is be inserted in the map and removed once e var is destroyed. So all you need to do is to add the same line to your all custom native function calls in the beginning. For example please check NI.cpp.

DonaldTrump88 commented 7 years ago

A/art: art/runtime/java_vm_ext.cc:410] JNI DETECTED ERROR IN APPLICATION: JNI NewByteArray called with pending exception java.lang.ClassNotFoundException: Didn't find class "kz.outlawstudio.viper.Network$UrlRequest" on path: DexPathList[[zip file

fnc12 commented 7 years ago

kz.outlawstudio.viper.Network$UrlRequest this class is located in `app/src/main/java/kz/outlawstudi/viper/network.java. Check whether it exists in your project

DonaldTrump88 commented 7 years ago

I know that, but this test code. To pass JNIEnv as function argument, I need to change couple of function signatures.

DonaldTrump88 commented 7 years ago

It is not in my project. I found it in
proj.android\app\src\main\java\kz\outlawstudio\viper

fnc12 commented 7 years ago

JNIEnv* is passed in every native calls as the first argument - every function that defined in java with native keyword. So create java::lang::JNI local object in every native C++ function body and you can access it in every function with java::lang::JNI::Env() static function.

DonaldTrump88 commented 7 years ago

Where should I place network.java in Mitsoko project?

fnc12 commented 7 years ago

in this dir

DonaldTrump88 commented 7 years ago

If this is Mitsoko functionality, will mind to move it to Mitsoko when you get time?

fnc12 commented 7 years ago

I'd like to but how? Mitsoko dir cannot contain java code cause it is shared between ios and android so it isn't located in java source tree. So git pull won't help. I think I'll make a python script for updating of this - something like update mitsoko version. It will pull mitsoko and refresh objc and java helper classes.

DonaldTrump88 commented 7 years ago

Is it compliation problem? What will be class signature if I place it in AndroidUtil? static auto networkUrlRequestClassSignature = "kz/outlawstudio/viper/Network$UrlRequest";

fnc12 commented 7 years ago

None. Classes in java project must be located in java source tree. Source tree has root inside proj.android dir which is located on the same level with Core which has AndroidUtil. So there is no way to add java classes outside and still compile it with android studio.

DonaldTrump88 commented 7 years ago

ok, should network.java be my application?

fnc12 commented 7 years ago

What? Class with "kz/outlawstudio/viper/Network$UrlRequest" signature? Yes if you want url request in C++ based on java bindings.

DonaldTrump88 commented 7 years ago

ok, let me try it. How frequently do you change network.java?

fnc12 commented 7 years ago

Seldom. You can set watch mode on for mitsoko-template repo to know when and what changed are made in it.

DonaldTrump88 commented 7 years ago

I shall do it. Thanks. BTW, do you have any plan to implement HTTPS in network.java?

fnc12 commented 7 years ago

HTTPS is already implemented cause it uses native java android requests which of course implement SSL. This is advantage of bindings on both platforms.

DonaldTrump88 commented 7 years ago

How are Mitsoko::Url::Request::urlResponseReceived and request.performAsync([=, &retval](Response response, std::vector data, Error error) connected?

DonaldTrump88 commented 7 years ago

This is my rest call.

std::string RestCall()
{
    Request request;
    //  https://api.vk.com/method/database.getCountries?lang=en
    request.url("https://api.vk.com/method/database.getCountries", {
        { "lang", "en" },
    });
    std::string retval ;
    request.performAsync([=, &retval](Response response, std::vector<char> data, Error error){
        if(response){
            auto statusCode = response.statusCode();
            if(statusCode == 200){
                try{
                    std::string dataString{ data.begin(), data.end() };
                    retval = dataString;
                    int i = 0;
                    ++i;
//                    auto j = json::parse(dataString);
//                    auto countries = j["response"].get<std::vector<Country>>();
//                    cb(std::move(countries), {});
                }catch(std::domain_error e){
//                    cb({}, e.what());
                    int i = 0;
                    ++i;
                }catch(...){
//                    cb({}, "Invalid data");
                    int i = 0;
                    ++i;
                }
            }else{
                std::string errorText{ data.begin(), data.end() };
                if(!errorText.length()){
                    errorText = "Undefined error";
                }
                retval = errorText;
//                cb({}, errorText);
            }
        }else{
            retval = error.message();
            int i = 0;
            ++i;
//            cb({}, error.message());
        }
    });
    return retval;
}

I get SEGFAULT at step retval = error.message();. BTW, Mitsoko::Url::Request::urlResponseReceived does get called.

fnc12 commented 7 years ago

Mitsoko::Url::Request::urlResponseReceived is called from NI.cpp within native function. You do not have to call it implicitly. Your RestCall func tries to return std::string with response body just like you call sync request but you're not - you're calling async request. It means you cannot get the result at once cause you don't know when response will be received (and whether it will be received at all). That's why you pass lambda to performAsync function. You got to call a callback function from it not return a value.

DonaldTrump88 commented 7 years ago

Yes, I am not calling it directly also. But I receive segmentation fault in request.performAsync lambda else clause( of if(response)) step retval = error.message();. What can be reason? BTW, request of java side fails due unavailability of internet permission. But in case of error also, there should not be segmentation fault.

fnc12 commented 7 years ago

Yes receive SEGFAULT cause you try to access a scope var retval in lambda which is already destroyed. Don't do it if you know that lambda will be called afterwards.

DonaldTrump88 commented 7 years ago

Thanks. Is it possible to do sync call or async call with timeout in request? I see protected ResponseTuple doInBackground(String... strings) has httpConnection.setConnectTimeout(100000); httpConnection.setReadTimeout(100000);

fnc12 commented 7 years ago

Right now there is no functionality for making sync requests and no public interface for setting a timeout. It is available to add interface for timeout. Do you need it?

DonaldTrump88 commented 7 years ago

Yes, I need it. How to do it? Setting in header of request and using them Network class? like httpConnection.setRequestProperty(header.getKey(), header.getValue());

fnc12 commented 7 years ago

Add a public function to Mitsoko::Url::Request with timeout setter and getter. Body of this functions must have different implementations for both platforms: on iOS this function must set timeout to NSMutableUrlRequest inner object, on Android we must add a function to java class Network$UrlRequest setTimeout and getTimeout which will set and get timeout to httpConnection var and call this function from C++ code. I'l add it soon if it is important

DonaldTrump88 commented 7 years ago

Good. Do you have integration test cases which should be run on commit?

fnc12 commented 7 years ago

No. The framework is raw

DonaldTrump88 commented 7 years ago

Then how can be new code tested?

fnc12 commented 7 years ago

Build and run template both on iOS and Android

DonaldTrump88 commented 7 years ago

Do you call Mitsoko library methods from UI(main) thread, async task or another thread?

DonaldTrump88 commented 7 years ago

It has to be on main UI because Network uses AsyncTask. AsyncTask cannot be nested and called from from Non-UI thread. https://developer.android.com/reference/android/os/AsyncTask.html. My worry is if fetching of data takes log time(say 60 seconds) and app starting displaying ANR message(limit 5 seconds). Did you encounter this case before?

fnc12 commented 7 years ago

I call it both from main and background threads just like I do in generic programming. performAsync is designed to be called from main thread and its callback is fired on the main thread too. What about ANR message? I don't understand you. If you need to show kinda progress bar just call performAsync and show your progress bar in the next line. Once response is received hide progress. You can view such a behavior in the template project on countries request

DonaldTrump88 commented 7 years ago

ANR = Appplication not responding.

DonaldTrump88 commented 7 years ago

It means that in case of network calls, you call from main UI thread and in other cases either from main thread/background thread. Am I correct?

fnc12 commented 7 years ago

No. performAsync is called directly from main thread and it doesn't freezes UI cause performAsync creates AsyncTask which works in background. You'll get a freezed UI on sync network request. Don't do it ever. Sync requests in main thread with UI is a non-professional way. Sync request must be emitted only in background.

DonaldTrump88 commented 7 years ago

Closed due to bring tracked(https://github.com/fnc12/Mitsoko/issues/9) in another issue and this issue is not required.