Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

[NEXUS-4613] guice servlet #158

Merged
merged 2 commits into from Dec 27, 2011
Merged

[NEXUS-4613] guice servlet #158

merged 2 commits into from Dec 27, 2011

Conversation

adreghiciu
Copy link
Contributor

No description provided.

@jdillon
Copy link
Contributor

jdillon commented Dec 19, 2011

Needs some javadoc on the key bits to explain how this works. Tested plugin installing a custom ServletModule, works great.

+1

@cstamas
Copy link
Contributor

cstamas commented Dec 19, 2011

+1 if this works for you JD and you need it ASAP
The reason why I "refreshed" (did a rebase + those POM fixes that were missing) on Friday is that I wanted to do a bit more, by dumping all the security related stuff out of Core too...

@jdillon
Copy link
Contributor

jdillon commented Dec 19, 2011

I don't "need" it ASAP, I "want" it ASAP. I want to see this get in as soon as possible though, this opens the door for lots of new and cool things...

Don't follow how "dumping all the security related stuff out of Core" is related to this change though.

@cstamas
Copy link
Contributor

cstamas commented Dec 20, 2011

Hm, if someone does not "need" something but "wants" it, isn't that considered luxury? :D

Wrt security: the core WAR's web.xml contains littered Shiro config stuff in place. This should be moved out to security module (best would be together with all security related filters and so on). The fact they are in web.xml makes the WAR non-workable on some containers, as currently web.xml relies on some unspec'd features (to preserve line endings which they actually should not by servlet spec). This works in Tomcat and Jetty, but for example breaks on Oracle and IBM containers (AFAIK, i am maybe wrong about one of those two). I guess by having these config lines in module instead, would make the WAR workable on all containers as the web.xml would be "clean" by the specs.

This is +1 from me, the security part is not considered a blocker by me, but a "nice to have". And this being in master, the security cleanup could happen easily.

@bdemers
Copy link
Contributor

bdemers commented Dec 21, 2011

Some of the problems with security and are web.xml are related to the order they are initialized (if i remember correctly), some containers will initialize filters before listeners. ( the PlexusContainerContextListener needs to be init before the NexusJSecurityFilter)

Either way switching to guice servlet will get us one step closer to ripping out the ini config of the security filter, it would just be injected. (hopefully a switch to shiro guice should remove a bunch of other plexus work arounds as well)

For example our WebRealmSecurityManager class is only used to inject and configure which session manager to use.
+1

@cstamas
Copy link
Contributor

cstamas commented Dec 22, 2011

Started new CI build since in previous both slaves failed with "default error reporter NPE", problem present in master:
https://builds.sonatype.org/view/nexus/job/nexus-oss-its-feature/133/

@cstamas
Copy link
Contributor

cstamas commented Dec 23, 2011

CI +1 (on win, the "error reporter NPE" still, coming from master):
https://builds.sonatype.org/view/nexus/job/nexus-oss-its-feature/134/

cstamas added a commit that referenced this pull request Dec 27, 2011
@cstamas cstamas merged commit 0aeb37d into master Dec 27, 2011
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants