Skip to content

Commit

Permalink
Merge pull request #12 from Kecon/fix_code_smells
Browse files Browse the repository at this point in the history
Fix code smells
  • Loading branch information
Kecon authored Aug 6, 2023
2 parents 71ee20d + c54ef7f commit bfbfd5d
Show file tree
Hide file tree
Showing 13 changed files with 187 additions and 52 deletions.
2 changes: 1 addition & 1 deletion CODE_OF_CONDUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ Violating these terms may lead to a permanent ban.
### 4. Permanent Ban

**Community Impact**: Demonstrating a pattern of violation of community
standards, including sustained inappropriate behavior, harassment of an
standards, including sustained inappropriate behavior, harassment of an
individual, or aggression toward or disparagement of classes of individuals.

**Consequence**: A permanent ban from any sort of public interaction within
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Kalbum

A photo album that you may host your self

[![Java CI](https://github.com/Kecon/Kalbum/actions/workflows/build.yaml/badge.svg?branch=main)](https://github.com/Kecon/Kalbum/actions/workflows/build.yaml)
Expand Down
17 changes: 7 additions & 10 deletions src/main/java/se/kecon/kalbum/Album.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;

import java.util.List;
import java.util.stream.Collectors;

/**
* POJO for an album.
Expand All @@ -31,19 +31,16 @@
*/
@EqualsAndHashCode(callSuper = true)
@Data
public class Album extends AlbumContent implements Cloneable {
@NoArgsConstructor
public class Album extends AlbumContent {

private List<ContentData> contents;

public Album(Album album) {
super(album);

@Override
public Album clone() {
Album clone = (Album) super.clone();

if (this.contents != null) {
clone.setContents(this.contents.stream().map(ContentData::clone).collect(Collectors.toList()));
if (album.getContents() != null) {
this.contents = album.getContents().stream().map(ContentData::new).toList();
}

return clone;
}
}
17 changes: 4 additions & 13 deletions src/main/java/se/kecon/kalbum/AlbumContent.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/
package se.kecon.kalbum;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;

Expand All @@ -28,23 +27,15 @@
* @since 2023-07-31
*/
@Data
@AllArgsConstructor
@NoArgsConstructor
public class AlbumContent implements Cloneable {
public class AlbumContent {

private String id;

private String name;

@Override
public AlbumContent clone() {
try {
AlbumContent clone = (AlbumContent) super.clone();
clone.setId(this.id);
clone.setName(this.name);
return clone;
} catch (CloneNotSupportedException e) {
throw new AssertionError();
}
public AlbumContent(AlbumContent albumContent) {
this.id = albumContent.getId();
this.name = albumContent.getName();
}
}
2 changes: 1 addition & 1 deletion src/main/java/se/kecon/kalbum/AlbumController.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ protected Path getContentPath(final String id, final String filename) throws Ill
* @return list of albums
*/
@GetMapping(path = "/albums/")
public ResponseEntity<List<AlbumContent>> listAlbums() throws IllegalAlbumIdException, IOException {
public ResponseEntity<List<AlbumContent>> listAlbums() {
log.info("List albums");

final List<AlbumContent> albumContents = new ArrayList<>();
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/se/kecon/kalbum/AlbumDaoComponent.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public Album create(String name) throws IOException {

int tries = 0;
while (true) {
String id = null;
String id;
do {
id = idGenerator.get();

Expand All @@ -89,7 +89,7 @@ public Album create(String name) throws IOException {

try {
saveToPersistentStorage(album);
return album.clone();
return album;
} catch (IllegalAlbumIdException e) {
log.error("Invalid album id was generated in create: " + id, e);
}
Expand All @@ -108,7 +108,7 @@ public Optional<Album> get(final String id) throws IllegalAlbumIdException {
checkValidAlbumId(id);
final Album album = this.albums.get(id);

return album != null ? Optional.of(album.clone()) : Optional.empty();
return album != null ? Optional.of(new Album(album)) : Optional.empty();
}

/**
Expand All @@ -118,7 +118,7 @@ public Optional<Album> get(final String id) throws IllegalAlbumIdException {
*/
@Override
public List<Album> getAll() {
return this.albums.values().stream().map(Album::clone).collect(Collectors.toList());
return this.albums.values().stream().map(Album::new).collect(Collectors.toList());
}

/**
Expand Down Expand Up @@ -195,7 +195,7 @@ protected void saveToPersistentStorage(final Album album) throws IOException, Il
final ObjectMapper objectMapper = getObjectMapper();
Files.write(path, objectMapper.writeValueAsBytes(album));

this.albums.put(album.getId(), album.clone());
this.albums.put(album.getId(), new Album(album));
} finally {
this.lock.unlock();
}
Expand All @@ -219,7 +219,7 @@ protected Optional<Album> getFromPersistentStorage(String id) throws IllegalAlbu

this.albums.put(id, album);

return Optional.of(album.clone());
return Optional.of(new Album(album));
}
log.warn("Album {} does not exist", id);
return Optional.empty();
Expand Down
27 changes: 11 additions & 16 deletions src/main/java/se/kecon/kalbum/ContentData.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package se.kecon.kalbum;

import lombok.Data;
import lombok.NoArgsConstructor;

import java.time.Instant;

Expand All @@ -28,7 +29,8 @@
* @since 2023-07-31
*/
@Data
public class ContentData implements Cloneable {
@NoArgsConstructor
public class ContentData {

private String contentType;

Expand All @@ -44,20 +46,13 @@ public class ContentData implements Cloneable {

private Instant timestamp;

@Override
public ContentData clone() {
try {
final ContentData clone = (ContentData) super.clone();
clone.setContentType(this.contentType);
clone.setSrc(this.src);
clone.setAlt(this.alt);
clone.setText(this.text);
clone.setWidth(this.width);
clone.setHeight(this.height);
clone.setTimestamp(this.timestamp);
return clone;
} catch (CloneNotSupportedException e) {
throw new AssertionError();
}
public ContentData(ContentData contentData) {
this.contentType = contentData.getContentType();
this.src = contentData.getSrc();
this.alt = contentData.getAlt();
this.text = contentData.getText();
this.width = contentData.getWidth();
this.height = contentData.getHeight();
this.timestamp = contentData.getTimestamp();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
* Exception for unsupported content format.
*
* @author Kenny Colliander
* @since 2023-08-05
* @see se.kecon.kalbum.ContentFormat
* @since 2023-08-05
*/
public class UnsupportedContentFormatException extends Exception {

Expand Down
41 changes: 41 additions & 0 deletions src/test/java/se/kecon/kalbum/AlbumContentTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* Kalbum
* <p>
* Copyright 2023 Kenny Colliander
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package se.kecon.kalbum;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

/**
* Test {@link AlbumContent}
* <p>
* Created 2023-08-06 by Kenny Colliander
*/
class AlbumContentTest {

@Test
void testCopyConstructor() {
AlbumContent albumContent = new AlbumContent();
albumContent.setId("id");
albumContent.setName("name");

AlbumContent albumContentCopy = new AlbumContent(albumContent);
assertEquals(albumContent.getId(), albumContentCopy.getId());
assertEquals(albumContent.getName(), albumContentCopy.getName());
}
}
6 changes: 3 additions & 3 deletions src/test/java/se/kecon/kalbum/AlbumDaoComponentTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ void getNonExisting() throws IOException, IllegalAlbumIdException {
}

@Test
void getInvalidId() throws IOException, IllegalAlbumIdException {
void getInvalidId() throws IOException {
albumDao.setIdGenerator(() -> "id1");
Album album = albumDao.create("name1");

Expand Down Expand Up @@ -231,7 +231,7 @@ void update() throws IOException, IllegalAlbumIdException {
}

@Test
void updateNonExisting() throws IOException, IllegalAlbumIdException {
void updateNonExisting() throws IOException {
albumDao.setIdGenerator(() -> "id1");
Album album = albumDao.create("name1");

Expand All @@ -242,7 +242,7 @@ void updateNonExisting() throws IOException, IllegalAlbumIdException {
}

@Test
void updateInvalidId() throws IOException, IllegalAlbumIdException {
void updateInvalidId() throws IOException {
albumDao.setIdGenerator(() -> "id1");
Album album = albumDao.create("name1");

Expand Down
57 changes: 57 additions & 0 deletions src/test/java/se/kecon/kalbum/AlbumTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* Kalbum
* <p>
* Copyright 2023 Kenny Colliander
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package se.kecon.kalbum;

import org.junit.jupiter.api.Test;

import java.time.Instant;
import java.util.Collections;

import static org.junit.jupiter.api.Assertions.*;

/**
* Test {@link Album}
* <p>
* Created 2023-08-06 by Kenny Colliander
*/
class AlbumTest {

@Test
void testCopyConstructor() {
ContentData contentData = new ContentData();
contentData.setContentType("contentType");
contentData.setSrc("src");
contentData.setAlt("alt");
contentData.setText("text");
contentData.setWidth(1);
contentData.setHeight(2);
contentData.setTimestamp(Instant.now());


Album album = new Album();
album.setId("id");
album.setName("name");
album.setContents(Collections.singletonList(contentData));
Album albumCopy = new Album(album);
assertEquals(album.getId(), albumCopy.getId());
assertEquals(album.getName(), albumCopy.getName());
assertEquals(album.getContents(), albumCopy.getContents());
assertNotSame(album.getContents(), albumCopy.getContents());
assertEquals(album.getContents().get(0), albumCopy.getContents().get(0));
}
}
53 changes: 53 additions & 0 deletions src/test/java/se/kecon/kalbum/ContentDataTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Kalbum
* <p>
* Copyright 2023 Kenny Colliander
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package se.kecon.kalbum;

import org.junit.jupiter.api.Test;

import java.time.Instant;

import static org.junit.jupiter.api.Assertions.*;

/**
* Test {@link ContentData}
* <p>
* Created 2023-08-06 by Kenny Colliander
*/
class ContentDataTest {

@Test
void testCopyConstructor() {
ContentData contentData = new ContentData();
contentData.setContentType("contentType");
contentData.setSrc("src");
contentData.setAlt("alt");
contentData.setText("text");
contentData.setWidth(1);
contentData.setHeight(2);
contentData.setTimestamp(Instant.now());

ContentData contentDataCopy = new ContentData(contentData);
assertEquals(contentData.getContentType(), contentDataCopy.getContentType());
assertEquals(contentData.getSrc(), contentDataCopy.getSrc());
assertEquals(contentData.getAlt(), contentDataCopy.getAlt());
assertEquals(contentData.getText(), contentDataCopy.getText());
assertEquals(contentData.getWidth(), contentDataCopy.getWidth());
assertEquals(contentData.getHeight(), contentDataCopy.getHeight());
assertEquals(contentData.getTimestamp(), contentDataCopy.getTimestamp());
}
}
Loading

0 comments on commit bfbfd5d

Please sign in to comment.