Hi Amila, Thank you for going through the code and pointing out things so that I can improve myself. I re-factored the code and still few more things to do. I will attach a patch soon.
On Sun, Nov 21, 2010 at 11:07 AM, Amila Suriarachchi <[email protected]> wrote: > hi Manjula, > > I went through your code. You have done the overall design correctly but > there are some problem with the logic. > > if you go through the amazon message queue document you see that the return > url for the > created queue looks like this, > > http://sqs.us-east-1.amazonaws.com/123456789012/queue2 > > this numeric number represents the AWS number and hence user name. > therefore two users can have the same queue name. > > so we need to add the username part to message queue as well. > > In order to do that we need to pass the composite message box name to the > osgi service. > eg. user1/queu1. > > at the user store we can use this to keep the messages boxes since now > message box name is unique. > > the return address of the queue should always be to MessageQueue since > users send to this address in order to > do the operations. > > I saw some private variables like (userid, messageBoxOwner) > kept in InMemoryMessageBoxService. Please remove them. You can keep > private variables only if they are part of the object attribute. > yes, I kept those private variables to keep logged in user and messagebox owner, now they have been changed. > > public boolean isAccessible(String messageBoxName, String operation) { > String loggedInUser = getLoggedInUser(); > Map<String, MessageBox> messageBoxMap = > messageBoxMapStore.get(loggedInUser); > if (isAdminLoggedIn()) { > return true; > } > // if user owns messageBoxName, enable direct access to it. > if (messageBoxMap != null && messageBoxMap.get(messageBoxName) != > null) { > messageBoxOwner = loggedInUser; > return true; > } else { > if (accessControllerMapStore.get(messageBoxName) == null) { > return false; > } else { > Set<String> permissionLabels = > accessControllerMapStore.get(messageBoxName).keySet(); > for (String permissionLabel : permissionLabels) { > AccessController accessController = > accessControllerMapStore.get(messageBoxName).get(permissionLabel); > if (accessController.isAccessible(loggedInUser, > operation)) { > messageBoxOwner = > accessController.getMessageBoxOwner(); > return true; > } > } > return false; > } > } > } > > this logic is wrong with the current implementation. Actually this is why > you need to have user name in the message box name. > > lets take the senario where a uesr2 wants to send a message to queue queue1 > created by user2. And user2 also have a queue called > queue1. > yes, this logic fails here with above scenario :( I understand that composite message box name needed here. I have taken the composite message box name and changed the code. > > it authorize user just checking the availability of his queue and finally > receive the message from that as well. > > Keep all the access control details in the Message Box as well. > I changed this as well. > > And also put more comments as well. > I will put more comments properly. > > > thanks, > Amila. > _______________________________________________ > Carbon-dev mailing list > [email protected] > https://wso2.org/cgi-bin/mailman/listinfo/carbon-dev > > thanks -- Manjula Rathnayaka Software Engineer WSO2, Inc. Mobile:+94 77 743 1987
_______________________________________________ Carbon-dev mailing list [email protected] https://wso2.org/cgi-bin/mailman/listinfo/carbon-dev
