-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug/cite 221 #276
base: develop
Are you sure you want to change the base?
Bug/cite 221 #276
Changes from 13 commits
1ae05d4
4b4e181
a8fbf93
c7a356b
7121561
fec500a
2ac9b55
73c3aaf
0b2624a
a87f135
12f6aa3
42d1c8e
d7a4219
a8a977c
b039e95
928ea95
1bea700
87abbe4
ac2599b
426cfbc
d707d59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,9 @@ | |
import javax.annotation.PostConstruct; | ||
import javax.transaction.Transactional; | ||
|
||
import org.bson.types.ObjectId; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.beans.factory.annotation.Value; | ||
import org.springframework.context.annotation.PropertySource; | ||
|
@@ -181,15 +184,15 @@ public void updateCitation(IUser user, String groupId, ICitation citation) | |
throws ZoteroConnectionException, CitationIsOutdatedException, ZoteroHttpStatusException, ZoteroItemCreationFailedException { | ||
long citationVersion = zoteroManager.getGroupItemVersion(user, groupId, citation.getKey()); | ||
Optional<ICitation> storedCitationOptional = citationStore.findById(citation.getKey()); | ||
ICitation updatedCitation = zoteroManager.updateCitation(user, groupId, citation); | ||
if (storedCitationOptional.isPresent()) { | ||
ICitation storedCitation = storedCitationOptional.get(); | ||
if (storedCitation.getVersion() != citationVersion) { | ||
throw new CitationIsOutdatedException(); | ||
} | ||
citationStore.delete(storedCitation); | ||
updatedCitation.setId(storedCitation.getId()); | ||
} | ||
|
||
ICitation updatedCitation = zoteroManager.updateCitation(user, groupId, citation); | ||
citationStore.save(updatedCitation); | ||
} | ||
|
||
|
@@ -251,7 +254,7 @@ public ICitation updateCitationFromZotero(IUser user, String groupId, String ite | |
|
||
Optional<ICitation> oldCitation = citationStore.findById(itemKey); | ||
if (oldCitation.isPresent()) { | ||
citationStore.delete(oldCitation.get()); | ||
citation.setId(oldCitation.get().getId()); | ||
} | ||
citationStore.save(citation); | ||
return citation; | ||
|
@@ -326,8 +329,9 @@ public List<ICitationGroup> getGroups(IUser user) { | |
if (groupOptional.isPresent()) { | ||
ICitationGroup group = groupOptional.get(); | ||
if (group.getMetadataVersion() != groupVersions.get(id)) { | ||
groupRepository.delete((CitationGroup) group); | ||
ObjectId groupID = group.getId(); | ||
group = zoteroManager.getGroup(user, id + "", true); | ||
group.setId(groupID); | ||
group.setUpdatedOn(OffsetDateTime.now().toString()); | ||
} | ||
addUserToGroup(group, user); | ||
|
@@ -379,14 +383,17 @@ public CitationResults getGroupItems(IUser user, String groupId, String collecti | |
|
||
ICitationGroup group = null; | ||
Optional<ICitationGroup> groupOptional = groupRepository.findFirstByGroupId(new Long(groupId)); | ||
if (!groupOptional.isPresent() || !groupOptional.get().getUsers().contains(user.getUsername())) { | ||
if (!groupOptional.isPresent()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, I'm fairly certain this create a security issue here. Basically, doesn't this say "if the group does not yet locally exist, get it from zotero" (that's fine zotero will complain if the user does not have access), however, "if the group does exist, let's just add the user to the group" (this seems wrong, where is the check if the user has access?). |
||
group = zoteroManager.getGroup(user, groupId, false); | ||
if (group != null) { | ||
group.getUsers().add(user.getUsername()); | ||
groupRepository.save((CitationGroup) group); | ||
} | ||
} else { | ||
group = groupOptional.get(); | ||
if (!group.getUsers().contains(user.getUsername())){ | ||
group.getUsers().add(user.getUsername()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should not be needed, should it? this else should only be called if the group is present and the user has access to it? |
||
} | ||
|
||
if (group == null) { | ||
|
@@ -399,13 +406,14 @@ public CitationResults getGroupItems(IUser user, String groupId, String collecti | |
long previousVersion = group.getContentVersion(); | ||
// first update the group info | ||
// if we are using a previously stored group, delete it | ||
ICitationGroup zeteroGroup = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. spelling mistake |
||
if (group.getId() != null) { | ||
groupRepository.delete((CitationGroup) group); | ||
group = zoteroManager.getGroup(user, groupId + "", true); | ||
zeteroGroup = zoteroManager.getGroup(user, groupId + "", true); | ||
zeteroGroup.setId(group.getId()); | ||
} | ||
group.setUpdatedOn(OffsetDateTime.now().toString()); | ||
addUserToGroup(group, user); | ||
group = groupRepository.save((CitationGroup) group); | ||
zeteroGroup.setUpdatedOn(OffsetDateTime.now().toString()); | ||
addUserToGroup(zeteroGroup, user); | ||
group = groupRepository.save((CitationGroup) zeteroGroup); | ||
|
||
// then update content | ||
results.setNotModified(false); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be a proper version