-
Notifications
You must be signed in to change notification settings - Fork 58
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
Modified backend and frontend for handling grouping of datasets #460
base: master
Are you sure you want to change the base?
Changes from 8 commits
d550337
8a0fe16
6130c24
0431bd9
6aac61e
66014cf
4e15ef8
05c8ff9
acd333a
4ac985c
4aa50ab
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 |
---|---|---|
|
@@ -23,11 +23,12 @@ | |
import org.aksw.gerbil.datatypes.ErrorTypes; | ||
import org.aksw.gerbil.datatypes.ExperimentType; | ||
import org.aksw.gerbil.exceptions.GerbilException; | ||
import org.aksw.gerbil.web.config.DatasetsConfig; | ||
|
||
/** | ||
* Contains all information needed to load an annotator for a specific | ||
* experiment type. | ||
* | ||
* | ||
* @author Michael Röder ([email protected]) | ||
* | ||
*/ | ||
|
@@ -39,7 +40,7 @@ public class AnnotatorConfigurationImpl extends AbstractAdapterConfiguration imp | |
public AnnotatorConfigurationImpl(String annotatorName, boolean couldBeCached, | ||
Constructor<? extends Annotator> constructor, Object constructorArgs[], | ||
ExperimentType applicableForExperiment) { | ||
super(annotatorName, couldBeCached, applicableForExperiment); | ||
super(annotatorName, DatasetsConfig.DEFAULT_DATASET_GROUP, couldBeCached, applicableForExperiment); | ||
this.constructor = constructor; | ||
this.constructorArgs = constructorArgs; | ||
} | ||
|
@@ -87,4 +88,4 @@ public String toString() { | |
builder.append(')'); | ||
return builder.toString(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package org.aksw.gerbil.dataset; | ||
|
||
import com.fasterxml.jackson.core.JsonGenerator; | ||
import com.fasterxml.jackson.databind.SerializerProvider; | ||
import com.fasterxml.jackson.databind.ser.std.StdSerializer; | ||
import org.aksw.gerbil.datatypes.AbstractAdapterConfiguration; | ||
|
||
import java.io.IOException; | ||
|
||
public class AdapterConfigSerializer extends StdSerializer<AbstractAdapterConfiguration> { | ||
|
||
|
||
public AdapterConfigSerializer(Class<AbstractAdapterConfiguration> t) { | ||
super(AbstractAdapterConfiguration.class); | ||
} | ||
|
||
@Override | ||
public void serialize(AbstractAdapterConfiguration value, JsonGenerator gen, SerializerProvider provider) throws IOException { | ||
gen.writeStartObject(); | ||
gen.writeStringField("name", value.getName()); | ||
gen.writeStringField("group", value.getGroup()); | ||
gen.writeEndObject(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,17 +18,14 @@ | |
|
||
import java.io.File; | ||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Set; | ||
import java.util.*; | ||
|
||
import javax.annotation.PostConstruct; | ||
import javax.servlet.http.HttpServletRequest; | ||
|
||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.core.type.TypeReference; | ||
import org.aksw.gerbil.Experimenter; | ||
import org.aksw.gerbil.config.GerbilConfiguration; | ||
import org.aksw.gerbil.database.ExperimentDAO; | ||
|
@@ -306,18 +303,29 @@ public ModelAndView experiment(@RequestParam(value = "id") String id, HttpServle | |
} | ||
|
||
@RequestMapping("/datasets") | ||
public @ResponseBody List<String> datasets(@RequestParam(value = "experimentType") String experimentType) { | ||
public @ResponseBody Map<String, List<String>> datasets(@RequestParam(value = "experimentType") String experimentType) { | ||
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. Returning a Set or List of adapter configurations and serializing them as JSON would be better, since it allows us to add more details about the datasets later on. I sent you a link about how the Jackson serialization (used by the Spring framework) works. You can also try to simply return the set of configuration objects as it is since the serialization is typically not hard for most Java objects. 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. Done 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. That is a solution that would work 👍 However, the current implementation handles something that the Spring framework can already do for you. I am afraid that I have described the goal in an ambiguous way, so let me try to explain what I would suggest. Of course, Spring cannot do magic things, so we still have to provide the serialization 😉 |
||
ExperimentType type = null; | ||
Map<String, List<String>> response = new TreeMap<>(); | ||
ObjectMapper mapper = new ObjectMapper(); | ||
try { | ||
type = ExperimentType.valueOf(experimentType); | ||
} catch (IllegalArgumentException e) { | ||
LOGGER.warn("Got a request containing a wrong ExperimentType (\"{}\"). Ignoring it.", experimentType); | ||
return null; | ||
} | ||
Set<String> datasets = adapterManager.getDatasetNamesForExperiment(type); | ||
List<String> list = Lists.newArrayList(datasets); | ||
Collections.sort(list); | ||
return list; | ||
List<String> adapterDetailsJsonList = adapterManager.getDatasetDetailsForExperiment(type); | ||
for (String adapterJson : adapterDetailsJsonList) { | ||
try { | ||
Map<String, String> adapterDetails = mapper.readValue(adapterJson, new TypeReference<Map<String, String>>() {}); | ||
String name = adapterDetails.get("name"); | ||
String group = adapterDetails.get("group"); | ||
response.computeIfAbsent(group, k -> new ArrayList<>()).add(name); | ||
Collections.sort(response.get(group)); | ||
} catch (IOException e) { | ||
LOGGER.error("Failed to parse adapter details JSON: {}", adapterJson, e); | ||
} | ||
} | ||
return response; | ||
MichaelRoeder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
|
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.
Should that be
super(t);
?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.
This is also what the Codacy check marked
https://app.codacy.com/gh/dice-group/gerbil/pull-requests/460/issues#issue-dd4f822d621fb37fa185c60c364e8db0