Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Domain logic bug in the models #73

Merged
merged 7 commits into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.stepango.blockme.feature.characters.list.api.domain.model
package com.stepango.blockme.feature.characters.core.api.domain.model

interface ICharacterItem {
interface ICharacter {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good improvement!
Now we have only 3 types of "Character"!
But we can do more, what do you think, if we'll remove CharacterItem and CharacterDetail? I think about added single entity implementation of ICharacter into core:impl, for example
class Character : ICharacter
And other domains Character Details/Items/Favorites will use only ICharacter type with default core implementation under hood.

In that case, will be only one Character single entity with unique structure placed in the core target.
What do you think?

val id: Long
val name: String
val description: String
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package com.stepango.blockme.feature.characters.core.api.domain.repository

import com.stepango.blockme.core.network.library.response.BaseResponse
import com.stepango.blockme.feature.characters.core.api.data.response.CharacterResponse
import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter

interface MarvelRepository {

suspend fun getCharacter(id: Long): BaseResponse<CharacterResponse>
suspend fun getCharacter(id: Long): ICharacter

suspend fun getCharacters(offset: Int, limit: Int): BaseResponse<CharacterResponse>
suspend fun getCharacters(offset: Int, limit: Int): List<ICharacter>
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,28 @@
* limitations under the License.
*/

package com.stepango.blockme.feature.characters.list.impl.data.mapper
package com.stepango.blockme.feature.characters.core.impl.data.mapper

import com.stepango.blockme.common.util.mapper.Mapper
import com.stepango.blockme.core.network.library.response.BaseResponse
import com.stepango.blockme.feature.characters.core.api.data.response.CharacterResponse
import com.stepango.blockme.feature.characters.list.impl.domain.model.CharacterItem
import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter
import com.stepango.blockme.feature.characters.core.impl.domain.model.Character

private const val IMAGE_URL_FORMAT = "%s.%s"

open class CharacterItemMapper : Mapper<BaseResponse<CharacterResponse>, List<CharacterItem>> {
class CharacterMapper : Mapper<BaseResponse<CharacterResponse>, List<ICharacter>> {

override suspend fun map(from: BaseResponse<CharacterResponse>) =
from.data.results.map {
CharacterItem(
id = it.id,
name = it.name,
description = it.description,
@Throws(NoSuchElementException::class)
override suspend fun map(from: BaseResponse<CharacterResponse>): List<ICharacter> =
from.data.results.map { characterResponse ->
Character(
id = characterResponse.id,
name = characterResponse.name,
description = characterResponse.description,
imageUrl = IMAGE_URL_FORMAT.format(
it.thumbnail.path.replace("http", "https"),
it.thumbnail.extension
characterResponse.thumbnail.path.replace("http", "https"),
characterResponse.thumbnail.extension
)
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package com.stepango.blockme.feature.characters.core.impl.di

import com.stepango.blockme.common.util.clock.Clock
import com.stepango.blockme.common.util.mapper.Mapper
import com.stepango.blockme.core.network.library.Config
import com.stepango.blockme.core.network.library.response.BaseResponse
import com.stepango.blockme.feature.characters.core.api.data.response.CharacterResponse
import com.stepango.blockme.feature.characters.core.api.data.service.MarvelService
import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter
import com.stepango.blockme.feature.characters.core.api.domain.repository.MarvelRepository
import com.stepango.blockme.feature.characters.core.impl.data.mapper.CharacterMapper
import com.stepango.blockme.feature.characters.core.impl.domain.repository.ServiceMarvelRepository
import dagger.Module
import dagger.Provides
Expand All @@ -17,8 +22,16 @@ internal class CharactersCoreModule {
@Provides
fun provideMarvelService(retrofit: Retrofit): MarvelService = retrofit.create(MarvelService::class.java)

@Singleton
@Provides
fun provideMarvelRepository(service: MarvelService, config: Config, clock: Clock): MarvelRepository =
ServiceMarvelRepository(service, config, clock)
@Singleton
@Provides
fun provideMarvelRepository(
service: MarvelService,
config: Config,
clock: Clock,
characterMapper: Mapper<BaseResponse<CharacterResponse>, List<ICharacter>>,
): MarvelRepository =
ServiceMarvelRepository(service, config, clock, characterMapper)

@Provides
fun provideCharacterMapper(): Mapper<BaseResponse<CharacterResponse>, List<ICharacter>> = CharacterMapper()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.stepango.blockme.feature.characters.core.impl.domain.model

import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter

data class Character(
override val id: Long,
override val name: String,
override val description: String,
override val imageUrl: String
) : ICharacter
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ package com.stepango.blockme.feature.characters.core.impl.domain.repository

import com.stepango.blockme.common.extensions.util.toMD5
import com.stepango.blockme.common.util.clock.Clock
import com.stepango.blockme.common.util.mapper.Mapper
import com.stepango.blockme.core.network.library.Config
import com.stepango.blockme.core.network.library.response.BaseResponse
import com.stepango.blockme.feature.characters.core.api.data.response.CharacterResponse
import com.stepango.blockme.feature.characters.core.api.data.service.MarvelService
import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter
import com.stepango.blockme.feature.characters.core.api.domain.repository.MarvelRepository
import javax.inject.Inject

Expand All @@ -34,6 +36,7 @@ internal class ServiceMarvelRepository @Inject constructor(
private val service: MarvelService,
private val config: Config,
private val clock: Clock,
private val characterMapper: Mapper<BaseResponse<CharacterResponse>, List<ICharacter>>
) : MarvelRepository {

/**
Expand All @@ -42,15 +45,17 @@ internal class ServiceMarvelRepository @Inject constructor(
* @param id A single character id.
* @return Response for single character resource.
*/
override suspend fun getCharacter(id: Long): BaseResponse<CharacterResponse> {
override suspend fun getCharacter(id: Long): ICharacter {
val timestamp = clock.currentTimeMillis().toString()

return service.getCharacter(
val result = service.getCharacter(
id = id,
apiKey = config.publicApiKey,
hash = generateApiHash(timestamp),
timestamp = timestamp
)

return characterMapper.map(result).first()
}

/**
Expand All @@ -60,15 +65,17 @@ internal class ServiceMarvelRepository @Inject constructor(
* @param limit Limit the result set to the specified number of resources.
* @return Response for comic characters resource.
*/
override suspend fun getCharacters(offset: Int, limit: Int): BaseResponse<CharacterResponse> {
override suspend fun getCharacters(offset: Int, limit: Int): List<ICharacter> {
val timestamp = clock.currentTimeMillis().toString()
return service.getCharacters(
val result = service.getCharacters(
apiKey = config.publicApiKey,
hash = generateApiHash(timestamp),
timestamp = timestamp,
offset = offset,
limit = limit
)

return characterMapper.map(result)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package com.stepango.blockme.feature.characters.core.impl.domain.repository

import com.stepango.blockme.common.extensions.util.toMD5
import com.stepango.blockme.common.util.clock.Clock
import com.stepango.blockme.common.util.mapper.Mapper
import com.stepango.blockme.core.network.library.Config
import com.stepango.blockme.core.network.library.response.BaseResponse
import com.stepango.blockme.feature.characters.core.api.data.response.CharacterResponse
import com.stepango.blockme.feature.characters.core.api.data.service.MarvelService
import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter
import com.stepango.blockme.feature.characters.core.api.domain.repository.MarvelRepository
import io.mockk.coEvery
import io.mockk.every
Expand All @@ -30,37 +32,42 @@ internal class ServiceMarvelRepositoryTest {
private val config: Config = mockk()
private val marvelService: MarvelService = mockk()
private val clock: Clock = mockk()
private val characterMapper: Mapper<BaseResponse<CharacterResponse>, List<ICharacter>> = mockk()

private val repository: MarvelRepository = ServiceMarvelRepository(marvelService, config, clock)
private val repository: MarvelRepository = ServiceMarvelRepository(marvelService, config, clock, characterMapper)

@Test
fun `get a character EXPECT the base response`() = runBlocking {
val expectedResponse: BaseResponse<CharacterResponse> = mockk()
val response: BaseResponse<CharacterResponse> = mockk()
val expectedMappedResponse: ICharacter = mockk()

every { clock.currentTimeMillis() } returns TIMESTAMP
every { config.privateApiKey } returns API_PRIVATE_KEY
every { config.publicApiKey } returns API_PUBLIC_KEY
coEvery { marvelService.getCharacter(ID, API_PUBLIC_KEY, HASH, TIMESTAMP.toString()) } returns expectedResponse
coEvery { marvelService.getCharacter(ID, API_PUBLIC_KEY, HASH, TIMESTAMP.toString()) } returns response
coEvery { characterMapper.map(response) } returns listOf(expectedMappedResponse)

val result = repository.getCharacter(ID)

assertEquals(expectedResponse, result)
assertEquals(expectedMappedResponse, result)
}

@Test
fun `get characters EXPECT the base response`() = runBlocking {
val offset = 10
val limit = 20

val expectedResponse: BaseResponse<CharacterResponse> = mockk()
val response: BaseResponse<CharacterResponse> = mockk()
val expectedMappedResponse: List<ICharacter> = mockk()

every { clock.currentTimeMillis() } returns TIMESTAMP
every { config.privateApiKey } returns API_PRIVATE_KEY
every { config.publicApiKey } returns API_PUBLIC_KEY
coEvery { marvelService.getCharacters(API_PUBLIC_KEY, HASH, TIMESTAMP.toString(), offset, limit) } returns expectedResponse
coEvery { marvelService.getCharacters(API_PUBLIC_KEY, HASH, TIMESTAMP.toString(), offset, limit) } returns response
coEvery { characterMapper.map(response) } returns expectedMappedResponse

val result = repository.getCharacters(offset, limit)

assertEquals(expectedResponse, result)
assertEquals(expectedMappedResponse, result)
}
}
5 changes: 4 additions & 1 deletion application/feature/characters/detail/api/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
api(
packageName = "com.stepango.blockme.character.detail.api",
owner = Teams.core
owner = Teams.core,
dependencies = deps(
target(":feature:characters:core:api")
)
)
Empty file.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package com.stepango.blockme.feature.characters.detail.impl.presentation

import androidx.lifecycle.LiveData
import com.stepango.blockme.feature.characters.detail.api.domain.model.ICharacterDetail
import com.stepango.blockme.feature.characters.core.api.domain.model.ICharacter

interface ICharacterDetailViewModel {

val data: LiveData<ICharacterDetail>
val data: LiveData<ICharacter>
val state: LiveData<ICharacterDetailViewState>

fun loadCharacterDetail(characterId: Long)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ package com.stepango.blockme.feature.characters.detail.impl.di
import androidx.lifecycle.ViewModel
import com.stepango.blockme.core.di.library.scopes.FeatureScope
import com.stepango.blockme.core.mvvm.library.di.ViewModelKey
import com.stepango.blockme.feature.characters.detail.impl.data.mapper.CharacterDetailMapper
import com.stepango.blockme.feature.characters.detail.impl.presentation.CharacterDetailViewModel
import dagger.Binds
import dagger.Module
import dagger.Provides
import dagger.multibindings.IntoMap

@Module
Expand All @@ -34,11 +32,4 @@ internal abstract class CharacterDetailModule {
@IntoMap
@ViewModelKey(CharacterDetailViewModel::class)
abstract fun bindsCharactersDetailViewModel(viewModel: CharacterDetailViewModel): ViewModel

companion object {

@FeatureScope
@Provides
fun providesCharacterDetailMapper() = CharacterDetailMapper()
}
}

This file was deleted.

Loading