0
\$\begingroup\$

I have recently been exploring React and Spring Boot, and I would greatly appreciate your feedback on a practice application that I have developed. The application consists of a Spring Boot backend utilizing an H2 database, along with a ReactJS frontend implemented in TypeScript. These layers communicate with each other through a REST API.

The API represents a basic account system, encompassing fields such as username, password, and gender. It also has some backend validation to ensure data integrity. There is also a unique identicon generated for each user.

The identicon generation process in the application involves two steps. First, the Jdenticon library is used to generate an SVG file representing the unique identicon based on a given username. This SVG file is then converted to a PNG image using Apache Batik's PNGTranscoder, resulting in a byte array representation of the image. The backend exposes this byte array as the response when the frontend requests the identicon, and the frontend consumes it as a blob to display the identicon image using an img element with a blob URL.

Please note that the code is quite long due to there being 2 separate codebases.

Here is the REST controller:

@RestController
@CrossOrigin
@RequestMapping("/accounts")
public class AccountController {

    private final AccountService accountService;

    @Autowired
    public AccountController(AccountService accountService) {
        this.accountService = accountService;
    }

    @GetMapping
    public ResponseEntity<List<Account>> getAccounts() {
        try {
            List<Account> accounts = accountService.getAccounts();

            return ResponseEntity.ok(accounts);
        } catch (Exception ex) {
            return ResponseEntity.internalServerError().build();
        }
    }

    @GetMapping("/identicon/{username}")
    public ResponseEntity<byte[]> getIdenticon(@PathVariable String username) {
        try {
            byte[] identiconOutput = accountService.getIdenticon(username);

            return ResponseEntity.ok(identiconOutput);
        } catch (Exception ex) {
            return ResponseEntity.internalServerError().build();
        }
    }

    @PostMapping
    public ResponseEntity<?> createAccount(@RequestBody Account account) {
        try {
            accountService.registerAccount(account);
            return ResponseEntity.ok().build();
        } catch (AccountValidatorException ex) {
            return ResponseEntity.badRequest().build();
        } catch (Exception ex) {
            return ResponseEntity.internalServerError().build();
        }
    }

    @DeleteMapping("/{username}")
    public ResponseEntity<?> deleteAccount(@PathVariable String username) {
        try {
            accountService.deleteAccount(username);

            return ResponseEntity.ok().build();
        } catch (Exception ex) {
            return ResponseEntity.internalServerError().build();
        }
    }
}

Here is the account service:

@Service
public class AccountService {

    private final AccountRepository accountRepository;
    private final List<AccountValidatorService> accountValidatorServices;

    @Autowired
    public AccountService(AccountRepository accountRepository, List<AccountValidatorService> accountValidatorServices) {
        this.accountRepository = accountRepository;
        this.accountValidatorServices = accountValidatorServices;
    }

    public List<Account> getAccounts() {
        return accountRepository.getAccounts();
    }

    public void registerAccount(Account account) throws AccountValidatorException {
        List<String> messages = new ArrayList<>();

        for (AccountValidatorService accountValidatorService : accountValidatorServices) {
            if (!accountValidatorService.validate(account)) {
                messages.add(accountValidatorService.getErrorMessage());
            }
        }

        if (!messages.isEmpty()) {
            throw new AccountValidatorException("Error trying to register account: " + messages);
        }

        accountRepository.addAccount(account.getUsername(), account.getPassword(), account.getGender());
    }

    public byte[] getIdenticon(String username) throws TranscoderException, IOException {
        // get the original SVG XML file which will be converted to a PNG with Apache Batik
        String svg = Jdenticon.toSvg(username);

        TranscoderInput input = new TranscoderInput(new StringReader(svg));

        ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
        TranscoderOutput output = new TranscoderOutput(byteStream);

        PNGTranscoder transcoder = new PNGTranscoder();
        transcoder.transcode(input, output);

        byteStream.flush();
        byteStream.close();

        return byteStream.toByteArray();
    }

    public void deleteAccount(String username) {
        accountRepository.deleteAccountByUsername(username);
    }
}

Here is the account repository:

public interface AccountRepository extends CrudRepository<Account, Long> {

    @Query("SELECT * FROM account")
    List<Account> getAccounts();

    @Modifying
    @Query("DELETE FROM account WHERE username = :username")
    void deleteAccountByUsername(String username);

    @Modifying
    @Query("INSERT INTO account VALUES(NULL, :username, :password, :gender)")
    void addAccount(String username, String password, Gender gender);
}

Here are the account validators:

public interface AccountValidatorService {

    String getErrorMessage();

    boolean validate(Account account);
}

@Service
public class PasswordValidatorService implements AccountValidatorService {

    @Override
    public String getErrorMessage() {
        return "Password must contain at least 3 digits, 3 letters, and 3 special characters";
    }

