Skip to content

Issues/247#248

Merged
lloydwatkin merged 16 commits intobuddycloud:masterfrom
garethf:issues/247
Oct 10, 2014
Merged

Issues/247#248
lloydwatkin merged 16 commits intobuddycloud:masterfrom
garethf:issues/247

Conversation

@garethf
Copy link
Copy Markdown
Contributor

@garethf garethf commented Oct 9, 2014

Findbugs reporting now integrated and build failing on high priority issues. All of the high priority issues that have been reported have been removed, and a couple of additional tests added. Lots of minor style/convention issues (equals method, string constants, literal booleans in conditionals and other bits and pieces). It probably went further than I wanted it to.

@imaginator
Copy link
Copy Markdown
Member

"Findbugs"?

Am I out of a job?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can change this to:

if (!reqIQ.getType().equals(IQ.Type.Result)) {

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.32%) when pulling f57f1c8 on garethf:issues/247 into bdb00aa on buddycloud:master.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this one then please?

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.31%) when pulling 17d6d35 on garethf:issues/247 into bdb00aa on buddycloud:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.31%) when pulling 17d6d35 on garethf:issues/247 into bdb00aa on buddycloud:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.31%) when pulling a05211f on garethf:issues/247 into bdb00aa on buddycloud:master.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a not comparison, original line:

if (false == actorJid.toBareJID().equals(ns.getUser())) {

Basically we don't want to show a subscription != 'subscribed' to anyone but the owner/admin or the user themselves.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for (NodeMembership ns : cur) {
                if (!actorJid.toBareJID().equals(ns.getUser().toBareJID()) &&
                    !isOwnerModerator() &&
                    !ns.getSubscription().equals(Subscriptions.subscribed)) {
                    continue;
                }
                Element subscription = subscriptions.addElement(XMLConstants.SUBSCRIPTION_ELEM);
                subscription.addAttribute(XMLConstants.NODE_ATTR, ns.getNodeId())
                        .addAttribute(XMLConstants.SUBSCRIPTION_ELEM, ns.getSubscription().toString())
                        .addAttribute(XMLConstants.JID_ATTR, ns.getUser().toBareJID());
                if (null != ns.getInvitedBy()) {
                    subscription.addAttribute(XMLConstants.INVITED_BY_ELEM, ns.getInvitedBy().toBareJID());
                }

            }

...would give us the correct logic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: comparison, in the previous version a JID was getting compared to a string, which is probably not what was intended.
re: logic - I'll come up with another test.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) when pulling 0778ce5 on garethf:issues/247 into bdb00aa on buddycloud:master.

…ver-java into issues/247

Conflicts:
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/AffiliationsGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/NodeConfigureGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/NodeThreadsGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/RepliesGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/SubscriptionsGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/ThreadGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/UserItemsGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/items/NodeItemsGet.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/AffiliationEvent.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/ItemDelete.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeCreate.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/NodeDelete.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/SubscriptionEvent.java
	src/main/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/search/SearchSet.java
	src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/get/SubscriptionsGetTest.java
	src/test/java/org/buddycloud/channelserver/packetprocessor/iq/namespace/pubsub/set/PublishTest.java
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.16%) when pulling 49e129f on garethf:issues/247 into 60f0952 on buddycloud:master.

lloydwatkin added a commit that referenced this pull request Oct 10, 2014
@lloydwatkin lloydwatkin merged commit a251692 into buddycloud:master Oct 10, 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

Development

Successfully merging this pull request may close these issues.

4 participants