Skip to content
Snippets Groups Projects

Users served in the past 24hrs

Users served in the past 24hrs
HashikD/snowflake-mobile:users-served into master
2 unresolved threads
Merged HashikD requested to merge HashikD/snowflake-mobile:users-served into master
2 unresolved threads

closes #6 (closed)

Edited by HashikD

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
app/src/main/java/org/torproject/snowflake/MyPersistentService.java
308 320
309 321 if (STATE == DataChannel.State.OPEN) {
310 322 updateNotification("Connection Established. Serving one client.");
323 updateServedCount();
  • Cecylia Bocovich
    Cecylia Bocovich @cohosh ·
    Owner

    I just ran this in the emulator and I had a client connect successfully but I never see this function being called. Did you test it out and get it working? My users served log messages are also never going past 0 even though I'm fully bootstrapping clients through the proxy.

  • HashikD
    HashikD @HashikD ·
    Author Maintainer

    That's very weird, I tested it and it was working fine. If you see "Connection Established..." it should've been updated. But, I'll give it another try and update you in a couple of minutes. Maybe I missed something since you are saying the bootstrapping reached 100%

  • Cecylia Bocovich
    Cecylia Bocovich @cohosh ·
    Owner

    Yeah for me the whole code path wasn't getting executed so I didn't see "Connection Established" either.

  • HashikD
    HashikD @HashikD ·
    Author Maintainer

    @cohosh It's working fine for me... it only updates if the connection is established (i.e when DataChannel is Open) can you restart your Snowbox? can you confirm that you are looking at the log of the correct client?

    Additionally, can you please confirm that you are checking out the right branch? this is a trivial suggestion but worth looking out I guess.

  • Cecylia Bocovich
    Cecylia Bocovich @cohosh ·
    Owner

    Ah alright I'm seeing the notification and that the code path is being executed (I was looking for it in the logs). I'm not seeing the UI update though. Even if I switch to settings and back, or toggle the services off and on the users served stays the same. The only way I got it to update was by killing the app and restarting it.

    Is there a good way to address this here, or should we do it in the changes for #11 (closed) ?

  • HashikD
    HashikD @HashikD ·
    Author Maintainer

    @cohosh made the patch. Please try again.

  • Please register or sign in to reply
  • Cecylia Bocovich
    Cecylia Bocovich @cohosh started a thread on an outdated change in commit a6ce892d
    • app/src/main/res/values/strings.xml
    5 5 <string name="not_channel_desc">This Channel should not be muted. The Android system will consider Snowflake not import and will kill the service if it\'s muted.</string>
    6 6 <string name="initial_run_boolean">initial_run</string>
    7 7 <string name="not_channel_name">Snowflake Service</string>
    8 <string name="users_served">users_served</string>
    9 <string name="served_date">date</string>
    • Cecylia Bocovich
      Cecylia Bocovich @cohosh ·
      Owner

      Storing this data as strings is an interesting approach. Is there another way to store data rather than putting it in the same place as our string constants?

    • HashikD
      HashikD @HashikD ·
      Author Maintainer

      The resource compiler looks for all the "strings.xml" files, we can create a separate file. I'll do that if you want me to. And let me know how to segregate the strings and files. Another approach is to comment on this same file separating/diving the file into sections.

      Edited by HashikD
    • Cecylia Bocovich
      Cecylia Bocovich @cohosh ·
      Owner

      Let's make them completely separate files at least. In general I don't like the idea of storing these values in the application's "preferences" since they are stored usage data and don't have anything to do with preferences. It seems like we should be storing this using the app's internal storage. Can we also store the count of users served directly as an int and not a string?

    • HashikD
      HashikD @HashikD ·
      Author Maintainer

      Sorry, I am a little confused. SharedPrefernces are widely used for storing data in Android.

      Apart from that, there is SQLite to store data but it's used in cases where we have to search through a bunch of data.

      For our use case, SharedPreferences are perfect since we are having just key-value pairs, not a repeated set of data. Many apps actually use it to store session data as-well like cookies (but mostly in encrypted form)

      Can we also store the count of users served directly as an int and not a string?

      We are, hence the call "getInt" and "putInt" on shared preference handling users_served.

      Does this answer what you were asking? or am I talking about different things? I am not sure what you meant by the app's internal storage tho, we can call these shared preferences the internal storage for the application.

      Edited by HashikD
    • Cecylia Bocovich
      Cecylia Bocovich @cohosh ·
      Owner

      SharedPrefernces are widely used for storing data in Android. For our use case, SharedPreferences are perfect since we are having just key-value pairs, not a repeated set of data. Many apps actually use it to store session data as-well like cookies (but mostly in encrypted form)

      Okay that's fine, I'll defer to your judgement here. But we should still split out these files to avoid confusion. IMO we should have one file for string constants, and one file for stored data.

      We are, hence the call "getInt" and "putInt" on shared preference handling users_served.

      We're still storing them as strings in the xml file: <string name="users_served">users_served</string> , and handling them as strings before the call to getInt: sp.getInt(getString(R.string.users_served), 0) + 1)

      If this is how things are usually done, that's fine. But it surprises me that we'd have to store this value as a string at all.

    • HashikD
      HashikD @HashikD ·
      Author Maintainer

      Ah okay! Now I got what you are saying. Anything is fine, Android OS will compile these String resources and put it on to the main memory into a lookup table. So, there is no performance benefit to either. So, we can create constants.

      But, Android official documentation did it our way, and personally I got used to it and many devs do it too.

    • Cecylia Bocovich
      Cecylia Bocovich @cohosh ·
      Owner

      Okay awesome, thanks!

    • HashikD
      HashikD @HashikD ·
      Author Maintainer

      Will make a patch splitting them.

    • HashikD changed this line in version 5 of the diff

      changed this line in version 5 of the diff

    • HashikD
      HashikD @HashikD ·
      Author Maintainer

      Made the patch. I named keys with trailing "_key" to recognize a key in the Java code.

    • Please register or sign in to reply
  • HashikD added 1 commit

    added 1 commit

    • 689949d2 - Updating the count of users served even when the app is in foreground

    Compare with previous version

  • HashikD added 1 commit

    added 1 commit

    • dafed99d - Moved and renamed String.xml SharedPreference keys

    Compare with previous version

  • Cecylia Bocovich
    Cecylia Bocovich @cohosh ·
    Owner

    Alright let's merge this. Thanks @HashikD!

  • Gitolite Merge Bot merged

    merged

    • Please register or sign in to reply