    @Override
    public boolean validate(Account account) {
        String password = account.getPassword();

        long digitCount = password
            .chars()
            .filter(Character::isDigit)
            .count();

        long letterCount = password
            .chars()
            .filter(Character::isLetter)
            .count();

        long specialCharacterCount = 0;
        Set<Character> specialCharacters = Set.of('!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '+', '-');

        for (Character c : password.toCharArray()) {
            if (specialCharacters.contains(c)) {
                ++specialCharacterCount;
            }
        }

        return digitCount >= 3 && letterCount >= 3 && specialCharacterCount >= 3;
    }
}

@Service
public class UniqueUsernameValidatorService implements AccountValidatorService {

    private final AccountRepository accountRepository;

    @Autowired
    public UniqueUsernameValidatorService(AccountRepository accountRepository) {
        this.accountRepository = accountRepository;
    }

    @Override
    public String getErrorMessage() {
        return "An account with this username already exists";
    }

    public boolean validate(String username) {
        return accountRepository
            .getAccounts()
            .stream()
            .noneMatch(_account -> _account.getUsername().equals(username));
    }

    @Override
    public boolean validate(Account account) {
        return validate(account.getUsername());
    }
}

@Service
public class UsernameLengthValidatorService implements AccountValidatorService {

    @Override
    public String getErrorMessage() {
        return "Username must have a length larger than 3 and smaller than or equal to 50";
    }

    @Override
    public boolean validate(Account account) {
        String username = account.getUsername();

        return username.length() >= 3 && username.length() <= 50;
    }
}

React frontend:

import { Button, Flex, Input, Select, Text } from '@chakra-ui/react';
import React, { useEffect, useState } from 'react';

type Gender = 'MALE' | 'FEMALE'

interface Account {
    username: string
    password: string
    gender: Gender
}

class API {
    private constructor() { }

    static readonly endpoint = 'http://localhost:8080/accounts'

    static async createAccount(account: Account): Promise<void> {
        await fetch(this.endpoint, {
            method: 'POST',
            headers: { 'Content-Type': 'application/json' },
            body: JSON.stringify(account)
        })
    }

    static async getIdenticon(username: string): Promise<Blob> {
        const result = await fetch(`${this.endpoint}/identicon/${username}`)

        const blob = result.blob()

        return blob
    }

    static async getAccounts(): Promise<Account[]> {
        const result = await fetch(this.endpoint)
        const json: Promise<Account[]> = await result.json()
        
        return json
    }

    static async deleteAccount(account: Account): Promise<void> {
        await fetch(`${this.endpoint}/${account.username}`, {
            method: 'DELETE',
            headers: { 'Content-Type': 'application/json' },
            body: JSON.stringify(account)
        })
    }
} 

type AccountDeleteAction = (account: Account) => void

interface AccountListItemProps {
    account: Account
    onDeleteClick: AccountDeleteAction
}

function AccountListItem(props: AccountListItemProps): JSX.Element {
    const [imageUrl, setImageUrl] = useState<string | null>(null)

    async function getIdenticon(): Promise<void> {
        const response = await API.getIdenticon(props.account.username)

        const blobUrl = URL.createObjectURL(response)

        setImageUrl(blobUrl)
    }

    if (imageUrl === null) {
        getIdenticon()
    }

    return (
        <Flex
            shadow='md' 
            borderRadius='md'
            width='100%'
            alignItems='center'
            paddingRight='16px'
        >
            <Flex flexDirection='row' width='100%' padding='16px' height='100px'>
                {imageUrl && <img src={imageUrl} />}
                <Flex flexDirection='column' justifyContent='center' marginLeft='16px'>
                    <h1><Text as='span' fontWeight='bold'>Username:</Text> {props.account.username}</h1>
                    <h2><Text as='span' fontWeight='bold'>Password:</Text> {props.account.password}</h2>
                    <h3><Text as='span' fontWeight='bold'>Gender:</Text> {props.account.gender}</h3>
                </Flex>
            </Flex>

            <Flex marginLeft='auto'>
                <Button colorScheme='red' onClick={() => props.onDeleteClick(props.account)}>Delete</Button>
            </Flex>
        </Flex>
    )
}

interface AccountListProps {
    accounts: Account[]
    onDeleteClick: AccountDeleteAction
}

function AccountList(props: AccountListProps): JSX.Element {
    return (
        <Flex flexDirection='column' width='100%'>
            {props.accounts.map((account, i) => 
                <AccountListItem 
                    key={i}
                    account={account} 
                    onDeleteClick={(account) => props.onDeleteClick(account)} 
                />
            )}
        </Flex>
    )
}

function App(): JSX.Element {
    const [accounts, setAccounts] = useState<Account[]>([])

    const [username, setUsername] = useState<string | null>(null)
    const [password, setPassword] = useState<string | null>(null)
    const [gender, setGender] = useState<Gender | null>(null)

    const [isLoading, setIsLoading] = useState(true)

    async function fetchAccounts(): Promise<void> {
        const data = await API.getAccounts()


        setAccounts(data)
        setIsLoading(false)
    }

    async function formSubmitHandler(e: React.FormEvent<HTMLFormElement>): Promise<void> {
        e.preventDefault()

        if (username === null || password === null || gender === null) {
            return
        }

        const account: Account = { 
            username: username,
            password: password,
            gender: gender
        }

        setIsLoading(true)

        await API.createAccount(account)

        setUsername(null)
        setPassword(null)
        setGender(null)

        await fetchAccounts()

        setIsLoading(false)
    }

    useEffect(() => {
        fetchAccounts()
    }, [])

    return (
        <Flex padding='16px' height='100vhw' flexDirection='column'>
            <Text fontSize='32px' marginBottom='8px'>Create Account</Text>

            <Flex 
                flexDirection='column' 
                padding='16px' 
                shadow='md' 
                borderRadius='md' 
            >
                <form onSubmit={formSubmitHandler}>
                    <Flex flexDirection='column' gap='8px'>
                        <Input placeholder='Username' value={username ?? ''} onChange={(e) => setUsername(e.target.value)} />
                        <Input placeholder='Password' type='password' value={password ?? ''} onChange={(e) => setPassword(e.target.value)} />

                        <Select
                            value={gender === null ? 0 : gender}
                            placeholder='Select Gender'
                            onChange={(e) => setGender(e.target.value as Gender)}
                        >
                            <option value='MALE'>Male</option>
                            <option value='FEMALE'>Female</option>
                        </Select>

                        <Button type='submit' alignSelf='flex-start'>Add Account</Button>
                    </Flex>
                </form>
            </Flex>

            {isLoading && <Flex width='100%' flexGrow={1} alignItems='center' justifyContent='center'><h1>Loading...</h1></Flex>}

            <AccountList onDeleteClick={async (account) => {
                await API.deleteAccount(account)

                fetchAccounts()
            }} accounts={accounts} />
        </Flex>
    )
}

export default App;

Here is the result:

enter image description here

I understand all of the security implications as well, so I want to reiterate that this is just a practice sort of thing :)

