neo4j-contrib / neo4j-guides

Tooling to create Neo4j Browser Guides from AsciiDoc Content
33 stars 30 forks source link

Fix broken getParams call for Neo4j >= 3.2 #15

Closed dhimmel closed 7 years ago

dhimmel commented 7 years ago

Closes https://github.com/neo4j-contrib/neo4j-guides/issues/14.

Uses solution from https://github.com/neo4j-contrib/neo4j-apoc-procedures/commit/4dcf50ad2e6ae2f4b4fb0eb6bf9cd1f2ca80d185

After merging, I'd recommend creating a new release that uploads neo4j-guide-extension-3.2.3.jar.

spacecowboy commented 7 years ago

@dhimmel have you actually tested this? I am not keen on your approach because it's not supposed to work.

The config should be throwing the value of that setting away during parsing because it is not a defined setting in Neo4j. Only official settings are kept, as well as those prefixed by apoc.*.

You should see this during startup of neo4j with a warning if you have it defined in you config file:

Unknown config option: org.neo4j.server.guide.directory

The correct approach is

1) ~setting should be prefixed by apoc.~ 2) add a setting class available to the config parser so the config is can properly validate the field

EDIT: Ah went looking in the source, the setting is actually not thrown away.

But you do get the warning during startup. And in case a user enables strict_validation then neo4j will refuse to start.

Relevant source: https://github.com/neo4j/neo4j/blob/8b1656eba53c1d97c7a1808ee63cc9625d8cc72f/community/kernel/src/main/java/org/neo4j/kernel/configuration/IndividualSettingsValidator.java#L70

dhimmel commented 7 years ago

In Neo4j 3.2.3 (latest Docker version available, currently running at https://neo4j.het.io/browser/), it doesn't appear that I'm seeing Unknown config option. Here's the entirety of the startup messages:

Not retrieving database as it already exists
Downloading and extracting guides
Active database: graph.db
Directories in use:
  home:         /var/lib/neo4j
  config:       /var/lib/neo4j/conf
  logs:         /logs
  plugins:      /var/lib/neo4j/plugins
  import:       /var/lib/neo4j/import
  data:         /var/lib/neo4j/data
  certificates: /ssl
  run:          /var/lib/neo4j/run
Starting Neo4j.
2017-09-18 21:27:28.397+0000 INFO  ======== Neo4j 3.2.3 ========
2017-09-18 21:27:28.494+0000 INFO  Starting...
2017-09-18 21:27:30.642+0000 INFO  Bolt enabled on 0.0.0.0:7687.
2017-09-18 21:27:30.679+0000 INFO  Initiating metrics...
2017-09-18 21:27:38.023+0000 INFO  Started.
2017-09-18 21:27:38.361+0000 INFO  Mounted REST API at: /db/manage
2017-09-18 21:27:38.382+0000 INFO  Mounted unmanaged extension [extension.web] at [/guides]
2017-09-18 21:27:39.498+0000 WARN  The following warnings have been detected with resource and/or provider classes:
  WARNING: A sub-resource method, public javax.ws.rs.core.Response extension.web.StaticWebResource.index() throws java.io.IOException, with URI template, "/", is treated as a resource method
2017-09-18 21:27:40.284+0000 INFO  Remote interface available at http://localhost:7474/

It's worth noting that it appears that if the setting were thrown out, it would default to guides, which would work for my application:

https://github.com/neo4j-contrib/neo4j-guides/blob/47bb04584b88ae5302198dcc9b40418f6d506c7a/guide-extension/src/main/java/extension/web/StaticWebResource.java#L30-L31

add a setting class available to the config parser so the config is can properly validate the field

@spacecowboy thanks for the help! I'm not going to pursue a more elegant solution (I'm not great with Java). Perhaps it make sense to adopt this as a temporary workaround... I'm not sure anyone is maintaining this repository anymore ):