Page Comparison

[07:45:41 EDT(-0400)] * michelled (n=team@142.150.154.197) has joined ##uportal
[09:20:33 EDT(-0400)] * awills (n=awills@mtw160-5.ippl.jhu.edu) has joined ##uportal
[09:40:14 EDT(-0400)] * dstn (n=dstn@unaffiliated/dstn) has joined ##uportal
[09:40:36 EDT(-0400)] * dstn (n=dstn@unaffiliated/dstn) has left ##uportal
[09:42:31 EDT(-0400)] * michelled (n=team@142.150.154.197) has left ##uportal
[10:02:16 EDT(-0400)] * colinclark (n=colin@bas1-toronto09-1279543604.dsl.bell.ca) has joined ##uportal
[10:08:35 EDT(-0400)] * MarkRogers (n=MarkRoge@addhcp129.cc.umanitoba.ca) has joined ##uportal
[10:17:50 EDT(-0400)] * EricDalquist (n=dalquist@bohemia.doit.wisc.edu) has joined ##uportal
[10:35:55 EDT(-0400)] <MarkRogers> i may have asked this before, but what is the next frontier for hibernate in relation to uPortal?
[10:37:10 EDT(-0400)] <EricDalquist> we plan on using hibernate for all new and refactoried DAOs
[10:41:10 EDT(-0400)] <MarkRogers> excellent
[10:41:44 EDT(-0400)] <EricDalquist> more specifically JPA via annotations with Hibernate as the underlying ORM provider.
[10:51:26 EDT(-0400)] <MarkRogers> do you make a lot of use of annotations already?
[10:59:21 EDT(-0400)] <awills> just posted that patch we talked about yesterday eric
[10:59:47 EDT(-0400)] <EricDalquist> the channel-type?
[11:00:29 EDT(-0400)] * holdorph (n=holdorph@12.164.139.7) has joined ##uportal
[11:01:12 EDT(-0400)] <awills> yep: http://www.ja-sig.org/issues/browse/UP-2097
[11:02:02 EDT(-0400)] <EricDalquist> looks good
[11:02:11 EDT(-0400)] <EricDalquist> feel free to commit it to trunk and merge it to the patches branch
[11:02:31 EDT(-0400)] <awills> good deal
[11:14:40 EDT(-0400)] <awills> done
[11:20:17 EDT(-0400)] * apetro-_ (n=apetro@ip68-98-37-188.ph.ph.cox.net) has joined ##uportal
[11:24:15 EDT(-0400)] * holdorph (n=holdorph@wsip-98-174-242-39.ph.ph.cox.net) has joined ##uportal
[11:33:17 EDT(-0400)] * michelled (n=team@142.150.154.197) has joined ##uportal
[12:48:06 EDT(-0400)] * colinclark (n=colin@142.150.154.101) has joined ##uportal
[12:48:53 EDT(-0400)] * colinclark (n=colin@142.150.154.101) has joined ##uportal
[12:57:40 EDT(-0400)] * apetro-_ (n=apetro@wsip-98-174-242-39.ph.ph.cox.net) has joined ##uportal
[14:21:54 EDT(-0400)] * apetro-- (n=apetro@12.164.139.7) has joined ##uportal
[14:59:46 EDT(-0400)] * apetro-_ (n=apetro@wsip-98-174-242-39.ph.ph.cox.net) has joined ##uportal
[15:43:25 EDT(-0400)] <awills> still there EricDalquist?
[15:43:40 EDT(-0400)] <EricDalquist> yup
[15:44:16 EDT(-0400)] <awills> how do we feel about contributing pathname=""? I think others will run into this issue too
[15:44:38 EDT(-0400)] <EricDalquist> they shouldn't
[15:44:44 EDT(-0400)] <EricDalquist> not in the default config
[15:44:52 EDT(-0400)] <EricDalquist> in the default uP3 config the session is serializable
[15:44:55 EDT(-0400)] <EricDalquist> and replicatable
[15:45:05 EDT(-0400)] <awills> but no one deploys their portal to prod with the default config
[15:45:14 EDT(-0400)] <EricDalquist> well, with the default code
[15:45:24 EDT(-0400)] <EricDalquist> it only is a problem if you're sticking 'dangerous' stuff in the session
[15:45:30 EDT(-0400)] <EricDalquist> documentation is probably good
[15:45:47 EDT(-0400)] <EricDalquist> but disabling a feature that works unless you change code I don't think is appropriate
[15:46:52 EDT(-0400)] <awills> this isn't quite adding up... we didn't stick anything in the session
[15:47:11 EDT(-0400)] <EricDalquist> you changed the behavior of an object that is put in the session
[15:47:20 EDT(-0400)] <awills> I think it was trying to reconstitute the IPerson
[15:47:23 EDT(-0400)] <EricDalquist> yes
[15:47:33 EDT(-0400)] <EricDalquist> but it was reconsistuing your IPerson impl
[15:47:35 EDT(-0400)] <EricDalquist> not uPortals
[15:47:49 EDT(-0400)] <EricDalquist> uPortal's IPerson reconstitutes just fine
[15:47:56 EDT(-0400)] <awills> I bet you could get to the same error just by changing context files
[15:48:15 EDT(-0400)] <EricDalquist> sure, if you write a new IPerson
[15:48:24 EDT(-0400)] <EricDalquist> that has to have access to the spring context for toString
[15:48:28 EDT(-0400)] <EricDalquist> and change the person factory
[15:48:37 EDT(-0400)] <EricDalquist> to return that new IPerson class
[15:49:12 EDT(-0400)] <EricDalquist> but your new IPerson class isn't really following the Serializable rules and the IPerson class is marked Serializable
[15:49:30 EDT(-0400)] <EricDalquist> not a big deal if you don't care
[15:49:44 EDT(-0400)] <EricDalquist> but the default config assumes that IPerson is serializable as marked
[15:52:34 EDT(-0400)] <awills> hmmm... it was calling isGuest()... i and I don't think the enhanced PersonImpl was answering that question in an outlandish, serializale-unfriendly way
[15:53:48 EDT(-0400)] <EricDalquist> so from the stack you pastebin'd: http://uportal.pastebin.com/m34da0af1
[15:54:00 EDT(-0400)] <EricDalquist> it looks like Tomcat does a toString on objects when it is deserializing them
[15:54:12 EDT(-0400)] <EricDalquist> IPerson.toString calls isGuest
[15:54:13 EDT(-0400)] <EricDalquist> :/
[15:54:17 EDT(-0400)] <awills> yep
[15:54:27 EDT(-0400)] <EricDalquist> which isn't appropriate in the JHU impl
[15:54:39 EDT(-0400)] <EricDalquist> since this happens before the app context is up and running
[15:54:57 EDT(-0400)] <awills> which would work perfectly fine, if tomcat had brought the context up before trying to re-hydrate the session
[15:55:28 EDT(-0400)] <EricDalquist> you could in your JHU impl call PortalApplicationContextLocator.isRunningInWebApplication() and take action based on that result
[15:55:41 EDT(-0400)] <EricDalquist> or you could calculate the value of isGuest in the constructor
[15:55:44 EDT(-0400)] <EricDalquist> and store it as a field
[15:56:02 EDT(-0400)] <EricDalquist> instead of dynamically calculating it every call (which seems like a reasonable performance fix anyways)
[15:57:11 EDT(-0400)] <awills> i don't think it's very natural to try to rehydrate an object that belongs within an app before starting that app... and i suspect we could see this problem again w/o chaanges to uP code
[15:57:53 EDT(-0400)] <EricDalquist> well that is an issue with Tomcat
[15:58:01 EDT(-0400)] <EricDalquist> and may be dictated by the servlet spec
[15:58:03 EDT(-0400)] <EricDalquist> but I don't know
[15:59:01 EDT(-0400)] <awills> idk either... but I do know this was kind of a thorny one to troubleshoot
[15:59:30 EDT(-0400)] <EricDalquist> that it is
[16:00:31 EDT(-0400)] <EricDalquist> that seems much more reasonable
[16:00:43 EDT(-0400)] <EricDalquist> or even better
[16:00:50 EDT(-0400)] <EricDalquist> just pass that value in from the person manager
[16:01:18 EDT(-0400)] <EricDalquist> which I need to document better because it should really only ever be used from command line tools
[16:01:28 EDT(-0400)] <EricDalquist> in a running webapp the goal is everything is injected
[16:01:44 EDT(-0400)] <EricDalquist> and your person manager is a spring managed bean
[16:02:38 EDT(-0400)] <awills> what about this enhancement then... change the error message code so that it will log once, not 32657236572 times (as it does now): http://uportal.pastebin.com/m23330492
[16:03:34 EDT(-0400)] <EricDalquist> that sounds like a good approach
[16:03:57 EDT(-0400)] <awills> yeah it should be wired by the bean container... we were trying to bring my.johnhopkins up on uP3 with the minimum changes to existing tech
[16:04:02 EDT(-0400)] <EricDalquist> perhaps even include a "new Throwable()" as the second arg of that log statement so you have a stack trace to find what caused it
[16:04:12 EDT(-0400)] <EricDalquist> understandable
[16:04:16 EDT(-0400)] <awills> which we did btw... it's 95% there
[16:04:22 EDT(-0400)] <EricDalquist> I would put that change to their IPerson as a required change though
[16:08:55 EDT(-0400)] <awills> Changing JHUPersonFactory to have the context injected wouldn't have prevented this issue, though it might have made it easier to analyze... changing PersonImpl to store an isGuest flag would have prevented it, but it won't prevent the next time this comes up (and I'm still pretty sure it will)
[16:09:48 EDT(-0400)] <EricDalquist> well the change in JHUPErsonFactory wouldn't be to make in ApplicationContextAware it would be to inject whatever resource the person impl needed from the ApplicationContext
[16:09:53 EDT(-0400)] <EricDalquist> it may well come up again
[16:10:01 EDT(-0400)] <EricDalquist> but I think the point here is that is broken code
[16:10:07 EDT(-0400)] <EricDalquist> and the fix needs to be in that code
[16:10:17 EDT(-0400)] <EricDalquist> not a work around to allow broken code to run
[16:11:55 EDT(-0400)] <awills> From my perspective, the broken code is in Tomcat (unless the behavior's dictated by the spec, but i'd be surprised)...(12:57:53 PM) EricDalquist: well that is an issue with Tomcat... and may be dictated by the servlet spec
[16:12:22 EDT(-0400)] <EricDalquist> true, but what if uPortal was a 'pure' Spring app
[16:12:48 EDT(-0400)] <EricDalquist> the only way to access the ApplicationContext would be if you already had a reference to the current ServletContext
[16:13:01 EDT(-0400)] <EricDalquist> so that code in your person impl would not be possible
[16:13:09 EDT(-0400)] <EricDalquist> that situation is the end goal
[16:13:26 EDT(-0400)] <EricDalquist> to allow legacy code access to the ApplicationContext
[16:13:30 EDT(-0400)] <awills> true... I should make the change to PersonFactory... and then I'd be left with an NPE, which (ironically) would be easier to diagnose
[16:13:33 EDT(-0400)] <EricDalquist> but it has its catches
[16:13:50 EDT(-0400)] <EricDalquist> how would you get a NPE?
[16:14:27 EDT(-0400)] <EricDalquist> my first try would be to:
[16:14:40 EDT(-0400)] <awills> when tomcat tried to rehydrate the IPerson it would call isGuest, which would try to access a Map which isn't wired in yet
[16:14:48 EDT(-0400)] <EricDalquist> when you create a new PersonImpl use that injected object to perform the isGuest calculation
[16:14:55 EDT(-0400)] <EricDalquist> and store the results of that calculation
[16:15:15 EDT(-0400)] <EricDalquist> unless you have a case where an IPerson's isGuest value can change over time?
[16:15:48 EDT(-0400)] <awills> ah, but that 2nd part ("store the results of that calculation") isn't "broken code" exactly
[16:16:13 EDT(-0400)] <awills> i shouldn't think it could change over time
[16:16:33 EDT(-0400)] <EricDalquist> well that is up to your use case
[16:16:35 EDT(-0400)] <EricDalquist> if it can;t
[16:16:48 EDT(-0400)] <EricDalquist> then why bother wasting cycles to re-calculate it for a method that gets called a lot
[16:17:15 EDT(-0400)] <EricDalquist> if it can, then there needs to be some extra consideration put into how that is done in reference to that object being serializable
[16:20:27 EDT(-0400)] <awills> there's no issue w/ storing the result of the calculation, usless you want to point out that the delta on the codebase is larger (not by much... no big deal)... i just think it's a bad idea to rehydrate portal objects before the app context is brought up b/c i think we'll see it again, and i believe i could reproduce it using only configuration, no code changes
[16:21:04 EDT(-0400)] * dstn (n=dstn@unaffiliated/dstn) has joined ##uportal
[16:21:08 EDT(-0400)] <EricDalquist> that would be interesting to see
[16:21:13 EDT(-0400)] <EricDalquist> part of this is cleaning up uPortal too
[16:21:22 EDT(-0400)] <awills> if i encounter it, i'll let you know
[16:21:25 EDT(-0400)] <EricDalquist> the expectation is that serializable objects in the session are just data objects
[16:21:35 EDT(-0400)] <EricDalquist> with no references to runtime code in the webapp
[16:21:41 EDT(-0400)] <EricDalquist> that's a requirement if you want to be able to replicate the session
[16:21:46 EDT(-0400)] <EricDalquist> uPortal is bad about this in many cases
[16:21:55 EDT(-0400)] <EricDalquist> and stores objects with references to daos and such in the session
[16:21:58 EDT(-0400)] <EricDalquist> or it used to at least
[16:22:04 EDT(-0400)] <EricDalquist> but they can creep in
[16:22:06 EDT(-0400)] <EricDalquist> like we've seen here
[16:24:13 EDT(-0400)] <awills> do you mean "runtime code" or "runtime objects" here?
[16:24:37 EDT(-0400)] <EricDalquist> I guess I mean services ... if that helps at all
[16:24:56 EDT(-0400)] <EricDalquist> objects in the session should not have references to spring beans, daos, static services, etc
[16:25:01 EDT(-0400)] <EricDalquist> if they do they cannot be replicated
[16:25:24 EDT(-0400)] <EricDalquist> and 'references' here means direct, or indirect like what you ran into
[16:25:34 EDT(-0400)] <EricDalquist> where there was no direct reference, so serialization worked
[16:25:46 EDT(-0400)] <EricDalquist> but the session object attempted to access a static service
[16:26:00 EDT(-0400)] <awills> in case it's unclear... the use of this service in this case static... JHUPersonFactory.isGuest(myUserName)
[16:26:15 EDT(-0400)] <EricDalquist> if we ever want uPortal to really become stable in clustered environments we have to move to where everything in the session is just a data object
[16:26:19 EDT(-0400)] <EricDalquist> with zero behavior
[16:26:27 EDT(-0400)] <EricDalquist> yup, but that is behavior
[16:26:36 EDT(-0400)] <EricDalquist> which we shouldn't have in our data objects
[16:26:44 EDT(-0400)] <EricDalquist> because we get issues like this
[16:26:58 EDT(-0400)] <EricDalquist> so if I was re-designing this from scratch
[16:27:13 EDT(-0400)] <EricDalquist> and I was thinking about an isGuest property on an IPerson
[16:27:16 EDT(-0400)] <awills> i'm surprised to read that... i should think this type of call would work just fine in a clustered environment
[16:27:35 EDT(-0400)] <EricDalquist> in theory it should if the static service is always available
[16:27:54 EDT(-0400)] <EricDalquist> but when you get into serialization of objects it is still dangerous
[16:28:11 EDT(-0400)] <EricDalquist> if isGuest may be dynamic I would not
[16:28:17 EDT(-0400)] <EricDalquist> and I would implement a service interface
[16:28:28 EDT(-0400)] <EricDalquist> boolean IPersonInfo.isGuest(IPerson)
[16:28:38 EDT(-0400)] <EricDalquist> that would keep IPerson a simple data object
[16:28:48 EDT(-0400)] <EricDalquist> and move the behavior into a stateless service API
[16:29:02 EDT(-0400)] <awills> yeah... and is it a requirement of "best practice" serialization that your objects can have toString invoked on them w/o behavior? i should think you'd be concatenating chunks of info at least
[16:29:35 EDT(-0400)] <EricDalquist> probably best practice that toString, hashCode and equals will always work
[16:29:42 EDT(-0400)] <EricDalquist> since those can be called by anything
[16:29:51 EDT(-0400)] <EricDalquist> as they're part of Object
[16:30:16 EDT(-0400)] <EricDalquist> I've run into my share of bad objects with unexpected hashCode/equals behavior
[16:30:22 EDT(-0400)] <EricDalquist> that breaks in a Collection or Map
[16:32:17 EDT(-0400)] <awills> i got to run for a bit... and I guess i've done enough to explain my perspective that the fault for this lies w/ tomcat... so it's totally cool just to agree to disagree on this minor and subtle point
[16:32:34 EDT(-0400)] <awills> yeah hashCode/equals mismatches are hard to debug too
[16:41:13 EDT(-0400)] <holdorph> I'm obviously not going to sway the decision Drew/JHU has to make here, but.... I have to agree with Eric, I don't believe anything that's marked as "Serializable" should be dependent on on non-serializable things.
[16:41:28 EDT(-0400)] <holdorph> if they are, there is a java keyword to use for that, called "transient"
[16:41:45 EDT(-0400)] <EricDalquist> well the case here wasn't a direct dependency
[16:41:53 EDT(-0400)] <EricDalquist> it was a serializable object that calls a static service
[16:41:55 EDT(-0400)] <holdorph> and if you have a transient field, the rest of your code, should be smart enough to know how to handle what to do when the transient field is not available.
[16:42:09 EDT(-0400)] <EricDalquist> and the object was de-serialized before that static service was ready
[16:42:44 EDT(-0400)] <holdorph> static services that have to be 'initialized' are fairly evil themselves in concept then.
[16:43:26 EDT(-0400)] <EricDalquist> yup
[16:43:42 EDT(-0400)] <EricDalquist> this is the problem that is the PortalApplicationContextLocatoir
[16:44:11 EDT(-0400)] <EricDalquist> in a perfect uPortal that class would not exist
[16:44:22 EDT(-0400)] <EricDalquist> maybe we can get rid of it for 3.5 or something
[17:02:46 EDT(-0400)] * colinclark (n=colin@142.150.154.101) has joined ##uportal