Skip to content
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

MODE-2339: Inject repository connector via <context-param> element #1307

Merged
merged 1 commit into from Nov 7, 2014

Conversation

okulikov
Copy link
Member

This PR implements approach for loading repositories:

  • should specify container specific implementation of the Connector class;
  • if is unspecified then default WF-compatible Connector will be loaded.

@hchiorean
Copy link
Member

@okulikov: this change is fine, but it still doesn't allow users to easily specify the JNDI name.
Using different connector implementations is fine, but then IMO we need to at least:
a) have a connector which is able to load repository JSON files (i.e. single repositories)
b) allow the JNDI name to be configurable - not hardcoded to jndi:jcr like in the current ConnectorImpl

@okulikov
Copy link
Member Author

@hchiorean , regarding b)
do you mean to extend the current, 'default' connector with configuration properties?

@hchiorean
Copy link
Member

@okulikov: yes, where the configuration comes from the web.xml in some way (maybe another <context-param>) The reason is that the web.xml is the main/only file users should have to overlay/edit for their own applications.

@okulikov
Copy link
Member Author

Next commit provides:

  • ability to configure jndi name where to look up engine;
  • another connector implementation which is able to deploy repository from json configurarion file

@hchiorean
Copy link
Member

looks good.
@okulikov can you please also update the modeshape-web-explorer-war module and test the JSON file connector ? (most likely adding some test resources - i.e. a repository JSON file & a test web.xml which contains the configuration)
Not sure if it's possible but ideally the ExplorerTest test case would then validate that this repository is accessible by accessing the repository "URL" (if there is such a thing, I'm thinking something along the lines of http://localhost:8090/modeshape-explorer/<repo>).
We also need to document these configuration options, but I'm not sure what the best approach is (we don't have any explorer documentation as of yet).
@rhauch thoughts ?

@rhauch
Copy link
Contributor

rhauch commented Oct 28, 2014

@hchiorean, perhaps a new page for the Explorer under https://docs.jboss.org/author/display/MODE40/ModeShape+Guide?

@hchiorean
Copy link
Member

@rhauch maybe a new page under https://docs.jboss.org/author/display/MODE40/Using+ModeShape ? (which contains the REST & WebDAV pages)

@rhauch
Copy link
Contributor

rhauch commented Oct 28, 2014

@hchiorean, sure, that's fine.

@hchiorean
Copy link
Member

@okulikov: can you pls add this and document the configuration aspects once this PR is merged

@okulikov
Copy link
Member Author

Pushed small commit with fix for several errors. Test case still a bit an issue due to ajax nature of application. It always returns same page with reference to java script.

@hchiorean
Copy link
Member

@okulikov see the above comments
For the test if there's no URL we can use to actually test the fact that the repository is accessible, I'm fine with leaving that as-is; i.e. unchanged

@okulikov
Copy link
Member Author

@hchiorean pushed commit with transient fields. missed it.

@okulikov
Copy link
Member Author

@hchiorean , regarding test I am trying to find something what we can catch to decide is it right test or failed.

@hchiorean
Copy link
Member

@okulikov after finishing updating the test, pls squash the commits & update this PR. thanks

@hchiorean
Copy link
Member

@okulikov any update on this ? As mentioned above, if finding a "static" link to test the repository is too difficult (thanks to GWT..) this PR is fine as is (just needs squashing)

@okulikov
Copy link
Member Author

okulikov commented Nov 7, 2014

@hchiorean , one more try to catch something for check inside java script and then I will squash either with test or as is.

@okulikov
Copy link
Member Author

okulikov commented Nov 7, 2014

@hchiorean , just squashed commits

hchiorean pushed a commit that referenced this pull request Nov 7, 2014
MODE-2339: Inject repository connector via <context-param> element
@hchiorean hchiorean merged commit 225286d into ModeShape:master Nov 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants