Skip to content

Commit f503faf

Browse files
authored
Merge pull request #250 from Olog/fix_tags_endpoint
Fixes for endpoints using a PathVariable, plus some code beautifying
2 parents 8ee75d6 + 083fd6c commit f503faf

File tree

7 files changed

+204
-54
lines changed

7 files changed

+204
-54
lines changed

src/main/java/org/phoebus/olog/LevelsResource.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public Iterable<org.phoebus.olog.entity.Level> findAll() {
5858
*/
5959
@SuppressWarnings("unused")
6060
@GetMapping("/{levelName}")
61-
public org.phoebus.olog.entity.Level findById(@PathVariable(name = "levelName") String levelName) {
61+
public org.phoebus.olog.entity.Level findByName(@PathVariable(name = "levelName") String levelName) {
6262
Optional<org.phoebus.olog.entity.Level> foundLevel = levelRepository.findById(levelName);
6363
if (foundLevel.isPresent()) {
6464
return foundLevel.get();

src/main/java/org/phoebus/olog/LogResource.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public class LogResource {
133133

134134
@GetMapping("{logId}")
135135
@SuppressWarnings("unused")
136-
public Log getLog(@PathVariable(name = "logId") String logId) {
136+
public Log getLogById(@PathVariable(name = "logId") String logId) {
137137
Optional<Log> foundLog = logRepository.findById(logId);
138138
if (foundLog.isPresent()) {
139139
return foundLog.get();

src/main/java/org/phoebus/olog/LogTemplateResource.java

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class LogTemplateResource {
5858

5959
@GetMapping("{logTemplateId}")
6060
@SuppressWarnings("unused")
61-
public LogTemplate getLogTemplate(@PathVariable(name = "logTemplateId") String logTemplateId) {
61+
public LogTemplate getLogTemplateById(@PathVariable(name = "logTemplateId") String logTemplateId) {
6262
return logTemplateRepository.findById(logTemplateId).get();
6363
}
6464

@@ -116,54 +116,12 @@ public LogTemplate createLogTemplate(@RequestBody LogTemplate logTemplate,
116116
return newLogTemplate;
117117
}
118118

119-
/**
120-
* Updates existing {@link LogTemplate}.
121-
*
122-
* @param logTemplateId The id of the template subject to update. It must exist, i.e. it is not created of not found.
123-
* @param markup Markup strategy, if any.
124-
* @param logTemplate The {@link LogTemplate} as sent by client.
125-
* @param principal The authenticated {@link Principal} of the request.
126-
* @return The updated {@link LogTemplate} record, or HTTP status 404 if the log template does not exist. If the path
127-
* variable does not match the id in the log record, HTTP status 400 (bad request) is returned.
128-
*/
129-
/*
130-
@SuppressWarnings("unused")
131-
@PostMapping("/{logTemplateId}")
132-
public LogTemplate updateLogTemplate(@PathVariable String logTemplateId,
133-
@RequestParam(value = "markup", required = false) String markup,
134-
@RequestBody LogTemplate logTemplate,
135-
@AuthenticationPrincipal Principal principal) {
136-
137-
// In case a client sends a log template record where the id does not match the path variable, return HTTP 400 (bad request)
138-
if (!logTemplateId.equals(logTemplate.getId())) {
139-
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, TextUtil.LOG_TEMPLATE_NOT_MATCH_PATH);
140-
}
141-
142-
Optional<LogTemplate> foundLogTemplate = logTemplateRepository.findById(logTemplateId);
143-
144-
LogTemplate persistedLogTemplate = foundLogTemplate.get();
145-
persistedLogTemplate.setName(logTemplate.getName());
146-
persistedLogTemplate.getLog().setOwner(principal.getName());
147-
persistedLogTemplate.getLog().setLevel(logTemplate.getLog().getLevel());
148-
persistedLogTemplate.getLog().setProperties(logTemplate.getLog().getProperties());
149-
persistedLogTemplate.getLog().setModifyDate(Instant.now());
150-
persistedLogTemplate.getLog().setDescription(logTemplate.getLog().getDescription()); // to make it work with old clients where description field is sent instead of source
151-
persistedLogTemplate.getLog().setTags(logTemplate.getLog().getTags());
152-
persistedLogTemplate.getLog().setLogbooks(logTemplate.getLog().getLogbooks());
153-
persistedLogTemplate.getLog().setTitle(logTemplate.getLog().getTitle());
154-
155-
return logTemplateRepository.update(persistedLogTemplate);
156-
157-
}
158-
159-
*/
160-
161119
/**
162120
* Delete a {@link LogTemplate} based on its unique id.
163121
* @param logTemplateId Unique id
164122
*/
165123
@DeleteMapping("/{logTemplateId}")
166-
public void deleteLogTemplate(@PathVariable String logTemplateId){
124+
public void deleteLogTemplate(@PathVariable(name = "logTemplateId") String logTemplateId){
167125
logTemplateRepository.deleteById(logTemplateId);
168126
}
169127

src/main/java/org/phoebus/olog/LogbooksResource.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
@RequestMapping(LOGBOOK_RESOURCE_URI)
3838
public class LogbooksResource {
3939

40-
private Logger log = Logger.getLogger(LogbooksResource.class.getName());
40+
private final Logger log = Logger.getLogger(LogbooksResource.class.getName());
4141

4242
@Autowired
4343
private LogbookRepository logbookRepository;
@@ -55,7 +55,7 @@ public Iterable<Logbook> findAll() {
5555

5656
@SuppressWarnings("unused")
5757
@GetMapping("/{logbookName}")
58-
public Logbook findByTitle(@PathVariable(name = "logbookName") String logbookName) {
58+
public Logbook findByName(@PathVariable(name = "logbookName") String logbookName) {
5959
Optional<Logbook> foundLogbook = logbookRepository.findById(logbookName);
6060
if (foundLogbook.isPresent()) {
6161
return foundLogbook.get();

src/main/java/org/phoebus/olog/PropertiesResource.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
@RequestMapping(PROPERTY_RESOURCE_URI)
3838
public class PropertiesResource {
3939

40-
private Logger log = Logger.getLogger(PropertiesResource.class.getName());
40+
private final Logger log = Logger.getLogger(PropertiesResource.class.getName());
4141

4242
@Autowired
4343
private PropertyRepository propertyRepository;
@@ -60,7 +60,7 @@ public Iterable<Property> findAll(@RequestParam(required=false, name = "inactive
6060
}
6161

6262
@GetMapping("/{propertyName}")
63-
public Property findByTitle(@PathVariable(name = "propertyName") String propertyName) {
63+
public Property findByName(@PathVariable(name = "propertyName") String propertyName) {
6464
Optional<Property> foundProperty = propertyRepository.findById(propertyName);
6565
if (foundProperty.isPresent()) {
6666
return foundProperty.get();

src/main/java/org/phoebus/olog/TagsResource.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
@RequestMapping(TAG_RESOURCE_URI)
3535
public class TagsResource {
3636

37-
private Logger log = Logger.getLogger(TagsResource.class.getName());
37+
private final Logger log = Logger.getLogger(TagsResource.class.getName());
3838

3939
@Autowired
4040
private TagRepository tagRepository;
@@ -60,7 +60,7 @@ public Iterable<Tag> findAll() {
6060
* @return the matching tag, or null
6161
*/
6262
@GetMapping("/{tagName}")
63-
public Tag findByTitle(@PathVariable String tagName) {
63+
public Tag findByName(@PathVariable(name = "tagName") String tagName) {
6464
Optional<Tag> foundTag = tagRepository.findById(tagName);
6565
if (foundTag.isPresent()) {
6666
return foundTag.get();
@@ -79,7 +79,7 @@ public Tag findByTitle(@PathVariable String tagName) {
7979
* @return the created tag
8080
*/
8181
@PutMapping("/{tagName}")
82-
public Tag createTag(@PathVariable String tagName, @RequestBody final Tag tag) {
82+
public Tag createTag(@PathVariable(name = "tagName") String tagName, @RequestBody final Tag tag) {
8383
// TODO Check permissions
8484
// Validate
8585

@@ -124,7 +124,7 @@ public Iterable<Tag> updateTag(@RequestBody final List<Tag> tags) {
124124
}
125125

126126
@DeleteMapping("/{tagName}")
127-
public void deleteTag(@PathVariable String tagName) {
127+
public void deleteTag(@PathVariable(name = "tagName") String tagName) {
128128
// TODO Check permissions
129129

130130
// check if present
Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/*
2+
* Copyright (C) 2020 European Spallation Source ERIC.
3+
*
4+
* This program is free software; you can redistribute it and/or
5+
* modify it under the terms of the GNU General Public License
6+
* as published by the Free Software Foundation; either version 2
7+
* of the License, or (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program; if not, write to the Free Software
16+
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
17+
*/
18+
package org.phoebus.olog;
19+
20+
import com.fasterxml.jackson.core.type.TypeReference;
21+
import org.junit.jupiter.api.BeforeAll;
22+
import org.junit.jupiter.api.Test;
23+
import org.junit.jupiter.api.extension.ExtendWith;
24+
import org.phoebus.olog.entity.Logbook;
25+
import org.phoebus.olog.entity.State;
26+
import org.phoebus.olog.entity.Tag;
27+
import org.springframework.beans.factory.annotation.Autowired;
28+
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
29+
import org.springframework.http.HttpHeaders;
30+
import org.springframework.test.context.ActiveProfiles;
31+
import org.springframework.test.context.ContextConfiguration;
32+
import org.springframework.test.context.ContextHierarchy;
33+
import org.springframework.test.context.TestPropertySource;
34+
import org.springframework.test.context.junit.jupiter.SpringExtension;
35+
import org.springframework.test.web.servlet.MvcResult;
36+
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
37+
38+
import java.util.ArrayList;
39+
import java.util.Arrays;
40+
import java.util.List;
41+
import java.util.Optional;
42+
43+
import static org.junit.jupiter.api.Assertions.assertEquals;
44+
import static org.junit.jupiter.api.Assertions.assertFalse;
45+
import static org.mockito.Mockito.reset;
46+
import static org.mockito.Mockito.times;
47+
import static org.mockito.Mockito.verify;
48+
import static org.mockito.Mockito.when;
49+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
50+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
51+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
52+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
53+
54+
/**
55+
* Tests {@link Logbook} resource endpoints. The authentication scheme used is the
56+
* hard coded user/userPass credentials. The {@link LogbookRepository} is mocked.
57+
*/
58+
59+
@ExtendWith(SpringExtension.class)
60+
@ContextHierarchy({@ContextConfiguration(classes = {ResourcesTestConfig.class})})
61+
@WebMvcTest(TagsResourceTest.class)
62+
@TestPropertySource(locations = "classpath:no_ldap_test_application.properties")
63+
@ActiveProfiles({"test"})
64+
public class TagsResourceTest extends ResourcesTestBase {
65+
66+
@Autowired
67+
private TagRepository tagRepository;
68+
69+
private static Tag tag1;
70+
private static Tag tag2;
71+
72+
73+
@BeforeAll
74+
public static void init() {
75+
tag1 = new Tag("tag1", State.Active);
76+
tag2 = new Tag("tag2");
77+
}
78+
79+
@Test
80+
void testFindAll() throws Exception {
81+
when(tagRepository.findAll()).thenReturn(Arrays.asList(tag1, tag2));
82+
83+
MockHttpServletRequestBuilder request = get("/" + OlogResourceDescriptors.TAG_RESOURCE_URI);
84+
MvcResult result = mockMvc.perform(request).andExpect(status().isOk())
85+
.andReturn();
86+
Iterable<Tag> tags = objectMapper.readValue(result.getResponse().getContentAsString(),
87+
new TypeReference<>() {
88+
});
89+
assertEquals("tag1", tags.iterator().next().getName());
90+
verify(tagRepository, times(1)).findAll();
91+
}
92+
93+
@Test
94+
void testFindAllNoTags() throws Exception {
95+
when(tagRepository.findAll()).thenReturn(new ArrayList<>());
96+
97+
MockHttpServletRequestBuilder request = get("/" + OlogResourceDescriptors.TAG_RESOURCE_URI);
98+
MvcResult result = mockMvc.perform(request).andExpect(status().isOk())
99+
.andReturn();
100+
Iterable<Logbook> logbooks = objectMapper.readValue(result.getResponse().getContentAsString(),
101+
new TypeReference<>() {
102+
});
103+
assertFalse(logbooks.iterator().hasNext());
104+
verify(tagRepository, times(1)).findAll();
105+
reset(tagRepository);
106+
}
107+
108+
@Test
109+
void testFindTagByName() throws Exception {
110+
when(tagRepository.findById("tag1")).thenReturn(Optional.of(tag1));
111+
112+
MockHttpServletRequestBuilder request = get("/" +
113+
OlogResourceDescriptors.TAG_RESOURCE_URI +
114+
"/tag1");
115+
MvcResult result = mockMvc.perform(request).andExpect(status().isOk())
116+
.andReturn();
117+
Tag tag = objectMapper.readValue(result.getResponse().getContentAsString(),
118+
Tag.class);
119+
assertEquals("tag1", tag.getName());
120+
verify(tagRepository, times(1)).findById("tag1");
121+
reset(tagRepository);
122+
}
123+
124+
@Test
125+
void testCreateTagUnauthorized() throws Exception {
126+
MockHttpServletRequestBuilder request = put("/" +
127+
OlogResourceDescriptors.TAG_RESOURCE_URI +
128+
"/tag1")
129+
.content(objectMapper.writeValueAsString(tag1))
130+
.contentType(JSON);
131+
mockMvc.perform(request).andExpect(status().isUnauthorized());
132+
}
133+
134+
@Test
135+
void testCreateTag() throws Exception {
136+
Tag tag = new Tag("tag");
137+
when(tagRepository.save(tag)).thenReturn(tag);
138+
MockHttpServletRequestBuilder request = put("/" +
139+
OlogResourceDescriptors.TAG_RESOURCE_URI +
140+
"/tag")
141+
.header(HttpHeaders.AUTHORIZATION, AUTHORIZATION)
142+
.content(objectMapper.writeValueAsString(tag))
143+
.contentType(JSON);
144+
mockMvc.perform(request).andExpect(status().isOk()).andReturn();
145+
reset(tagRepository);
146+
}
147+
148+
@Test
149+
void testUpdateLogbooksUnauthorized() throws Exception {
150+
MockHttpServletRequestBuilder request = put("/" +
151+
OlogResourceDescriptors.LOGBOOK_RESOURCE_URI)
152+
.content(objectMapper.writeValueAsString(new ArrayList<>()))
153+
.contentType(JSON);
154+
mockMvc.perform(request).andExpect(status().isUnauthorized());
155+
}
156+
157+
@Test
158+
void testUpdateTags() throws Exception {
159+
List<Tag> tags = Arrays.asList(tag1, tag2);
160+
when(tagRepository.saveAll(tags)).thenReturn(tags);
161+
MockHttpServletRequestBuilder request = put("/" +
162+
OlogResourceDescriptors.TAG_RESOURCE_URI)
163+
.header(HttpHeaders.AUTHORIZATION, AUTHORIZATION)
164+
.content(objectMapper.writeValueAsString(tags))
165+
.contentType(JSON);
166+
MvcResult result = mockMvc.perform(request).andExpect(status().isOk()).andReturn();
167+
tags = objectMapper.readValue(result.getResponse().getContentAsString(),
168+
new TypeReference<>() {
169+
});
170+
assertEquals("tag1", tags.iterator().next().getName());
171+
verify(tagRepository, times(1)).saveAll(tags);
172+
reset(tagRepository);
173+
}
174+
175+
@Test
176+
void testDeleteUnauthorized() throws Exception {
177+
MockHttpServletRequestBuilder request = delete("/" +
178+
OlogResourceDescriptors.TAG_RESOURCE_URI +
179+
"/tag1");
180+
mockMvc.perform(request).andExpect(status().isUnauthorized());
181+
}
182+
183+
@Test
184+
void testDelete() throws Exception {
185+
MockHttpServletRequestBuilder request = delete("/" +
186+
OlogResourceDescriptors.TAG_RESOURCE_URI +
187+
"/tag1")
188+
.header(HttpHeaders.AUTHORIZATION, AUTHORIZATION);
189+
mockMvc.perform(request).andExpect(status().isNotFound());
190+
reset(tagRepository);
191+
}
192+
}

0 commit comments

Comments
 (0)