I would appreciate some feedback in regards to:

  1. Overall Design and Architecture: I would like to know your thoughts on the overall design and architecture of the application. Are there any design patterns or architectural principles that could be improved or followed more effectively?

  2. Code Organization and Structure: Please review the organization and structure of the codebase. Are there any areas where the code could be better organized or modularized? Are there any naming conventions that could be improved?

  3. Backend Implementation: I would like feedback on the backend implementation using Spring Boot. Are there any areas where the code could be optimized or simplified? Are there any potential performance or scalability concerns?

  4. Frontend Implementation: Please provide feedback on the React frontend implemented in TypeScript. Are there any best practices or optimizations that could be applied to improve the frontend code? How is the user interface and overall user experience?

  5. REST API Design: Review the design of the REST API endpoints and their functionality. Are there any improvements that can be made to enhance the usability and efficiency of the API? Are there any security concerns that need to be addressed?

  6. Data Validation and Error Handling: Evaluate the data validation and error handling mechanisms implemented in the backend. Are there any potential vulnerabilities or edge cases that need to be handled more effectively?

Regards.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Just a quick comment: the mapping should be "/{username}/identicon" for it to follow REST best practises. \$\endgroup\$ Commented May 29, 2023 at 7:56
  • \$\begingroup\$ @TorbenPutkonen OK \$\endgroup\$ Commented May 29, 2023 at 10:05

1 Answer 1

1
\$\begingroup\$

Overall Design and Architecture

  1. H2-Database might give bad performance, you might concider using a more performance efficient solution for heavy workloads. Postgres or MySQL might be better in performant (and they support ODBC connections directly to support interdisciplinary-developer to choose to use a different programming language). But: It "might" give bad performance, this means also that it might be enougth.
  2. The username might be a private-person-bound-information, some governments (like the german DSGVO) might regulate the username in the logs. (The right of be forgotten forces you to post-process all existing and backuped logfiles and wipe the username of the person who like to be forgotten).
  3. In order to have a efficient development-cycle you might concider to use PMD, spotbugs/findbugs, Checkstyle, eslint (etc. ...). It is obvious that the sourcecode missing Documentation, final-keywords etc. but this depends strongly on the quality-principles of the company and project.
  4. In order to track KPIs of the restful-backend-service you might like to instrument a Micrometer or InfluxDB/Grafana or something.

