Skip to content

Commit 520552d

Browse files
SebastianKrupinskiSebastianKrupinski
authored
Merge pull request #1030 from nextcloud/fix/clear-updatedproperties-after-proppatch
fix: Clear internal list of updated properties on successful update
2 parents d05047b + e93bfd1 commit 520552d

4 files changed

Lines changed: 171 additions & 4 deletions

File tree

‎src/models/davCollection.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,11 @@ export class DavCollection extends DAVEventListener {
185185

186186
dPropSet.push(...propSet)
187187

188-
const body = XMLUtility.serialize(skeleton)
189-
await this._request.propPatch(this._url, {}, body)
188+
if (propSet.length >= 1) {
189+
const body = XMLUtility.serialize(skeleton)
190+
await this._request.propPatch(this._url, {}, body)
191+
this._updatedProperties = []
192+
}
190193
}
191194

192195
/**

‎src/models/principal.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,11 @@ export class Principal extends DavObject {
245245

246246
dPropSet.push(...propSet)
247247

248-
const body = XMLUtility.serialize(skeleton)
249-
await this._request.propPatch(this._url, {}, body)
248+
if (propSet.length >= 1) {
249+
const body = XMLUtility.serialize(skeleton)
250+
await this._request.propPatch(this._url, {}, body)
251+
this._updatedProperties = []
252+
}
250253
}
251254

252255
}

‎test/unit/models/davCollectionTest.js‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import DAVEventListener from "../../../src/models/davEventListener.js";
1414
import { DavObject } from "../../../src/models/davObject.js";
1515
import * as XMLUtility from '../../../src/utility/xmlUtility.js';
1616
import RequestMock from "../../mocks/request.mock.js";
17+
import NetworkRequestServerError from '../../../src/errors/networkRequestServerError.js'
1718

1819
describe('Dav collection model', () => {
1920

@@ -693,6 +694,95 @@ describe('Dav collection model', () => {
693694
});
694695
});
695696

697+
it('should clear the internal list of changed properties after a successful update', async () => {
698+
const parent = {
699+
'findAll': vi.fn(),
700+
'findAllByFilter': vi.fn(),
701+
'find': vi.fn(),
702+
'createCollection': vi.fn(),
703+
'createObject': vi.fn(),
704+
'update': vi.fn(),
705+
'delete': vi.fn(),
706+
'isReadable': vi.fn(),
707+
'isWriteable': vi.fn()
708+
};
709+
const request = new RequestMock();
710+
const url = '/foo/bar/folder';
711+
const props = {
712+
'{DAV:}displayname': 'Foo Bar Bla Blub',
713+
'{DAV:}owner': 'https://foo/bar/',
714+
'{DAV:}resourcetype': ['{DAV:}collection'],
715+
'{DAV:}sync-token': 'https://foo/bar/token/3',
716+
'{custom}property': 'custom property value 123',
717+
'{DAV:}current-user-privilege-set': ['{DAV:}write',
718+
'{DAV:}write-properties', '{DAV:}write-content',
719+
'{DAV:}unlock', '{DAV:}bind', '{DAV:}unbind',
720+
'{DAV:}write-acl', '{DAV:}read', '{DAV:}read-acl',
721+
'{DAV:}read-current-user-privilege-set'],
722+
};
723+
724+
request.propPatch.mockImplementation(() => {
725+
return Promise.resolve({
726+
status: 207,
727+
body: {
728+
'{DAV:}displayname': 'test',
729+
'{http://apple.com/ns/ical/}calendar-color': ''
730+
},
731+
headers: {}
732+
});
733+
});
734+
735+
const collection = new DavCollection(parent, request, url, props);
736+
collection.displayname = 'test';
737+
738+
await collection.update();
739+
await collection.update();
740+
await collection.update();
741+
742+
expect(request.propPatch).toHaveBeenCalledTimes(1);
743+
});
744+
745+
it('should not clear the internal list of changed properties after an unsuccessful update', async () => {
746+
const parent = {
747+
'findAll': vi.fn(),
748+
'findAllByFilter': vi.fn(),
749+
'find': vi.fn(),
750+
'createCollection': vi.fn(),
751+
'createObject': vi.fn(),
752+
'update': vi.fn(),
753+
'delete': vi.fn(),
754+
'isReadable': vi.fn(),
755+
'isWriteable': vi.fn()
756+
};
757+
const request = new RequestMock();
758+
const url = '/foo/bar/folder';
759+
const props = {
760+
'{DAV:}displayname': 'Foo Bar Bla Blub',
761+
'{DAV:}owner': 'https://foo/bar/',
762+
'{DAV:}resourcetype': ['{DAV:}collection'],
763+
'{DAV:}sync-token': 'https://foo/bar/token/3',
764+
'{custom}property': 'custom property value 123',
765+
'{DAV:}current-user-privilege-set': ['{DAV:}write',
766+
'{DAV:}write-properties', '{DAV:}write-content',
767+
'{DAV:}unlock', '{DAV:}bind', '{DAV:}unbind',
768+
'{DAV:}write-acl', '{DAV:}read', '{DAV:}read-acl',
769+
'{DAV:}read-current-user-privilege-set'],
770+
};
771+
772+
request.propPatch.mockImplementation(() => {
773+
return Promise.reject(new NetworkRequestServerError({ status: 500 }));
774+
});
775+
776+
const collection = new DavCollection(parent, request, url, props);
777+
collection.displayname = 'test';
778+
779+
await expect(collection.update()).rejects.toThrow();
780+
await expect(collection.update()).rejects.toThrow();
781+
await expect(collection.update()).rejects.toThrow();
782+
783+
expect(request.propPatch).toHaveBeenCalledTimes(3);
784+
});
785+
696786
it('should delete a collection', () => {
697787
const parent = {
698788
'findAll': vi.fn(),

‎test/unit/models/principalTest.js‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { Principal } from '../../../src/models/principal.js';
1414
import * as XMLUtility from '../../../src/utility/xmlUtility.js';
1515
import RequestMock from "../../mocks/request.mock.js";
1616
import { DavCollection as DavCollectionMock } from "../../mocks/davCollection.mock.js";
17+
import NetworkRequestServerError from '../../../src/errors/networkRequestServerError.js'
1718

1819
describe('Principal model', () => {
1920
beforeEach(() => {
@@ -499,4 +500,74 @@ describe('Principal model', () => {
499500
assert.fail('Principal update was not supposed to assert.fail');
500501
});
501502
});
503+
504+
it('should clear the internal list of changed properties after a successful update', async () => {
505+
const parent = new DavCollectionMock();
506+
const request = new RequestMock();
507+
const url = '/nextcloud/remote.php/dav/foo/bar/baz/';
508+
const props = {
509+
'{DAV:}displayname': 'Umberto',
510+
'{urn:ietf:params:xml:ns:caldav}calendar-user-type': 'INDIVIDUAL',
511+
'{urn:ietf:params:xml:ns:caldav}calendar-user-address-set': [],
512+
'{DAV:}principal-URL': '/nextcloud/remote.php/dav/principals/users/user2/',
513+
'{http://sabredav.org/ns}email-address': 'foo-bar@example.com',
514+
'{urn:ietf:params:xml:ns:caldav}calendar-home-set': ['/nextcloud/remote.php/dav/calendars/admin/'],
515+
'{urn:ietf:params:xml:ns:caldav}schedule-inbox-URL': '/nextcloud/remote.php/dav/calendars/admin/inbox/',
516+
'{urn:ietf:params:xml:ns:caldav}schedule-outbox-URL': '/nextcloud/remote.php/dav/calendars/admin/outbox/',
517+
'{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL': '/nextcloud/remote.php/dav/calendars/admin/personal/',
518+
'{urn:ietf:params:xml:ns:carddav}addressbook-home-set': ['/nextcloud/remote.php/dav/addressbooks/users/admin/'],
519+
};
520+
521+
const principal = new Principal(parent, request, url, props);
522+
523+
request.propPatch.mockImplementation(() => {
524+
return Promise.resolve({
525+
status: 207,
526+
body: {
527+
'{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL': '/nextcloud/remote.php/dav/calendars/admin/changed/',
528+
},
529+
headers: {}
530+
});
531+
});
532+
533+
principal.scheduleDefaultCalendarUrl = '/nextcloud/remote.php/dav/calendars/admin/changed/';
534+
535+
await principal.update();
536+
await principal.update();
537+
await principal.update();
538+
539+
expect(request.propPatch).toHaveBeenCalledTimes(1);
540+
});
541+
542+
it('should not clear the internal list of changed properties after an unsuccessful update', async () => {
543+
const parent = new DavCollectionMock();
544+
const request = new RequestMock();
545+
const url = '/nextcloud/remote.php/dav/foo/bar/baz/';
546+
const props = {
547+
'{DAV:}displayname': 'Umberto',
548+
'{urn:ietf:params:xml:ns:caldav}calendar-user-type': 'INDIVIDUAL',
549+
'{urn:ietf:params:xml:ns:caldav}calendar-user-address-set': [],
550+
'{DAV:}principal-URL': '/nextcloud/remote.php/dav/principals/users/user2/',
551+
'{http://sabredav.org/ns}email-address': 'foo-bar@example.com',
552+
'{urn:ietf:params:xml:ns:caldav}calendar-home-set': ['/nextcloud/remote.php/dav/calendars/admin/'],
553+
'{urn:ietf:params:xml:ns:caldav}schedule-inbox-URL': '/nextcloud/remote.php/dav/calendars/admin/inbox/',
554+
'{urn:ietf:params:xml:ns:caldav}schedule-outbox-URL': '/nextcloud/remote.php/dav/calendars/admin/outbox/',
555+
'{urn:ietf:params:xml:ns:caldav}schedule-default-calendar-URL': '/nextcloud/remote.php/dav/calendars/admin/personal/',
556+
'{urn:ietf:params:xml:ns:carddav}addressbook-home-set': ['/nextcloud/remote.php/dav/addressbooks/users/admin/'],
557+
};
558+
559+
const principal = new Principal(parent, request, url, props);
560+
561+
request.propPatch.mockImplementation(() => {
562+
return Promise.reject(new NetworkRequestServerError({ status: 500 }));
563+
});
564+
565+
principal.scheduleDefaultCalendarUrl = '/nextcloud/remote.php/dav/calendars/admin/changed/';
566+
567+
await expect(principal.update()).rejects.toThrow();
568+
await expect(principal.update()).rejects.toThrow();
569+
await expect(principal.update()).rejects.toThrow();
570+
571+
expect(request.propPatch).toHaveBeenCalledTimes(3);
572+
});
502573
});

0 commit comments

Comments
 (0)