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

init commit #151

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
44 changes: 44 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<maven.checkstyle.plugin.configLocation>
https://raw.githubusercontent.com/mate-academy/style-guides/master/java/checkstyle.xml
</maven.checkstyle.plugin.configLocation>
<springdoc.version>2.5.0</springdoc.version>
</properties>
<dependencies>
<dependency>
Expand All @@ -41,6 +42,49 @@
<groupId>com.h2database</groupId>
<artifactId>h2</artifactId>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-web</artifactId>
</dependency>

<dependency>
<groupId>org.hibernate.orm</groupId>
<artifactId>hibernate-core</artifactId>
<version>${hibernate.version}</version>
</dependency>

<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>${lombok.version}</version>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>com.mysql</groupId>
<artifactId>mysql-connector-j</artifactId>
<version>${mysql.version}</version>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>${jackson-bom.version}</version>
</dependency>

<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
<version>${httpclient5.version}</version>
</dependency>

<dependency>
<groupId>org.springdoc</groupId>
<artifactId>springdoc-openapi-starter-webmvc-ui</artifactId>
<version>${springdoc.version}</version>
</dependency>

</dependencies>

<build>
Expand Down
60 changes: 60 additions & 0 deletions src/main/java/mate/academy/rickandmorty/config/DataFetcher.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package mate.academy.rickandmorty.config;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.annotation.PostConstruct;
import java.util.List;
import lombok.RequiredArgsConstructor;
import mate.academy.rickandmorty.model.Character;
import mate.academy.rickandmorty.repository.CharacterRepository;
import org.springframework.stereotype.Component;
import org.springframework.web.client.RestTemplate;

@Component
@RequiredArgsConstructor
public class DataFetcher {

private static final String CHARACTER_URL = "https://rickandmortyapi.com/api/character";

Choose a reason for hiding this comment

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

I would prefer to make this url configurable via propertioes

private static final String ROOT_INFO = "info";
private static final String ROOT_RESULTS = "results";
private static final String ROOT_NEXT = "next";
private static final String NULL_VALUE = "null";
private final RestTemplate restTemplate;
private final ObjectMapper objectMapper;
private final CharacterRepository characterRepository;

@PostConstruct

Choose a reason for hiding this comment

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

PostConstruct often used for bean configuration, the code that you wrote in not related to bean configuration, so I suggest to use different approach for this, you can use CommandLineRunner interface(or bean) or use event listeners to run code on startup

public void fetchData() throws JsonProcessingException {

Choose a reason for hiding this comment

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

Bad method name, I believe fetch data method should return something

Choose a reason for hiding this comment

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

Please catch JsonProcessingException and rethrow it as runtime

String url = CHARACTER_URL;

while (url != null) {
String jsonResponse = restTemplate.getForObject(url, String.class);
JsonNode rootNode = objectMapper.readTree(jsonResponse);

Choose a reason for hiding this comment

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

What is the reason to readTree? Is it possible to map to an object?

Copy link
Author

Choose a reason for hiding this comment

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

I choose this aproach because its clear for me to create object from which then I'll fetch info by key, and then manipulate with it. Then I get by key="results" and map to Character.class


List<Character> pageCharacters = deserializeCharacters(rootNode);
characterRepository.saveAll(pageCharacters);
url = getNextPageUrl(rootNode);
}
}

private String getNextPageUrl(JsonNode rootNode) {
JsonNode infoNode = rootNode.get(ROOT_INFO);
if (infoNode != null) {
JsonNode nextNode = infoNode.get(ROOT_NEXT);
if (nextNode.asText() != NULL_VALUE) {
return nextNode.asText();
}
}
return null;
}

private List<Character> deserializeCharacters(JsonNode rootNode)
throws JsonProcessingException {
JsonNode resultNode = rootNode.get(ROOT_RESULTS);
return objectMapper.treeToValue(resultNode,
objectMapper.getTypeFactory().constructCollectionType(List.class, Character.class));

}
}

Choose a reason for hiding this comment

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

Suggested change

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package mate.academy.rickandmorty.config;

import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
import org.springframework.web.client.RestTemplate;