Code Organization and Structure

  1. The Java language looks like Jdk8, concider updating to a new version and use their benefits. In example, in the code is this line: String username = account.getUsername();, you could use var username = account.getUsername(); instead. If someday a class is introduced that encapsules Usernames in general, you have fewer refactoring effort.
  2. Please implement some kind of CSRF-Protection.
  3. If you use a CVS like git or svn you might like to review code. It might be my personal prefer. In TS in createAccount I love to have a ending comma like body: JSON.stringify(account), instead of body: JSON.stringify(account) end the end of the line because if you add an additional header you have one incomming line instaed a additional modification of the line before to add a ,-Sign.

Backend Implementation

I will not repeat what PMD, Spotbugs, Checkstyle will tell you. I will comment other things:

  1. You could use @Named instead of @Service and @Inject instead of @Autowired because if you move to a different CDI(Context-Dependency-Injection-System) its less effort to rewrite.

  2. In registerAccount(Account account) you rely on the interface List<String> in order to comply the Coding to interfaces, not implementation, contratiolations, that is a good thing! Concider use a more abstract Interface than List<String>. You could use Collection<String> here.

  3. The rest-path /accounts/identicon/{username} should be /accounts/{username}/identicon because of the hierarchial/tree build of the path. You could also concider to use only plurals in rest or singulars. I prefer account instead of accounts, can not tell exactly why I prefer the singular.

  4. UsernameLengthValidatorService allowes usernames having 3 characters. But the message is indicating that a username having 3 characters is not allowed. I am confused are they allowed or not?

  5. You are using Constructor-Parameter injection, but I do not know why!

  6. Your database is the datasource. But how sourcy are those data? getIdenticon is storing a binary image in the database, the binary image is the result of a calculation, the caluclation from the svg to the image-bytes. Why dont you store the SVG in the database? A eTag-mechanism might solve your transfer overhead.

  7. @CrossOrigin without allowedOrigins means every domain can access, even if the browser is compromited throu scripts that execute requests by their domain in your browser-app. You might concider reducing the allowed origins to the domain of your website only.

  8. If you generate the image, you could transform directly to the servlet-output-stream, this will reduce the memory requirements.

  9. createAccount swallow-handle the AccountValidatorException. The constructed useful message from AccountValidatorException is neither logged to the console nor transported to the browser.

  10. To have a password that misses 3 special characters is not - by default - a common requirement. Therefore violation against the 3-special-character rule is not a Exception. Please report violations throu a non-throwable-object. Think that some application-servers will transform all exception to the http status-code 500 - Internal Server Error. But the server had no error. Pleas do not use exceptions this way.

Frontend Implementation

  1. The account-server is not always https, not always localhost :D
  2. Please use CSRF-Protection.
  3. If you return a Mobx-Element you do not need round bracets around the return-value;
  4. Use URL.createObjectURL might work wonderful but if you introduce CSP you might like to block object-urls. Why dont you use the genuine url of the image?
  5. Jsdoc is your friend.

REST API Design

Some points mentioned before will not repeated here. Recheck the backend-issues.

  1. In rest there are some frameworks that support you offer an represenational (R in rest) UI like Swagger,Swagger2,Springfox. They do document almost themselve.
  2. I miss some health check, but it is not very important.
  3. Rendering the user-image is more work intensive than register/unregister, please concider to split this service into two microservices in order to balance them according to two-times of the expected live workload. One microservice for heavy image processing and one microservice for business-critical basic accounting.

Data Validation and Error Handling

Data validation should be done as soon as possible, if you are using a Validator you could avoid calling registerAccount with bad-objects.

Error handling (without Exception-Handling) is ok (not good) but I see no strong improvement potential.

After all (except the CSRF and CORS issue) I see absolute no dealbreaker!

Good work.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks for the feedback. I found a couple of the stuff you said to be a bit confusing, such as: - JDK version -- the JDK version of this code is 20, not 8. I'm curious as to what features I'm not utilizing that would make you think I'm using JDK 8? - H2 database -- I am of the belief that H2 is actually performant, could you recommend a more performant database if this is not the case? - You could use @ Named instead of @ Service and @ Inject instead of @ Autowired - ?? not sure what you mean here, I'm not using @ Inject in the code \$\endgroup\$ Commented Jul 14, 2023 at 0:44
  • \$\begingroup\$ - You are using Constructor-Parameter injection, but I do not know why! - because it's best practise? \$\endgroup\$ Commented Jul 14, 2023 at 0:48
  • \$\begingroup\$ @thebluepandabear Can you give reference for the best practice thing? I am not sure how prototype-scoped beans can be injected using constructor-parameter injection. \$\endgroup\$
    – Grim
    Commented Jul 14, 2023 at 9:17
  • \$\begingroup\$ @thebluepandabear The @Inject thing, yes you are not using it. But you could if you like to be compatible to other CDI technologies. By the way, I edited my answer to go deeper into your suggestions from the comments. \$\endgroup\$
    – Grim
    Commented Jul 14, 2023 at 9:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.