-
Notifications
You must be signed in to change notification settings - Fork 240
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
first commit #29
base: main
Are you sure you want to change the base?
first commit #29
Conversation
@Tag(name = "Product controller", description = "Endpoints for API") | ||
@RestController | ||
@RequestMapping(value = "/rick_morty") | ||
public class ApplicationController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class ApplicationController { | |
public class CharacterController { |
|
||
@Tag(name = "Product controller", description = "Endpoints for API") | ||
@RestController | ||
@RequestMapping(value = "/rick_morty") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix all URLs according to convention
|
||
@Data | ||
@JsonIgnoreProperties(ignoreUnknown = true) | ||
public class RickMortyDtoResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at class field CharacterDtoResponse is more suitable name here
public class RickMortyDtoResponse { | |
public class CharacterDtoResponse { |
@Entity | ||
@Data | ||
@Table(name = "characters") | ||
public class RickMorty { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class RickMorty { | |
public class Character { |
|
||
@Service | ||
public class RickMortyServiceImpl implements RickMortyService { | ||
private static final String characterURL = "https://rickandmortyapi.com/api/character"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uppercase
this.restTemplate = restTemplate; | ||
this.rickMortyRepository = rickMortyRepository; | ||
this.rickMortyMapper = rickMortyMapper; | ||
downloadToDB(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use PostCunstruct
downloadToDB(); |
@Service | ||
public class RickMortyServiceImpl implements RickMortyService { | ||
private static final String characterURL = "https://rickandmortyapi.com/api/character"; | ||
private static boolean isInitialized = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static boolean isInitialized = false; |
} | ||
|
||
public RickMorty getRandomCharacter() { | ||
Random random = new Random(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make it class fied
public RickMortyMapperImpl() { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't undestand the question! I need this implementation because we must convert responce object to model))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mapstruct should create Mapper impl - or why do you need its dependency in pom.xml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a problem with this!)) I did with Mapper, but during maven test I have checkstyle error in target package, MapperImpl class(which generates in the runtime) and I can't fix this!
} | ||
|
||
@Operation(summary = "Find random character", description = "Get random character") | ||
@GetMapping("/random-character") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/characters/random-character ? Too many "character"
@GetMapping("/random-character") | |
@GetMapping("/random") |
import mate.academy.rickandmorty.dto.CharacterDtoResponse; | ||
import mate.academy.rickandmorty.models.Character; | ||
|
||
public interface RickMortyMapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public interface RickMortyMapper { | |
public interface CharacterMapper { |
import org.springframework.stereotype.Repository; | ||
|
||
@Repository | ||
public interface RickMortyRepository extends JpaRepository<Character, Long> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public interface RickMortyRepository extends JpaRepository<Character, Long> { | |
public interface CharacterRepository extends JpaRepository<Character, Long> { |
import java.util.List; | ||
import mate.academy.rickandmorty.models.Character; | ||
|
||
public interface RickMortyService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public interface RickMortyService { | |
public interface CharacterService { |
public RickMortyMapperImpl() { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mapstruct should create Mapper impl - or why do you need its dependency in pom.xml?
private CharacterService characterService; | ||
|
||
@Operation(summary = "Find all characters by name", description = "Get all characters by name") | ||
@GetMapping("/by-name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URI convention violation.
see:
https://restfulapi.net/resource-naming/
|
||
@Operation(summary = "Find random character", description = "Get random character") | ||
@GetMapping("/random") | ||
public Character getRandomCharacter() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URI convention violation.
see:
https://restfulapi.net/resource-naming/
|
||
@Service | ||
public class CharacterServiceImpl implements CharacterService { | ||
private static final String CHARACTER_URL = "https://rickandmortyapi.com/api/character"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: move to property file.
private static final String CHARACTER_URL = "https://rickandmortyapi.com/api/character"; | |
private static final String CHARACTER_URL = "https://rickandmortyapi.com/api/character"; |
this.restTemplate = restTemplate; | ||
this.characterRepository = characterRepository; | ||
this.rickMortyMapper = rickMortyMapper; | ||
random = new Random(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Spring @bean for injection Random class
.
private void downloadToDB() { | ||
CharactersDtoResponse charactersDtoResponse = restTemplate.getForObject(CHARACTER_URL, | ||
CharactersDtoResponse.class); | ||
assert charactersDtoResponse != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using assert
out of tests is bad practice.
Consider throwing custom exception.
assert charactersDtoResponse != null; |
spring.datasource.url=jdbc:mysql://localhost:3306/rick_morty | ||
spring.datasource.username=serg | ||
spring.datasource.password=22062020 | ||
spring.datasource.driver-class-name=com.mysql.cj.jdbc.Driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this property required to be used?
should be removed if not.
spring.datasource.driver-class-name=com.mysql.cj.jdbc.Driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
if (charactersDtoResponse != null) { | ||
characterRepository.saveAll(charactersDtoResponse.getResults() | ||
.stream() | ||
.map(rickMortyMapper::toModel) | ||
.collect(Collectors.toList())); | ||
} else { | ||
throw new CustomException("Characters response object is NULL!!"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would be more compact
if (charactersDtoResponse != null) { | |
characterRepository.saveAll(charactersDtoResponse.getResults() | |
.stream() | |
.map(rickMortyMapper::toModel) | |
.collect(Collectors.toList())); | |
} else { | |
throw new CustomException("Characters response object is NULL!!"); | |
} | |
if (charactersDtoResponse == null) { | |
throw new CustomException("Characters response object is NULL!!"); | |
} | |
characterRepository.saveAll(charactersDtoResponse.getResults() | |
.stream() | |
.map(rickMortyMapper::toModel) | |
.collect(Collectors.toList())); |
No description provided.