@Configuration
public class RestTemplateConfig {

@Bean
public RestTemplate restTemplate() {
HttpComponentsClientHttpRequestFactory httpRequestFactory
= new HttpComponentsClientHttpRequestFactory();
httpRequestFactory.setHttpClient(httpClient());
return new RestTemplate(httpRequestFactory);
}

@Bean
public CloseableHttpClient httpClient() {
PoolingHttpClientConnectionManager connManager = new PoolingHttpClientConnectionManager();
connManager.setMaxTotal(100);
connManager.setDefaultMaxPerRoute(20);
return HttpClients.custom()
.setConnectionManager(connManager)
.build();
}
Comment on lines +11 to +30

Choose a reason for hiding this comment

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

This beans should be created by spring, no need to create them manually, if you want to configure them somehow you can you application properties

Copy link
Author

Choose a reason for hiding this comment

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

In my case I add those, because spring said
"Parameter 0 of constructor in mate.academy.rickandmorty.config.DataFetcher required a bean of type 'org.springframework.web.client.RestTemplate' that could not be found.

Action:

Consider defining a bean of type 'org.springframework.web.client.RestTemplate' in your configuration.
"

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package mate.academy.rickandmorty.controller;

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import java.util.List;
import lombok.RequiredArgsConstructor;
import mate.academy.rickandmorty.model.Character;
import mate.academy.rickandmorty.service.CharacterService;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@Tag(name = "Rick and Morty character management",
description = "Endpoints for managing characters")
@RestController
@RequestMapping("characters")

Choose a reason for hiding this comment

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

Suggested change
@RequestMapping("characters")
@RequestMapping("/characters")

@RequiredArgsConstructor
public class CharacterController {
private final CharacterService characterService;

@Operation(summary = "Get one character",
description = "Get random character from Rick and Morty universe")
@GetMapping
public Character generate() {

Choose a reason for hiding this comment

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

bad method name

return characterService.generateRandomCharacter();
}

@Operation(summary = "Get list of characters by name contains 'row'",
description = "Get list of character from Rick and Morty universe "
+ "where character name like '%:row%'")
@GetMapping("/byNameContains")
public List<Character> findCharacterNameContains(@RequestParam String row) {
return characterService.findCharacterNameContains(row);
}

Choose a reason for hiding this comment

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

Suggested change

}
28 changes: 28 additions & 0 deletions src/main/java/mate/academy/rickandmorty/model/Character.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package mate.academy.rickandmorty.model;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import lombok.Getter;
import lombok.Setter;
import mate.academy.rickandmorty.utils.CharacterDeserializer;

@Entity
@Getter
@Setter
@Table(name = "characters")
@JsonDeserialize(using = CharacterDeserializer.class)
public class Character {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@Column(name = "external_id")
private Long externalId;
private String name;
private String status;
private String gender;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package mate.academy.rickandmorty.repository;

import java.util.List;
import mate.academy.rickandmorty.model.Character;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;

@Repository
public interface CharacterRepository extends JpaRepository<Character, Long> {
@Query("SELECT c FROM Character c WHERE LOWER(c.name) LIKE %:row%")
List<Character> findByNameContainsIgnoreCase(String row);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package mate.academy.rickandmorty.service;

import java.util.List;
import mate.academy.rickandmorty.model.Character;

public interface CharacterService {
Character generateRandomCharacter();

List<Character> findCharacterNameContains(String row);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package mate.academy.rickandmorty.service.impl;

import java.util.List;
import java.util.Optional;
import java.util.Random;
import lombok.RequiredArgsConstructor;
import mate.academy.rickandmorty.model.Character;
import mate.academy.rickandmorty.repository.CharacterRepository;
import mate.academy.rickandmorty.service.CharacterService;
import org.springframework.stereotype.Service;

@Service
@RequiredArgsConstructor
public class CharacterServiceImpl implements CharacterService {
private static final int CHARACTERS_AMOUNT = 826;

Choose a reason for hiding this comment

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

What if some characters will be added or removed?

private final CharacterRepository characterRepository;

@Override
public Character generateRandomCharacter() {
return Optional.of(characterRepository.findById(new Random().nextLong(CHARACTERS_AMOUNT)))

Choose a reason for hiding this comment

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

Do you really want to create Random object each time this method is called

.get()
.get();

Choose a reason for hiding this comment

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

What if optional will be empty?

}

@Override
public List<Character> findCharacterNameContains(String row) {
return characterRepository.findByNameContainsIgnoreCase(row);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package mate.academy.rickandmorty.utils;

import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonNode;
import java.io.IOException;
import mate.academy.rickandmorty.model.Character;

public class CharacterDeserializer extends JsonDeserializer<Character> {

Choose a reason for hiding this comment

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

I think it is redundant, object mapper could deserialize your objects without additional configuration

Copy link
Author

@TonyH277 TonyH277 May 6, 2024

Choose a reason for hiding this comment

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

It does, but it maps id from Json to Entity.id. I thought JsonProperty annotation will solve this problem, but had mapping exception with msg like (I want to set json.id to entity.id, but your annotation said that i should set json.id to other field)

Choose a reason for hiding this comment

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

That is why you need to create an externalDto to map response from API and then remap it to entity and save it, such approach will be more flexible because we have a separate class to represent characters in external API. In general better to avoid custom deserialization logic, because it is often confusing. Flow when we map external response as is and then work with it as we want is preferable, I could describe in more details on QnA

private static final String ID = "id";
private static final String NAME = "name";
private static final String STATUS = "status";
private static final String GENDER = "gender";

@Override
public Character deserialize(JsonParser jsonParser,
DeserializationContext deserializationContext)
throws IOException, JacksonException {
JsonNode treeNode = jsonParser.getCodec().readTree(jsonParser);

String id = treeNode.get(ID).asText();
String name = treeNode.get(NAME).asText();
String status = treeNode.get(STATUS).asText();
String gender = treeNode.get(GENDER).asText();

Character character = new Character();
character.setExternalId(Long.valueOf(id));
character.setName(name);
character.setStatus(status);
character.setGender(gender);

return character;
}
}
9 changes: 8 additions & 1 deletion src/main/resources/application.properties
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@

spring.application.name=rickandmorty
spring.datasource.url=jdbc:mysql://localhost:3306/rickandmorty
spring.datasource.driver-class-name=com.mysql.cj.jdbc.Driver
spring.datasource.username=root
spring.datasource.password=root
spring.jpa.hibernate.ddl-auto=create-drop

Choose a reason for hiding this comment

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

Don`t push createntials

spring.jpa.show-sql=true
spring.jpa.open-in-view=false
Loading