Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Improved servlet lifecycles for AudioServlet and ChartServlet #6161

Merged
merged 6 commits into from Sep 7, 2018
Merged

Improved servlet lifecycles for AudioServlet and ChartServlet #6161

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9c214eb
Improved servlet lifecycles for AudioServlet and ChartServlet
kaikreuzer Sep 6, 2018
d6fe191
also adapted DefaultChartProvider
kaikreuzer Sep 6, 2018
8824d2b
send proper HTTPResponses instead of exceptions
kaikreuzer Sep 6, 2018
b1724c4
removed empty deactivate method
kaikreuzer Sep 6, 2018
75a90ee
properly handle NumberFormatExceptions
kaikreuzer Sep 6, 2018
8caf42c
changed config description to correct types
kaikreuzer Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 15 additions & 8 deletions ...home.core.audio/src/main/java/org/eclipse/smarthome/core/audio/internal/AudioServlet.java
View file
Open in desktop
Expand Up @@ -33,7 +33,9 @@
import org.eclipse.smarthome.core.audio.AudioHTTPServer;
import org.eclipse.smarthome.core.audio.AudioStream;
import org.eclipse.smarthome.core.audio.FixedLengthAudioStream;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.http.HttpContext;
import org.osgi.service.http.HttpService;
Expand Down Expand Up @@ -63,10 +65,8 @@ public class AudioServlet extends HttpServlet implements AudioHTTPServer {

protected HttpService httpService;

@Reference
protected void setHttpService(HttpService httpService) {
this.httpService = httpService;

@Activate
protected void activate() {
try {
logger.debug("Starting up the audio servlet at " + SERVLET_NAME);
Hashtable<String, String> props = new Hashtable<String, String>();
Expand All @@ -78,8 +78,17 @@ protected void setHttpService(HttpService httpService) {
}
}

protected void unsetHttpService(HttpService httpService) {
@Deactivate
protected void deactivate() {
httpService.unregister(SERVLET_NAME);
}

@Reference
protected void setHttpService(HttpService httpService) {
this.httpService = httpService;
}

protected void unsetHttpService(HttpService httpService) {
this.httpService = null;
}

Expand Down Expand Up @@ -129,9 +138,7 @@ private InputStream prepareInputStream(final String streamId, final HttpServletR
// try to set the content-length, if possible
if (stream instanceof FixedLengthAudioStream) {
final Long size = ((FixedLengthAudioStream) stream).length();
if (size != null) {
resp.setContentLength(size.intValue());
}
resp.setContentLength(size.intValue());
}

if (multiAccess) {
Expand Down
41 changes: 20 additions & 21 deletions ...ipse.smarthome.ui/src/main/java/org/eclipse/smarthome/ui/internal/chart/ChartServlet.java
View file
Open in desktop
Expand Up @@ -21,11 +21,11 @@
import java.util.Hashtable;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;

import javax.imageio.IIOException;
import javax.imageio.ImageIO;
import javax.imageio.stream.ImageOutputStream;
import javax.servlet.Servlet;
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
Expand All @@ -35,7 +35,6 @@
import org.apache.commons.lang.BooleanUtils;
import org.eclipse.smarthome.core.items.ItemNotFoundException;
import org.eclipse.smarthome.ui.chart.ChartProvider;
import org.eclipse.smarthome.ui.items.ItemUIRegistry;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
Expand Down Expand Up @@ -68,7 +67,7 @@
* @author Holger Reichert - Support for themes, DPI, legend hiding
*
*/
@Component(immediate = true, service = Servlet.class, configurationPid = "org.eclipse.smarthome.chart", property = {
@Component(immediate = true, service = ChartServlet.class, configurationPid = "org.eclipse.smarthome.chart", property = {
"service.pid=org.eclipse.smarthome.chart", "service.config.description.uri=system:chart",
"service.config.label=Charts", "service.config.category=system" })
public class ChartServlet extends HttpServlet {
Expand Down Expand Up @@ -108,10 +107,9 @@ public class ChartServlet extends HttpServlet {
}

protected HttpService httpService;
protected ItemUIRegistry itemUIRegistry;
protected static Map<String, ChartProvider> chartProviders = new HashMap<String, ChartProvider>();
protected static Map<String, ChartProvider> chartProviders = new ConcurrentHashMap<String, ChartProvider>();

@Reference(policy = ReferencePolicy.DYNAMIC)
@Reference
public void setHttpService(HttpService httpService) {
this.httpService = httpService;
}
Expand All @@ -120,15 +118,6 @@ public void unsetHttpService(HttpService httpService) {
this.httpService = null;
}

@Reference(policy = ReferencePolicy.DYNAMIC)
public void setItemUIRegistry(ItemUIRegistry itemUIRegistry) {
this.itemUIRegistry = itemUIRegistry;
}

public void unsetItemUIRegistry(ItemUIRegistry itemUIRegistry) {
this.itemUIRegistry = null;
}

@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
public void addChartProvider(ChartProvider provider) {
chartProviders.put(provider.getName(), provider);
Expand Down Expand Up @@ -208,6 +197,7 @@ private void applyConfig(Map<String, Object> config) {
}
}

@SuppressWarnings({ "null" })
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException {
logger.debug("Received incoming chart request: {}", req);
Expand All @@ -230,7 +220,9 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser
// To avoid ambiguity you are not allowed to specify period, begin and end time at the same time.
if (req.getParameter("period") != null && req.getParameter("begin") != null
&& req.getParameter("end") != null) {
throw new ServletException("Do not specify the three parameters period, begin and end at the same time.");
res.sendError(HttpServletResponse.SC_BAD_REQUEST,
"Do not specify the three parameters period, begin and end at the same time.");
return;
}

// Read out the parameter period, begin and end and save them.
Expand All @@ -247,15 +239,19 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser
try {
timeBegin = new SimpleDateFormat(DATE_FORMAT).parse(req.getParameter("begin"));
} catch (ParseException e) {
throw new ServletException("Begin and end must have this format: " + DATE_FORMAT + ".");
res.sendError(HttpServletResponse.SC_BAD_REQUEST,
"Begin and end must have this format: " + DATE_FORMAT + ".");
return;
}
}

if (req.getParameter("end") != null) {
try {
timeEnd = new SimpleDateFormat(DATE_FORMAT).parse(req.getParameter("end"));
} catch (ParseException e) {
throw new ServletException("Begin and end must have this format: " + DATE_FORMAT + ".");
res.sendError(HttpServletResponse.SC_BAD_REQUEST,
"Begin and end must have this format: " + DATE_FORMAT + ".");
return;
}
}

Expand All @@ -279,7 +275,8 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser

ChartProvider provider = getChartProviders().get(providerName);
if (provider == null) {
throw new ServletException("Could not get chart provider.");
res.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Could not get chart provider.");
return;
}

// Read out the parameter 'dpi'
Expand All @@ -288,10 +285,12 @@ protected void doGet(HttpServletRequest req, HttpServletResponse res) throws Ser
try {
dpi = Integer.valueOf(req.getParameter("dpi"));
} catch (NumberFormatException e) {
throw new ServletException("dpi parameter is invalid");
res.sendError(HttpServletResponse.SC_BAD_REQUEST, "dpi parameter is invalid");
return;
}
if (dpi <= 0) {
throw new ServletException("dpi parameter is <= 0");
res.sendError(HttpServletResponse.SC_BAD_REQUEST, "dpi parameter is <= 0");
return;
}
}

Expand Down
6 changes: 1 addition & 5 deletions ...va/org/eclipse/smarthome/ui/internal/chart/defaultchartprovider/DefaultChartProvider.java
View file
Open in desktop
Expand Up @@ -54,7 +54,6 @@
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Deactivate;
import org.osgi.service.component.annotations.Reference;
import org.osgi.service.component.annotations.ReferencePolicy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -85,7 +84,7 @@ public class DefaultChartProvider implements ChartProvider {

public static final int DPI_DEFAULT = 96;

@Reference(policy = ReferencePolicy.DYNAMIC)
@Reference
public void setItemUIRegistry(ItemUIRegistry itemUIRegistry) {
this.itemUIRegistry = itemUIRegistry;
}
Expand Down Expand Up @@ -125,9 +124,6 @@ protected void activate() {
protected void deactivate() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be removed.

}

public void destroy() {
}

@Override
public String getName() {
return "default";
Expand Down