Added dispatcher and output format #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- configuration file parser and change listerner - db storage - file storage - clean task
nmco
reviewed
Sep 1, 2017
src/community/nsg-profile/pom.xml
Outdated
| <groupId>org.eclipse.emf</groupId> | ||
| <artifactId>org.eclipse.emf.ecore.xmi</artifactId> | ||
| <version>2.12.0</version> | ||
| </dependency> |
There was a problem hiding this comment.
No need to declare the version here (and it should not be done here anyway), GS root pom.xml dependency-management section already declare this dependency with the same version.
All dependencies should always be declared on the root pom.xml dependency management section to force every core module, extension and community module to sue the same version.
| * | ||
| * @param currentDataStoreParams | ||
| * @param currentDataStore | ||
| */ |
There was a problem hiding this comment.
Invalid java-doc @param values, this function has only one parameter.
| * @param timeToLive | ||
| */ | ||
| public static void setTimeToLive(Long timeToLive) { | ||
| IndexConfiguration.timeToLive = timeToLive; |
There was a problem hiding this comment.
When dealing with times the time unit should always be clearly defined, for example timeToLiveSec or better the method could named setTimeToLiveInSec, another more advanced option is too explicitly ask the invoker to provide the time unit using the TimeUnit enum.
|
|
||
| public static Long getTimeToLive() { | ||
| return timeToLive; | ||
| } |
There was a problem hiding this comment.
Similar to the setter method, the time unit used should be clearly stayed, for example getTimeToLiveInSec or getSecondsToLive.
| private static Resource storageResource; | ||
|
|
||
| private static Long timeToLive = 600l; | ||
|
|
There was a problem hiding this comment.
The time unit used should be explicitly stayed.
| featureStore.addFeatures(collection); | ||
|
|
||
| // Create and store file | ||
| Resource storageResource = IndexConfiguration.getStorageResource(); |
There was a problem hiding this comment.
It seems to be that this block of code and the IndexInitializer manage methods should by synchronizer by a read-write lock. If a request comes and we get the storage location but in the meantime the storage location is changed, this request will be stored in the old path causing a very nasty bug to found.
I think the best option here would be a ReadWriteLock shared by the IndexConfiguration and used by the IndexInitializer and the IndexOutputFormat. Any operation that needs to access the storage and database should need a read lock (multiple threads will be able to access it at the same time) but the operations in IndexInitializer will need to obtain a write lock.
|
|
||
| public class IndexResultTypeDispatcherCallback extends AbstractDispatcherCallback { | ||
|
|
||
| static Logger LOGGER = Logging.getLogger(IndexResultTypeDispatcherCallback.class); |
| Filter filter = CQL.toFilter("ID = '" + resultSetID + "'"); | ||
| store.modifyFeatures("updated", new Date().getTime(), filter); | ||
| // Retrieve GetFeature from file | ||
| Resource storageResource = IndexConfiguration.getStorageResource(); |
There was a problem hiding this comment.
Same concurrent issue that when storing.
|
|
||
| public class PageResultsDispatcherCallback extends AbstractDispatcherCallback { | ||
|
|
||
| static Logger LOGGER = Logging.getLogger(PageResultsDispatcherCallback.class); |
| PageResultsWebFeatureService prService = (PageResultsWebFeatureService) service | ||
| .getService(); | ||
| prService.setResultSetID((String) request.getKvp().get("resultSetID")); | ||
| request.getKvp().put("featureId", Arrays.asList("dummy")); |
There was a problem hiding this comment.
I know this is a hack that may be removed ... in this cases is better use Collections.singletonList("dummy").
|
Hi Nuno,
fantastic review! I will apply the suggested modifies.
What about EMF read problem? I think it's quite a critical problem.
Thanks.
2017-09-02 1:24 GMT+02:00 Nuno Oliveira <[email protected]>:
… ***@***.**** commented on this pull request.
------------------------------
In src/community/nsg-profile/pom.xml
<#1 (comment)>:
> </dependency>
+ <!-- geotools dependencies -->
+ <dependency>
+ <groupId>org.geotools.jdbc</groupId>
+ <artifactId>gt-jdbc-h2</artifactId>
+ </dependency>
+ <!-- emf dependencies-->
+ <dependency>
+ <groupId>org.eclipse.emf</groupId>
+ <artifactId>org.eclipse.emf.ecore.xmi</artifactId>
+ <version>2.12.0</version>
+ </dependency>
No need to declare the version here (and it should not be done here
anyway), GS root pom.xml dependency-management section already declare
this dependency with the same version.
All dependencies should always be declared on the root pom.xml dependency
management section to force every core module, extension and community
module to sue the same version.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexConfiguration.java
<#1 (comment)>:
> + *
+ * @param currentDataStoreParams
+ * @param currentDataStore
+ */
+ public static void setCurrentDataStore(Map<String, Object> currentDataStoreParams,
+ DataStore currentDataStore) {
+ IndexConfiguration.currentDataStoreParams = currentDataStoreParams;
+ IndexConfiguration.currentDataStore = currentDataStore;
+ }
+
+ /**
+ * Store the reference to resource used to archive the serialized GetFeatureRequest
+ *
+ * @param currentDataStoreParams
+ * @param currentDataStore
+ */
Invalid java-doc @param values, this function has only one parameter.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexConfiguration.java
<#1 (comment)>:
> + * Store the reference to resource used to archive the serialized GetFeatureRequest
+ *
+ * @param currentDataStoreParams
+ * @param currentDataStore
+ */
+ public static void setStorageResource(Resource storageResource) {
+ IndexConfiguration.storageResource = storageResource;
+ }
+
+ /**
+ * Store the value of time to live of stored GetFeatureRequest
+ *
+ * @param timeToLive
+ */
+ public static void setTimeToLive(Long timeToLive) {
+ IndexConfiguration.timeToLive = timeToLive;
When dealing with times the time unit should always be clearly defined,
for example timeToLiveSec or better the method could named
setTimeToLiveInSec, another more advanced option is too explicitly ask
the invoker to provide the time unit using the TimeUnit enum.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexConfiguration.java
<#1 (comment)>:
> +
+ public static DataStore getCurrentDataStore() {
+ return currentDataStore;
+ }
+
+ public static Map<String, Object> getCurrentDataStoreParams() {
+ return currentDataStoreParams;
+ }
+
+ public static Resource getStorageResource() {
+ return storageResource;
+ }
+
+ public static Long getTimeToLive() {
+ return timeToLive;
+ }
Similar to the setter method, the time unit used should be clearly stayed,
for example getTimeToLiveInSec or getSecondsToLive.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexConfiguration.java
<#1 (comment)>:
> +
+/**
+ *
+ * Class used to store the index result type configuration managed by ***@***.*** IndexInitializer}
+ *
+ * @author sandr
+ *
+ */
+public class IndexConfiguration {
+
+ private static DataStore currentDataStore;
+
+ private static Resource storageResource;
+
+ private static Long timeToLive = 600l;
+
The time unit used should be explicitly stayed.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + public void initialize(GeoServer geoServer) throws Exception {
+ GeoServerResourceLoader loader = GeoServerExtensions.bean(GeoServerResourceLoader.class);
+ GeoServerDataDirectory dd = new GeoServerDataDirectory(loader);
+ Resource resource = dd.get(MODULE_DIR + "/" + PROPERTY_FILENAME);
+ if (loader != null) {
+ File directory = loader.findOrCreateDirectory(MODULE_DIR);
+ File file = new File(directory, PROPERTY_FILENAME);
+ // Create default configuration file
+ if (!file.exists()) {
+ InputStream stream = IndexInitializer.class
+ .getResourceAsStream("/" + PROPERTY_FILENAME);
+ Properties properties = new Properties();
+ properties.load(stream);
+ // Replace GEOSERVER_DATA_DIR placeholder
+ properties.replaceAll((k, v) -> ((String) v).replace("${GEOSERVER_DATA_DIR}",
+ dd.root().getPath()));
Nice one !
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> +
+ @OverRide
+ public void initialize(GeoServer geoServer) throws Exception {
+ GeoServerResourceLoader loader = GeoServerExtensions.bean(GeoServerResourceLoader.class);
+ GeoServerDataDirectory dd = new GeoServerDataDirectory(loader);
+ Resource resource = dd.get(MODULE_DIR + "/" + PROPERTY_FILENAME);
+ if (loader != null) {
+ File directory = loader.findOrCreateDirectory(MODULE_DIR);
+ File file = new File(directory, PROPERTY_FILENAME);
+ // Create default configuration file
+ if (!file.exists()) {
+ InputStream stream = IndexInitializer.class
+ .getResourceAsStream("/" + PROPERTY_FILENAME);
+ Properties properties = new Properties();
+ properties.load(stream);
+ // Replace GEOSERVER_DATA_DIR placeholder
Method load doesn't close the stream so this stream stays open.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + if (loader != null) {
+ File directory = loader.findOrCreateDirectory(MODULE_DIR);
+ File file = new File(directory, PROPERTY_FILENAME);
+ // Create default configuration file
+ if (!file.exists()) {
+ InputStream stream = IndexInitializer.class
+ .getResourceAsStream("/" + PROPERTY_FILENAME);
+ Properties properties = new Properties();
+ properties.load(stream);
+ // Replace GEOSERVER_DATA_DIR placeholder
+ properties.replaceAll((k, v) -> ((String) v).replace("${GEOSERVER_DATA_DIR}",
+ dd.root().getPath()));
+ // Create resource and save properties
+ OutputStream out = resource.out();
+ properties.store(out, null);
+ out.close();
I would do this with a try { ... } finally { ... } block.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + .getResourceAsStream("/" + PROPERTY_FILENAME);
+ Properties properties = new Properties();
+ properties.load(stream);
+ // Replace GEOSERVER_DATA_DIR placeholder
+ properties.replaceAll((k, v) -> ((String) v).replace("${GEOSERVER_DATA_DIR}",
+ dd.root().getPath()));
+ // Create resource and save properties
+ OutputStream out = resource.out();
+ properties.store(out, null);
+ out.close();
+ }
+ loadConfigurations(resource);
+ // Listen for changes in configuration file and reload properties
+ resource.addListener(new ResourceListener() {
+ @OverRide
+ public void changed(ResourceNotification notify) {
Can be substituted with a lambda expression:
resource.addListener(notify -> {
if (notify.getKind() == Kind.ENTRY_MODIFY) {
try {
loadConfigurations(resource);
} catch (Exception exception) {
throw new RuntimeException("Error reload confiugrations.", exception);
}
}
});
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + // Create resource and save properties
+ OutputStream out = resource.out();
+ properties.store(out, null);
+ out.close();
+ }
+ loadConfigurations(resource);
+ // Listen for changes in configuration file and reload properties
+ resource.addListener(new ResourceListener() {
+ @OverRide
+ public void changed(ResourceNotification notify) {
+ if (notify.getKind() == Kind.ENTRY_MODIFY) {
+ try {
+ loadConfigurations(resource);
+ } catch (Exception exception) {
+ throw new RuntimeException("Error reload confiugrations.", exception);
+ }
Typo you probably want something like this Error reloading configurations.
.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + * <li>When the storage path is changed, the new storage path should be used and the old storage
+ * path content should be moved to the new one,
+ * <li>When the the time to live is changed the ***@***.*** #clean()} procedure will update.
+ * </ul>
+ *
+ * The class is also responsible to ***@***.*** #clean()} the stored requests (result sets) that have not
+ * been used for a period of time bigger than the configured time to live value
+ * <p>
+ *
+ * @author sandr
+ *
+ */
+
+public class IndexInitializer implements GeoServerInitializer {
+
+ static Logger LOGGER = Logging.getLogger(IndexInitializer.class);
Can be final.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + public void changed(ResourceNotification notify) {
+ if (notify.getKind() == Kind.ENTRY_MODIFY) {
+ try {
+ loadConfigurations(resource);
+ } catch (Exception exception) {
+ throw new RuntimeException("Error reload confiugrations.", exception);
+ }
+ }
+ }
+ });
+ }
+ }
+
+ /**
+ * Helper method that
+ */
Incomplete java-doc.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + throw new RuntimeException("Error reload confiugrations.", exception);
+ }
+ }
+ }
+ });
+ }
+ }
+
+ /**
+ * Helper method that
+ */
+ private void loadConfigurations(Resource resource) throws IOException {
+ synchronized (lock) {
+ Properties properties = new Properties();
+ properties.load(resource.in());
+ // Reload database
Resource stream obtained with resource.in() needs to be closed
explicitly, the load(...) method doesn't close the stream.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + * If the storage path is changed, the new storage path should be used and the old
+ * storage path content should be moved to the new one
+ */
+ manageStorageChange(resource, properties.get("resultSets.storage.path"));
+ /*
+ * Change time to live
+ */
+ manageTimeToLiveChange(properties.get("resultSets.timeToLive"));
+ }
+
+ }
+
+ /**
+ * Helper method that
+ */
+ private void manageTimeToLiveChange(Object timneToLive) {
Incomplete java-doc and the time unit used should be explicit.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + }
+ } else {
+ // Create schema
+ createTable(newDataStore, false);
+ }
+ IndexConfiguration.setCurrentDataStore(params, newDataStore);
+ } catch (Exception exception) {
+ throw new RuntimeException("Error reload DB confiugrations.", exception);
+ }
+ }
+
+ /**
+ * Helper method that check id the DB is the same, matching the JDBC configurations parameters.
+ */
+ private Boolean isStorageTheSame(Map<String, Object> newParams) {
+ Map<String, Object> currentParams = IndexConfiguration.getCurrentDataStoreParams();
Typo in the java-doc ...check if the DB is..., I would rename this method
to make it clear that it checks if the database is the same not the storage.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + .equals(newParams.get(JDBCDataStoreFactory.DBTYPE.key))
+ && currentParams.get(JDBCDataStoreFactory.DATABASE.key)
+ .equals(newParams.get(JDBCDataStoreFactory.DATABASE.key))
+ && currentParams.get(JDBCDataStoreFactory.HOST.key)
+ .equals(newParams.get(JDBCDataStoreFactory.HOST.key))
+ && currentParams.get(JDBCDataStoreFactory.PORT.key)
+ .equals(newParams.get(JDBCDataStoreFactory.PORT.key))
+ && currentParams.get(JDBCDataStoreFactory.SCHEMA.key)
+ .equals(newParams.get(JDBCDataStoreFactory.SCHEMA.key));
+ }
+
+ /**
+ * Helper method that create a new table on DB to store resource informations
+ */
+ private void createTable(DataStore dataStore, Boolean forceDelete) throws Exception {
+ Boolean exists = dataStore.getNames().contains(new NameImpl(STORE_SCHEMA_NAME));
Variable forceDelete can use the boolean primitive type here.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + }
+
+ /**
+ * Delete all the stored requests (result sets) that have not been used for a period of time
+ * bigger than the configured time to live value. Clean also related resource files.
+ * <p>
+ * Executed by scheduler, for details see Spring XML configuration
+ */
+ public void clean() throws Exception {
+ synchronized (lock) {
+ Transaction session = new DefaultTransaction("RemoveOld");
+ try {
+ // Remove record
+ Long timeToLive = IndexConfiguration.getTimeToLive();
+ DataStore currentDataStore = IndexConfiguration.getCurrentDataStore();
+ Long now = new Date().getTime();
I would change new Date().getTime() by System.currentTimeMillis().
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> + // Remove file
+ Resource currentResource = IndexConfiguration.getStorageResource();
+ SimpleFeatureIterator iterator = toRemoved.features();
+ try {
+ while (iterator.hasNext()) {
+ SimpleFeature feature = iterator.next();
+ currentResource.get(feature.getID()).delete();
+ }
+ } finally {
+ iterator.close();
+ }
+ store.removeFeatures(filter);
+ }
+ if (LOGGER.isLoggable(Level.FINEST)) {
+ LOGGER.finest("CLEAN executed, removed stored requests older than "
+ + new SimpleDateFormat("yyyy-MM-dd HH:mm:ss")
This log should only happen when something is actually removed and provide
the number of resources removed.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> +import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+
+import net.opengis.wfs20.GetFeatureType;
+
+/**
+ * This output format handles requests if the original requested result type was "index" </br>
+ * See ***@***.*** IndexResultTypeDispatcherCallback}
+ *
+ * @author sandr
+ *
+ */
+public class IndexOutputFormat extends HitsOutputFormat {
+
+ static Logger LOGGER = Logging.getLogger(IndexOutputFormat.class);
+
This could be private final. It is important to be the more restrictive
possible since this can ease future re-factorings, we can pretty much what
we want with a private variable but a public will require us to
deprecated and can probably only be done in a major release. Going from
private to public is normally not an issue and can even be back-ported.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> +
+import net.opengis.wfs20.GetFeatureType;
+
+/**
+ * This output format handles requests if the original requested result type was "index" </br>
+ * See ***@***.*** IndexResultTypeDispatcherCallback}
+ *
+ * @author sandr
+ *
+ */
+public class IndexOutputFormat extends HitsOutputFormat {
+
+ static Logger LOGGER = Logging.getLogger(IndexOutputFormat.class);
+
+ String resultSetId;
+
Same thing here can be a private variable.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> + // Register XMI serializer
+ resSet = new ResourceSetImpl();
+ resSet.getResourceFactoryRegistry().getExtensionToFactoryMap().put("feature",
+ new XMIResourceFactoryImpl());
+ }
+
+ public IndexOutputFormat(GeoServer gs) {
+ super(gs);
+ }
+
+ @OverRide
+ public void write(Object value, OutputStream output, Operation operation)
+ throws IOException, ServiceException {
+ // extract GetFeature request
+ GetFeatureType request = (GetFeatureType) OwsUtils.parameter(operation.getParameters(),
+ GetFeatureType.class);
Unnecessary cast.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> + } catch (Exception exception) {
+ throw new RuntimeException("Error encoding INDEX result.", exception);
+ }
+ // add the resultSetID attribute to the result
+ addResultSetIdElement(document, resultSetId);
+ // write the XML document to response output stream
+ writeDocument(document, output);
+ }
+
+ /**
+ * Helper method that serialize GetFeature request, store it in the file system and associate it
+ * with resultSetId
+ *
+ * @param request
+ * @param resultSetId
+ * @throws Exception
Non correct java-doc parameters.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexInitializer.java
<#1 (comment)>:
> +import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import org.geoserver.config.GeoServer;
+import org.geoserver.config.GeoServerDataDirectory;
+import org.geoserver.config.GeoServerInitializer;
+import org.geoserver.platform.GeoServerExtensions;
+import org.geoserver.platform.GeoServerResourceLoader;
+import org.geoserver.platform.resource.FileSystemResourceStore;
+import org.geoserver.platform.resource.Resource;
+import org.geoserver.platform.resource.ResourceListener;
+import org.geoserver.platform.resource.ResourceNotification;
The two imports above are never used.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> + resSet.getResourceFactoryRegistry().getExtensionToFactoryMap().put("feature",
+ new XMIResourceFactoryImpl());
+ }
+
+ public IndexOutputFormat(GeoServer gs) {
+ super(gs);
+ }
+
+ @OverRide
+ public void write(Object value, OutputStream output, Operation operation)
+ throws IOException, ServiceException {
+ // extract GetFeature request
+ GetFeatureType request = (GetFeatureType) OwsUtils.parameter(operation.getParameters(),
+ GetFeatureType.class);
+ // generate an UUID (resultSetID) for this request
+ resultSetId = UUID.randomUUID().toString().replaceAll("-", "");
GeoTools complained about the - in the ID 😛 ?
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> + /**
+ * Helper method that serialize GetFeature request, store it in the file system and associate it
+ * with resultSetId
+ *
+ * @param request
+ * @param resultSetId
+ * @throws Exception
+ */
+ protected void storeGetFeature(String resultSetId, GetFeatureType ft) throws RuntimeException {
+ try {
+ DataStore dataStore = IndexConfiguration.getCurrentDataStore();
+ // Create and store new feature
+ SimpleFeatureStore featureStore = (SimpleFeatureStore) dataStore
+ .getFeatureSource(IndexInitializer.STORE_SCHEMA_NAME);
+ SimpleFeatureBuilder builder = new SimpleFeatureBuilder(featureStore.getSchema());
+ Long now = new Date().getTime();
I would use System.currentTimeMillis(), unless I'm missing something they
give the same value, except this version is a faster and doesn't allocate
an unnecessary object.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> + *
+ * @param request
+ * @param resultSetId
+ * @throws Exception
+ */
+ protected void storeGetFeature(String resultSetId, GetFeatureType ft) throws RuntimeException {
+ try {
+ DataStore dataStore = IndexConfiguration.getCurrentDataStore();
+ // Create and store new feature
+ SimpleFeatureStore featureStore = (SimpleFeatureStore) dataStore
+ .getFeatureSource(IndexInitializer.STORE_SCHEMA_NAME);
+ SimpleFeatureBuilder builder = new SimpleFeatureBuilder(featureStore.getSchema());
+ Long now = new Date().getTime();
+ builder.add(resultSetId);
+ builder.add(now);
+ builder.add(now);
Duplicate invocation of builder.add(now).
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexOutputFormat.java
<#1 (comment)>:
> + DataStore dataStore = IndexConfiguration.getCurrentDataStore();
+ // Create and store new feature
+ SimpleFeatureStore featureStore = (SimpleFeatureStore) dataStore
+ .getFeatureSource(IndexInitializer.STORE_SCHEMA_NAME);
+ SimpleFeatureBuilder builder = new SimpleFeatureBuilder(featureStore.getSchema());
+ Long now = new Date().getTime();
+ builder.add(resultSetId);
+ builder.add(now);
+ builder.add(now);
+ SimpleFeature feature = builder.buildFeature(null);
+ SimpleFeatureCollection collection = new ListFeatureCollection(featureStore.getSchema(),
+ Arrays.asList(feature));
+ featureStore.addFeatures(collection);
+
+ // Create and store file
+ Resource storageResource = IndexConfiguration.getStorageResource();
It seems to be that this block of code and the IndexInitializer manage
methods should by synchronizer by a read-write lock. If a request comes and
we get the storage location but in the meantime the storage location is
changed, this request will be stored in the old path causing a very nasty
bug to found.
I think the best option here would be a ReadWriteLock shared by the
IndexConfiguration and used by the IndexInitializer and the
IndexOutputFormat. Any operation that needs to access the storage and
database should need a read lock (multiple threads will be able to access
it at the same time) but the operations in IndexInitializer will need to
obtain a write lock.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/IndexResultTypeDispatcherCallback.java
<#1 (comment)>:
> + * <p>
+ * A new entry named RESULT_TYPE_INDEX specifying that the original result type was "index" is added
+ * to KVP maps
+ * </p>
+ * <p>
+ * The object that manage response of type HitsOutputFormat is replaced with IndexOutputFormat
+ * before response has been dispatched
+ * </p>
+ *
+ * @author sandr
+ *
+ */
+
+public class IndexResultTypeDispatcherCallback extends AbstractDispatcherCallback {
+
+ static Logger LOGGER = Logging.getLogger(IndexResultTypeDispatcherCallback.class);
Should be private and final.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/PageResultsWebFeatureService.java
<#1 (comment)>:
> + * @throws IOException
+ * @throws Exception
+ */
+ private GetFeatureType getFeature(String resultSetID) throws IOException {
+ GetFeatureType feature = null;
+ Transaction transaction = new DefaultTransaction("Update");
+ try {
+ // Update GetFeature utilization
+ DataStore currentDataStore = IndexConfiguration.getCurrentDataStore();
+ SimpleFeatureStore store = (SimpleFeatureStore) currentDataStore
+ .getFeatureSource(IndexInitializer.STORE_SCHEMA_NAME);
+ store.setTransaction(transaction);
+ Filter filter = CQL.toFilter("ID = '" + resultSetID + "'");
+ store.modifyFeatures("updated", new Date().getTime(), filter);
+ // Retrieve GetFeature from file
+ Resource storageResource = IndexConfiguration.getStorageResource();
Same concurrent issue that when storing.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/PageResultsDispatcherCallback.java
<#1 (comment)>:
> +
+/**
+ *
+ * This dispatcher manages service of type ***@***.*** PageResultsWebFeatureService} and sets the
+ * parameter ResultSetID present on KVP map.
+ * <p>
+ * Dummy featureId value is added to KVP map to allow dispatcher to manage it as usual WFS 2.0
+ * request.
+ *
+ * @author sandr
+ *
+ */
+
+public class PageResultsDispatcherCallback extends AbstractDispatcherCallback {
+
+ static Logger LOGGER = Logging.getLogger(PageResultsDispatcherCallback.class);
Should be private and final.
------------------------------
In src/community/nsg-profile/src/main/java/org/geoserver/nsg/
pagination/random/PageResultsDispatcherCallback.java
<#1 (comment)>:
> + static Logger LOGGER = Logging.getLogger(PageResultsDispatcherCallback.class);
+
+ private GeoServer gs;
+
+ public PageResultsDispatcherCallback(GeoServer gs) {
+ this.gs = gs;
+ }
+
+ @SuppressWarnings("unchecked")
+ @OverRide
+ public Service serviceDispatched(Request request, Service service) throws ServiceException {
+ if (service.getService() instanceof PageResultsWebFeatureService) {
+ PageResultsWebFeatureService prService = (PageResultsWebFeatureService) service
+ .getService();
+ prService.setResultSetID((String) request.getKvp().get("resultSetID"));
+ request.getKvp().put("featureId", Arrays.asList("dummy"));
I know this is a hack that may be removed ... in this cases is better use
Collections.singletonList("dummy").
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADlvpsCldxKjwcAxCBF-Bs0WG4wl_e2tks5seJIggaJpZM4O-s7v>
.
--
Dott. Ing. Sandro Salari
|
|
Hi Sandro, I'm working on the EMF issue, I manage to fix the issue you are experiencing but then it fails on the deserialization of a complex query. EMF (what else expecting from this) will probably not do, I'm working on an alternative solution. I will provide you the code that serializes the query and gets is result back. |
- configuration file parser and change listerner - db storage - file storage - clean task
… into ogc-nsg-profile
Implemented final response of PageResult operation
nmco
pushed a commit
that referenced
this pull request
Nov 22, 2018
nmco
pushed a commit
that referenced
this pull request
May 9, 2020
WIP: implementation of the HTML pages for GSR services using freemark…
nmco
pushed a commit
that referenced
this pull request
Jan 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.