bubblev
/
bubble-droid
5
0
Fork 0
Code Issues 0 Pull Requests 0 Releases 0 Wiki Activity
Labels Milestones
New Pull Request

#8 connection_functionality

Merged
gegham merged 14 commits from connection_functionality into dev 4 years ago
Conversation 0 Commits 14 Files Changed 11
musheghsahakyan98 commented 4 years ago
There is no content yet.
gegham was assigned by musheghsahakyan98 4 years ago
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -25,6 +25,9 @@ public class LoginActivity extends BaseActivityBubble {
private EditText password;
private Button sign;

private static final String HTTPS = "https://";
gegham commented 4 years ago

rename to BASE_URL_PREFIX

rename to BASE_URL_PREFIX
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -26,2 +26,4 @@
private Button sign;

private static final String HTTPS = "https://";
private static final String BUBBLE_API = ":1443/api/";
gegham commented 4 years ago

Rename to BASE_URL_SUFFIX

Rename to BASE_URL_SUFFIX
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -42,3 +45,3 @@
sign.setOnClickListener(new OnClickListener() {
@Override public void onClick(final View v) {
final String tunnelName = bubbleName.getText().toString().trim();
final String url = HTTPS + bubbleName.getText().toString() + BUBBLE_API;
gegham commented 4 years ago

Make a validity check on the URL format, if it’s not valid, return error

Make a validity check on the URL format, if it's not valid, return error
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -45,2 +48,4 @@
loginViewModel.buildRepositoryInstance(LoginActivity.this,url);
loginViewModel.setUserURL(LoginActivity.this,url);
final String username = userName.getText().toString().trim();
final String password = LoginActivity.this.password.getText().toString().trim();
gegham commented 4 years ago

Why do we need to access the property like this?
LoginActivity.this.password

Why do we need to access the property like this? LoginActivity.this.password
musheghsahakyan98 commented 4 years ago

local variable have same name

local variable have same name
ui/src/main/java/com/wireguard/android/activity/MainActivity.java
@@ -67,0 +68,4 @@
ObservableTunnel tunnel = mainViewModel.getTunnel(this, connectionStateFlag);
pendingTunnel = tunnel;
mainViewModel.getTunnelManager().getTunnelState(pendingTunnel).whenComplete((state, throwable) -> {
if (state == State.DOWN) {
gegham commented 4 years ago

Why do we set a connection flag to true when state is down?

Why do we set a connection flag to true when state is down?
musheghsahakyan98 commented 4 years ago

if it does not work, turn it on and set the flag to true

if it does not work, turn it on and set the flag to true
ui/src/main/java/com/wireguard/android/api/ApiConstants.java
@@ -12,4 +11,5 @@ public class ApiConstants {
public static final String AUTHORIZATION_HEADER = "X-Bubble-Session";
public static final String DEVICE_NAME = "name";
public static final String DEVICE_TYPE = "deviceType";
public static final String TUNNEL_NAME = "BubbleVPN";
gegham commented 4 years ago

This is not connected with the API, it should be located in another class.

This is not connected with the API, it should be located in another class.
ui/src/main/java/com/wireguard/android/viewmodel/MainViewModel.java
@@ -12,0 +31,4 @@
return DataRepository.getRepositoryInstance().isBubbleConnected(context);
}

public void setConnectionState(final Context context, final boolean state, final String stateConnection){
gegham commented 4 years ago

The connection state should not be saved manually by the app but taken from the tunnel every time when needed.

The connection state should not be saved manually by the app but taken from the tunnel every time when needed.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -45,2 +59,4 @@
}
loginViewModel.setUserURL(LoginActivity.this,url);
final String username = userName.getText().toString().trim();
final String password = LoginActivity.this.password.getText().toString().trim();
gegham commented 4 years ago

Why do we need to access the property like this? LoginActivity.this.password.getText().toString().trim();
Why not just: password.getText().toString().trim();

Why do we need to access the property like this? LoginActivity.this.password.getText().toString().trim(); Why not just: password.getText().toString().trim();
musheghsahakyan98 commented 4 years ago

password is a local variable i have be use this + that is anonym class use LoginActivity.this

password is a local variable i have be use this + that is anonym class use LoginActivity.this
gegham commented 4 years ago

Then just rename String username to usernameInput, and password to passwordInput.

Then just rename String username to usernameInput, and password to passwordInput.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -44,1 +49,3 @@
final String tunnelName = bubbleName.getText().toString().trim();
final String url = BASE_URL_PREFIX + bubbleName.getText().toString() + BASE_URL_SUFFIX;
if(url.split(SEPARATOR).length!=3){
Toast.makeText(LoginActivity.this,"Hostname not valid",Toast.LENGTH_LONG).show();
gegham commented 4 years ago

The string is hardcoded. Put in the string resources.

The string is hardcoded. Put in the string resources.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/viewmodel/MainViewModel.java
@@ -12,0 +16,4 @@
return !UserStore.USER_TOKEN_DEFAULT_VALUE.equals(UserStore.getInstance(context).getToken());
}

public ObservableTunnel getTunnel(Context context, boolean stateTunnel) {
gegham commented 4 years ago

The UI shouldn’t have a tunnel object either. It should have only the end results that it needs. Like methods isVPNConnected, if it listens to an interface then it should just get the interface. All the logic should be connected with UI only.

The UI shouldn't have a tunnel object either. It should have only the end results that it needs. Like methods isVPNConnected, if it listens to an interface then it should just get the interface. All the logic should be connected with UI only.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/activity/LoginActivity.java
@@ -71,0 +88,4 @@
loginViewModel.getCertificate(LoginActivity.this).observe(LoginActivity.this, new Observer<String>() {
@Override public void onChanged(final String s) {
closeLoadingDialog();
final byte[] cert = s.getBytes();
gegham commented 4 years ago

This logic should not happen in the LoginActivity. The login activity should directly receive an encoded certificate just to put it in the intent.

This logic should not happen in the LoginActivity. The login activity should directly receive an encoded certificate just to put it in the intent.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/repository/DataRepository.java
@@ -321,0 +375,4 @@
.build();
client.newCall(request).enqueue(new Callback() {
@Override public void onFailure(final okhttp3.Call call, final IOException e) {

gegham commented 4 years ago

Handle errors and exception catches correctly. Return an error, otherwise, during errors, we will get an infinite loading dialog.

Handle errors and exception catches correctly. Return an error, otherwise, during errors, we will get an infinite loading dialog.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/activity/MainActivity.java
@@ -44,0 +39,4 @@

@Override protected void onResume() {
super.onResume();
if (mainViewModel.isVPNConnected(this, connectionStateFlag)) {
gegham commented 4 years ago

Encapsulate this code in a method: setConnectionStateUI(boolean state){
if (state) {
Toast.makeText(MainActivity.this, getString(R.string.connected_bubble), Toast.LENGTH_SHORT).show();
bubbleStatus.setText(getString(R.string.connected_bubble));
connectButton.setText(getString(R.string.disconnect));
} else {
Toast.makeText(MainActivity.this, getString(R.string.not_connected_bubble), Toast.LENGTH_SHORT).show();
bubbleStatus.setText(getString(R.string.not_connected_bubble));
connectButton.setText(getString(R.string.connect));
}
}
After this, you can reuse this method.

Encapsulate this code in a method: setConnectionStateUI(boolean state){ if (state) { Toast.makeText(MainActivity.this, getString(R.string.connected_bubble), Toast.LENGTH_SHORT).show(); bubbleStatus.setText(getString(R.string.connected_bubble)); connectButton.setText(getString(R.string.disconnect)); } else { Toast.makeText(MainActivity.this, getString(R.string.not_connected_bubble), Toast.LENGTH_SHORT).show(); bubbleStatus.setText(getString(R.string.not_connected_bubble)); connectButton.setText(getString(R.string.connect)); } } After this, you can reuse this method.
gegham requested changes 4 years ago
ui/src/main/java/com/wireguard/android/activity/MainActivity.java
@@ -104,0 +90,4 @@
});
} else {
connectionStateFlag = false;
Toast.makeText(this, getString(R.string.not_connected_bubble), Toast.LENGTH_SHORT).show();
gegham commented 4 years ago

use the setConnectionState(false)

use the setConnectionState(false)
gegham approved these changes 4 years ago
gegham closed this pull request 4 years ago
gegham deleted branch connection_functionality 4 years ago

Reviewers

gegham approved these changes 4 years ago
The pull request has been merged as ee7304b9f3.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
 gegham
Labels
Apply labels
Clear labels
No items
No Label
Milestone
Set milestone
Clear milestone
No items
No Milestone
Assignees
Assign users
Clear assignees
No Assignees
 gegham
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'connection_functionality'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes