-
Notifications
You must be signed in to change notification settings - Fork 66
Allow adding a large product #1075
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
base: master
Are you sure you want to change the base?
Changes from all commits
e3b5e7d
67400c3
31c5cef
6eb9282
962798d
505ae48
71ec319
9fda38d
0b0019f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,15 +35,23 @@ | |
import javax.servlet.MultipartConfigElement; | ||
import javax.servlet.http.HttpServletRequest; | ||
import javax.ws.rs.InternalServerErrorException; | ||
import javax.ws.rs.core.HttpHeaders; | ||
import javax.ws.rs.core.MediaType; | ||
import javax.ws.rs.core.MultivaluedHashMap; | ||
import javax.ws.rs.core.MultivaluedMap; | ||
import javax.ws.rs.core.UriBuilder; | ||
import javax.ws.rs.core.UriInfo; | ||
import org.apache.commons.codec.CharEncoding; | ||
import org.apache.commons.io.IOUtils; | ||
import org.apache.commons.lang.StringUtils; | ||
import org.apache.cxf.jaxrs.impl.UriBuilderImpl; | ||
import org.apache.http.HttpStatus; | ||
import org.codice.ddf.catalog.ui.config.ConfigurationApplication; | ||
import org.codice.ddf.catalog.ui.util.jaxrs.JaxRsHttpHeaders; | ||
import org.codice.ddf.catalog.ui.util.jaxrs.JaxRsUriInfo; | ||
import org.codice.ddf.catalog.ui.util.multipart.CleanableMultipartBody; | ||
import org.codice.ddf.catalog.ui.util.multipart.CleanableMultipartBodyFactory; | ||
import org.codice.ddf.endpoints.rest.RESTEndpoint; | ||
import org.codice.ddf.rest.api.CatalogService; | ||
import org.codice.ddf.rest.api.CatalogServiceException; | ||
import org.slf4j.Logger; | ||
|
@@ -66,16 +74,27 @@ public class CatalogApplication implements SparkApplication { | |
|
||
private static final String HEADER_ACCEPT_RANGES = "Accept-Ranges"; | ||
|
||
private static final String HEADER_ID = "id"; | ||
|
||
private static final String BYTES = "bytes"; | ||
|
||
private static final String CATALOG_PATH = "/catalog/"; | ||
|
||
private static final String CATALOG_ID_PATH = "/catalog/:id"; | ||
|
||
private static final String TRANSFORM = "transform"; | ||
|
||
private final ConfigurationApplication config; | ||
|
||
private CatalogService catalogService; | ||
|
||
public CatalogApplication(CatalogService catalogService) { | ||
private RESTEndpoint restEndpoint; | ||
|
||
public CatalogApplication( | ||
CatalogService catalogService, RESTEndpoint restEndpoint, ConfigurationApplication config) { | ||
this.catalogService = catalogService; | ||
this.restEndpoint = restEndpoint; | ||
this.config = config; | ||
} | ||
|
||
@Override | ||
|
@@ -141,35 +160,52 @@ public void init() { | |
}); | ||
|
||
post( | ||
"/catalog/", | ||
CATALOG_PATH, | ||
(req, res) -> { | ||
if (req.contentType().startsWith("multipart/")) { | ||
req.attribute( | ||
ECLIPSE_MULTIPART_CONFIG, | ||
new MultipartConfigElement(System.getProperty(JAVA_IO_TMPDIR))); | ||
try { | ||
LOGGER.debug("POST Path: {}", CATALOG_PATH); | ||
String contentType = req.contentType(); | ||
|
||
// Convert req and res for RESTEndpoint | ||
HttpServletRequest httpRequest = req.raw(); | ||
HttpHeaders headers = new JaxRsHttpHeaders(req); | ||
UriInfo uriInfo = new JaxRsUriInfo(req); | ||
String transformerParam = req.queryParams(TRANSFORM); | ||
InputStream inputStream = httpRequest.getInputStream(); | ||
|
||
if (contentType.startsWith("multipart/")) { | ||
LOGGER.debug("POST Path: {} multipart", CATALOG_PATH); | ||
CleanableMultipartBody multipartBody = | ||
CleanableMultipartBodyFactory.create( | ||
httpRequest, config.getMaximumUploadSize(), config.getMaxFileSizeInMemory()); | ||
|
||
javax.ws.rs.core.Response response = | ||
restEndpoint.addDocument( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ I don't understand the reasoning for passing this off to the DDF RESTEndpoint. The DDF RESTEndpoint does basically the same thing that the original In the end, though, there is no good reason to embed and use the RESTEndpoint code here that is a simple wrapper around the catalog service calls that we can already make here in this class. At a minimum, we should change this class' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried your suggestions..
Error:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copy and pasting from email
Error:
|
||
headers, uriInfo, httpRequest, multipartBody, transformerParam, inputStream); | ||
|
||
multipartBody.cleanup(); | ||
|
||
return setResponse( | ||
res, httpRequest.getRequestURL(), response.getHeaderString(HEADER_ID)); | ||
} | ||
|
||
return addDocument( | ||
res, | ||
req.raw().getRequestURL(), | ||
req.contentType(), | ||
req.queryParams(TRANSFORM), | ||
req.raw(), | ||
new ByteArrayInputStream(req.bodyAsBytes())); | ||
} | ||
if (contentType.startsWith("text/") || contentType.startsWith("application/")) { | ||
LOGGER.debug("POST Path: {} text/application", CATALOG_PATH); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: slightly misleading debug statement. The string "text/application" in this context looks like a MIME type, but it's not. I get what you mean, but it confuse someone looking that the logs. Suggestion: "text/* or application/*" |
||
javax.ws.rs.core.Response response = | ||
restEndpoint.addDocument( | ||
headers, uriInfo, httpRequest, transformerParam, inputStream); | ||
|
||
if (req.contentType().startsWith("text/") | ||
|| req.contentType().startsWith("application/")) { | ||
return addDocument( | ||
res, | ||
req.raw().getRequestURL(), | ||
req.contentType(), | ||
req.queryParams(TRANSFORM), | ||
null, | ||
new ByteArrayInputStream(req.bodyAsBytes())); | ||
} | ||
return setResponse( | ||
res, httpRequest.getRequestURL(), response.getHeaderString(HEADER_ID)); | ||
} | ||
|
||
res.status(HttpStatus.SC_NOT_FOUND); | ||
return res; | ||
res.status(HttpStatus.SC_NOT_FOUND); | ||
return "Not Found"; | ||
} catch (Exception e) { | ||
LOGGER.error("Unexpected error in request handler", e); | ||
res.status(HttpStatus.SC_INTERNAL_SERVER_ERROR); | ||
return "Internal Server Error"; | ||
} | ||
}); | ||
|
||
put( | ||
|
@@ -373,19 +409,8 @@ private String updateDocument( | |
} | ||
} | ||
|
||
private String addDocument( | ||
Response res, | ||
StringBuffer requestUrl, | ||
String contentType, | ||
String transformerParam, | ||
HttpServletRequest httpServletRequest, | ||
InputStream inputStream) { | ||
private String setResponse(Response res, StringBuffer requestUrl, String id) { | ||
try { | ||
List<String> contentTypeList = ImmutableList.of(contentType); | ||
String id = | ||
catalogService.addDocument( | ||
contentTypeList, httpServletRequest, transformerParam, inputStream); | ||
|
||
URI uri = new URI(requestUrl.toString()); | ||
UriBuilder uriBuilder = new UriBuilderImpl(uri).path("/" + id); | ||
|
||
|
@@ -394,7 +419,7 @@ private String addDocument( | |
res.header(Metacard.ID, id); | ||
return ""; | ||
|
||
} catch (CatalogServiceException | URISyntaxException e) { | ||
} catch (URISyntaxException e) { | ||
return createBadRequestResponse(res, e.getMessage()); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,7 +168,8 @@ public class ConfigurationApplication implements SparkApplication { | |
|
||
private String mapHome = ""; | ||
|
||
private int maximumUploadSize = 1_048_576; | ||
private long maximumUploadSize = 1_048_576; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✏️ delete old int variable and update Will want to add maxFileSizeInMemory to the above file too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm I'm confused. It's already been done like that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry - I copy & pasted the wrong filepath 🙃 |
||
private int maxFileSizeInMemory = 50 * 1024 * 1024; // 50 MB | ||
|
||
private List<String> readOnly = | ||
ImmutableList.of( | ||
|
@@ -402,11 +403,11 @@ public void setResultShow(List<String> resultShow) { | |
this.resultShow = resultShow; | ||
} | ||
|
||
public void setMaximumUploadSize(int size) { | ||
public void setMaximumUploadSize(long size) { | ||
this.maximumUploadSize = size; | ||
} | ||
|
||
public int getMaximumUploadSize() { | ||
public long getMaximumUploadSize() { | ||
return maximumUploadSize; | ||
} | ||
|
||
|
@@ -573,6 +574,8 @@ public Map<String, Object> getConfig() { | |
config.put("menuIconSrc", menuIconSrc); | ||
config.put("customBranding", customBranding); | ||
config.put("extra", extra); | ||
config.put("maximumUploadSize", maximumUploadSize); | ||
config.put("maxFileSizeInMemory", maxFileSizeInMemory); | ||
|
||
return config; | ||
} | ||
|
@@ -1264,4 +1267,12 @@ public String getMenuIconSrc() { | |
public void setMenuIconSrc(String menuIconSrc) { | ||
this.menuIconSrc = menuIconSrc; | ||
} | ||
|
||
public void setMaxFileSizeInMemory(int size) { | ||
this.maxFileSizeInMemory = size; | ||
} | ||
|
||
public int getMaxFileSizeInMemory() { | ||
return maxFileSizeInMemory; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
package org.codice.ddf.catalog.ui.util.jaxrs; | ||
|
||
import java.util.*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't do star imports. |
||
import javax.ws.rs.core.Cookie; | ||
import javax.ws.rs.core.HttpHeaders; | ||
import javax.ws.rs.core.MediaType; | ||
import javax.ws.rs.core.MultivaluedHashMap; | ||
import javax.ws.rs.core.MultivaluedMap; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import spark.Request; | ||
|
||
public class JaxRsHttpHeaders implements HttpHeaders { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(JaxRsHttpHeaders.class); | ||
private final Request sparkRequest; | ||
|
||
public JaxRsHttpHeaders(Request sparkRequest) { | ||
this.sparkRequest = sparkRequest; | ||
} | ||
|
||
@Override | ||
public List<String> getRequestHeader(String name) { | ||
String header = sparkRequest.headers(name); | ||
return header != null ? Collections.singletonList(header) : Collections.emptyList(); | ||
} | ||
|
||
@Override | ||
public String getHeaderString(String s) { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getHeaderString()"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make sense to throw a "NotImplementedException"? |
||
return ""; | ||
} | ||
|
||
@Override | ||
public MultivaluedMap<String, String> getRequestHeaders() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getRequestHeaders()"); | ||
return new MultivaluedHashMap<>(); | ||
} | ||
|
||
@Override | ||
public List<MediaType> getAcceptableMediaTypes() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getAcceptableMediaTypes()"); | ||
return List.of(); | ||
} | ||
|
||
@Override | ||
public List<Locale> getAcceptableLanguages() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getAcceptableLanguages()"); | ||
return List.of(); | ||
} | ||
|
||
@Override | ||
public MediaType getMediaType() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getMediaType()"); | ||
return null; | ||
} | ||
|
||
@Override | ||
public Locale getLanguage() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getLanguage()"); | ||
return null; | ||
} | ||
|
||
@Override | ||
public Map<String, Cookie> getCookies() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getCookies()"); | ||
return Map.of(); | ||
} | ||
|
||
@Override | ||
public Date getDate() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getDate()"); | ||
return null; | ||
} | ||
|
||
@Override | ||
public int getLength() { | ||
LOGGER.warn("Not implemented: JaxRsHttpHeaders.getLength()"); | ||
return 0; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually it is bad practice to embed bundles. I would expect that the rest endpoint classes you are trying to access would be available within the OSGI service registry and so embedding this jar shouldn't be necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, we should not be embedding this bundle here as it will likely cause unintended consequences. Also see my other comments on its use later as this shouldn't be needed.