#8/create a class for savind session #22

temnik15 · 2020-11-08T12:48:40Z

A class has been created to persist the session while the application is running and for long-term storage.
Set the initialization of this class when starting the application

Shiplayer · 2020-11-08T12:57:57Z

app/build.gradle

@@ -33,5 +33,8 @@ dependencies {
    testImplementation 'junit:junit:4.12'
    androidTestImplementation 'androidx.test.ext:junit:1.1.2'
    androidTestImplementation 'androidx.test.espresso:espresso-core:3.3.0'
-
+    // https://mvnrepository.com/artifact/io.reactivex.rxjava3/rxjava
+    implementation group: 'io.reactivex.rxjava3', name: 'rxjava', version: '3.0.0-RC1'


Зачем две одинаковых зависимости?

@Shiplayer разве они одинаковые? без второй с name 'android' становятся не доступны возможности RxJava для android (Например AndroidSchedulers.mainThread() из io.reactivex.rxjava3.android.schedulers.AndroidSchedulers

Наверное, не заметил, что name был разный

Подними лучше у rxjava до 3.0.7

Shiplayer · 2020-11-08T13:00:42Z

app/src/main/java/so/codex/hawk/SessionKeeper.kt

+     * @param context Must be an applicationContext to avoid memory leaks.
+     */
+    fun init(context: Context) {
+        this.context = context


Может стоит как то обезопасить этот момент? Хотя бы бросить эксепшен, если попытаться его заново инициализировать

Я тут думаю, может лучше не исключение кидать, а просто проверять через if что поле уже инициализировано? и все последующие вызовы не будут никак влиять

Shiplayer · 2020-11-08T13:01:21Z

app/src/main/java/so/codex/hawk/SessionKeeper.kt

+     * @param preferences sharedPreferences saved on the device.
+     * @see init
+     */
+    private fun getSessionFromPref(preferences: SharedPreferences) {


Не похоже, чтобы он что-то возвращал

Shiplayer · 2020-11-08T15:05:59Z

app/src/main/java/so/codex/hawk/SessionKeeper.kt

+    /**
+     * @property KEY_ACCESS_TOKEN The key to get the access token value stored on the device.
+     */
+    const val KEY_ACCESS_TOKEN = "ACCESS_TOKEN"


Почему такие поля не приватные?

Это как раз то о чем я хотел рассказать при созвоне) Есть один нюанс с пробросом обновления сессии подписчикам

Shiplayer · 2020-11-08T15:07:36Z

app/src/main/java/so/codex/hawk/SessionKeeper.kt

+     *                 the observer will receive the last session update and will continue
+     *                 to receive updates until the observable sends an .onComplete().
+     */
+    fun subscribeToSessionUpdate(observer: Observer<Pair<String, String>>) {


Я думаю, что лучше не запрашивать обзервер, а предоставлять обсервабл, чтобы они решали, где им подписываться

Так ещё получается, что этот подписчик никогда не отпишется и будет меморилик

Shiplayer · 2020-11-09T12:25:43Z

app/src/main/java/so/codex/hawk/SessionKeeper.kt

+     * @return [Observable] <[Pair] <String, String >> when subscribing to which it will be possible
+     *                                                 to handle the session change.
+     */
+    fun getObservableSession(): Observable<Pair<String, String>> {


почему Pair? может нам стоит возвращать Session или просто Token

Shiplayer · 2020-11-09T13:08:06Z

app/src/main/java/so/codex/hawk/entity/Session.kt

+data class Session(val token:Token,val time:Long)
+data class Token(val accessToken: String, val refreshToken: String)


Добавь документацию

Shiplayer · 2020-11-09T13:08:21Z

app/src/main/java/so/codex/hawk/HawkApp.kt

+class HawkApp: Application() {
+
+    override fun onCreate() {
+        super.onCreate()
+        SessionKeeper.init(applicationContext)
+    }
+}


И сюда тоже документацию добавь

Shiplayer · 2020-11-09T13:10:39Z

app/build.gradle

+    // https://mvnrepository.com/artifact/io.reactivex.rxjava3/rxjava
+    implementation group: 'io.reactivex.rxjava3', name: 'rxjava', version: '3.0.7'
+    // https://mvnrepository.com/artifact/io.reactivex.rxjava3/rxandroid
+    implementation group: 'io.reactivex.rxjava3', name: 'rxandroid', version: '3.0.0'


Сделай над общим коммитом и чтобы выглядело однообразно

Suggested change

// https://mvnrepository.com/artifact/io.reactivex.rxjava3/rxjava

implementation group: 'io.reactivex.rxjava3', name: 'rxjava', version: '3.0.7'

// https://mvnrepository.com/artifact/io.reactivex.rxjava3/rxandroid

implementation group: 'io.reactivex.rxjava3', name: 'rxandroid', version: '3.0.0'

// RxJava 3

implementation 'io.reactivex.rxjava3:rxjava:3.0.7'

implementation 'io.reactivex.rxjava3:rxandroid:3.0.0'

Shiplayer

отформатируй код

Shiplayer · 2020-11-09T13:20:10Z

app/src/main/java/so/codex/hawk/entity/Session.kt

@@ -0,0 +1,4 @@
+package so.codex.hawk.entity
+
+data class Session(val token:Token,val time:Long)


пробел после , поставь и после :

Shiplayer · 2020-11-11T11:23:44Z

app/src/main/java/so/codex/hawk/SessionKeeper.kt

+                    val value = pref.getString(key, "")!!
+                    prefSubject.onNext(Token(value, ""))


почему мы должны отправлять только accessToken, если у нас изменились все данные?
мы ведь знаем, что уведомлять слушателей будут уже после того, как все указанные данные изменятся в файле

думаю, что тут можно будет воспользоваться методом restoreSessionFromPref и после него отправить данные в сабджект

…Java 3.

temnik15 linked an issue Nov 8, 2020 that may be closed by this pull request

Создать класс для сохранения сессии #8

Closed

5 tasks

temnik15 requested review from nikmel2803 and Shiplayer November 8, 2020 12:49

Shiplayer reviewed Nov 8, 2020

View reviewed changes

Shiplayer suggested changes Nov 8, 2020

View reviewed changes

temnik15 force-pushed the #8/create-a-class-for-savind-session branch from 1790da8 to 6590144 Compare November 8, 2020 15:08

temnik15 requested a review from Shiplayer November 8, 2020 15:28

Shiplayer reviewed Nov 9, 2020

View reviewed changes

temnik15 requested review from Shiplayer and neSpecc November 9, 2020 13:18

Shiplayer reviewed Nov 9, 2020

View reviewed changes

temnik15 requested a review from Shiplayer November 9, 2020 13:32

temnik15 force-pushed the #8/create-a-class-for-savind-session branch 4 times, most recently from 0bda22e to d47a5a7 Compare November 11, 2020 11:12

Shiplayer suggested changes Nov 11, 2020

View reviewed changes

temnik15 force-pushed the #8/create-a-class-for-savind-session branch 2 times, most recently from 43c80a0 to b29f2f4 Compare November 11, 2020 18:39

temnik15 requested a review from Shiplayer November 11, 2020 18:43

Shiplayer approved these changes Nov 11, 2020

View reviewed changes

nikmel2803 approved these changes Nov 12, 2020

View reviewed changes

#8: created a class for saving the session, added dependencies for Rx…

f30146c

…Java 3.

temnik15 force-pushed the #8/create-a-class-for-savind-session branch from b29f2f4 to f30146c Compare November 12, 2020 22:11

temnik15 merged commit 995b37e into main Nov 12, 2020

temnik15 deleted the #8/create-a-class-for-savind-session branch November 12, 2020 22:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#8/create a class for savind session #22

#8/create a class for savind session #22

temnik15 commented Nov 8, 2020

Shiplayer Nov 8, 2020

temnik15 Nov 8, 2020

Shiplayer Nov 8, 2020

Shiplayer Nov 8, 2020

Shiplayer Nov 8, 2020

temnik15 Nov 8, 2020

Shiplayer Nov 8, 2020

Shiplayer Nov 8, 2020

temnik15 Nov 8, 2020

Shiplayer Nov 8, 2020

Shiplayer Nov 8, 2020

Shiplayer Nov 9, 2020

Shiplayer Nov 9, 2020

Shiplayer Nov 9, 2020

Shiplayer Nov 9, 2020

Shiplayer left a comment

Shiplayer Nov 9, 2020

Shiplayer Nov 11, 2020

Shiplayer Nov 11, 2020

		data class Session(val token:Token,val time:Long)
		data class Token(val accessToken: String, val refreshToken: String)

		@@ -0,0 +1,4 @@
		package so.codex.hawk.entity

		data class Session(val token:Token,val time:Long)

		val value = pref.getString(key, "")!!
		prefSubject.onNext(Token(value, ""))

#8/create a class for savind session #22

#8/create a class for savind session #22

Conversation

temnik15 commented Nov 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shiplayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment