Hi All, Please find below a list of improvements identified during the code review.
- buildSAMLResponse() method is getting called even if its not a SAML response, because to determine if its a SAML response we need to build the message, which is expensive. - getResult() method checks if the request is a logout request for every request. This too is expensive. - The way SAML Authentication request has been constructed is different from the way logout has been done. Approach needs to be consistent. - When checking the subscription, for each call, we do a DB query to check if subscription is active. This needs to be cached, once retrieved. Since there are only two states this check can be done in the db query itself. - Cache key entries will need to be standardized, use unique component based names to avoid possible conflicts. - When sending SAML response to backend, we should read the property once and cache it for future references. - Add App Manager Exceptions instead of API Manager Exceptions - Check if debug is enabled first before printing debug logs. - Comments overall and formatting improvements & Remove printed stack traces from exceptions Thanks and Regards, Ruwan Yatawara Senior Software Engineer, WSO2 Inc. email : [email protected] mobile : +94 77 9110413 blog : http://thoughts.ruwan-ace.com/ www: :http://wso2.com On Wed, Jul 2, 2014 at 7:34 PM, Dinusha Senanayaka <[email protected]> wrote: > more details » > <https://www.google.com/calendar/event?action=VIEW&eid=ZXBndW5obnFtaGs3ZWFiaWVlYjMzZzM3a2cgcnV3YW55QHdzbzIuY29t&tok=MTYjZGludXNoYUB3c28yLmNvbWM3M2VmZjZhMmQyN2NjMzY5M2QwOGY4NTY2YWE3ZjMwN2Y4MTIzM2Y&ctz=Asia/Colombo&hl=en> > App Manager SAML2AuthenticationHandler code review > *When* > Thu Jul 3, 2014 3pm – 3:30pm Colombo > *Video call* > https://plus.google.com/hangouts/_/wso2.com/dinusha > <https://plus.google.com/hangouts/_/wso2.com/dinusha?hceid=ZGludXNoYUB3c28yLmNvbQ.epgunhnqmhk7eabieeb33g37kg> > *Calendar* > [email protected] > *Who* > • > Dinusha Senanayaka - organizer > • > Kasun Dissanayake > • > Rajeeva Uthayasangar > • > Sumedha Rubasinghe > • > Rushmin Fernando > • > Ruwan Yatawara > • > Amila De Silva > • > Johann Nallathamby > • > Nuwan Dias > > Going? *Yes > <https://www.google.com/calendar/event?action=RESPOND&eid=ZXBndW5obnFtaGs3ZWFiaWVlYjMzZzM3a2cgcnV3YW55QHdzbzIuY29t&rst=1&tok=MTYjZGludXNoYUB3c28yLmNvbWM3M2VmZjZhMmQyN2NjMzY5M2QwOGY4NTY2YWE3ZjMwN2Y4MTIzM2Y&ctz=Asia/Colombo&hl=en> > - Maybe > <https://www.google.com/calendar/event?action=RESPOND&eid=ZXBndW5obnFtaGs3ZWFiaWVlYjMzZzM3a2cgcnV3YW55QHdzbzIuY29t&rst=3&tok=MTYjZGludXNoYUB3c28yLmNvbWM3M2VmZjZhMmQyN2NjMzY5M2QwOGY4NTY2YWE3ZjMwN2Y4MTIzM2Y&ctz=Asia/Colombo&hl=en> > - No > <https://www.google.com/calendar/event?action=RESPOND&eid=ZXBndW5obnFtaGs3ZWFiaWVlYjMzZzM3a2cgcnV3YW55QHdzbzIuY29t&rst=2&tok=MTYjZGludXNoYUB3c28yLmNvbWM3M2VmZjZhMmQyN2NjMzY5M2QwOGY4NTY2YWE3ZjMwN2Y4MTIzM2Y&ctz=Asia/Colombo&hl=en>* > more options » > <https://www.google.com/calendar/event?action=VIEW&eid=ZXBndW5obnFtaGs3ZWFiaWVlYjMzZzM3a2cgcnV3YW55QHdzbzIuY29t&tok=MTYjZGludXNoYUB3c28yLmNvbWM3M2VmZjZhMmQyN2NjMzY5M2QwOGY4NTY2YWE3ZjMwN2Y4MTIzM2Y&ctz=Asia/Colombo&hl=en> > > Invitation from Google Calendar <https://www.google.com/calendar/> > > You are receiving this email at the account [email protected] because you > are subscribed for invitations on calendar [email protected]. > > To stop receiving these notifications, please log in to > https://www.google.com/calendar/ and change your notification settings > for this calendar. >
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
