Skip to content
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.

Sign up for GitHub

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

Distinguish section mutations from item mutations #16

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Distinguish section mutations from item mutations #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe1cd9f
Distinguish between section mutations and item mutations
Dec 31, 2018
785aab0
Update `sections` precondition
ivopintodasilva Feb 20, 2021
b2bea45
Merge branch 'master' into feature/decouple-section-mutations
ivopintodasilva Feb 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions FlexibleDiff/SectionedChangeset.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public struct SectionedChangeset {
/// The changes of sections.
///
/// - precondition: Offsets in `sections.mutations` and `sections.moves` must have a
/// corresponding entry in `mutatedSections` if they represent a
/// mutation.
/// corresponding entry in `mutatedSections` if their metadata has
/// been mutated.
public var sections = Changeset()

/// The changes of items in the mutated sections.
Expand Down Expand Up @@ -126,11 +126,10 @@ public struct SectionedChangeset {
identifier: itemIdentifier,
areEqual: areItemsEqual)

let isMutated = !changeset.hasNoChanges
|| metadata.mutations.contains(sourceOffset)
let isMutatedMetadata = metadata.mutations.contains(sourceOffset)
|| mutatedMoveDests.contains(destinationOffset)

if isMutated {
if changeset.hasNoChanges == false {
let section = SectionedChangeset.MutatedSection(source: sourceOffset,
destination: destinationOffset,
changeset: changeset)
Expand All @@ -140,8 +139,8 @@ public struct SectionedChangeset {
if isMove {
moves.append(Changeset.Move(source: sourceOffset,
destination: destinationOffset,
isMutated: isMutated))
} else if isMutated {
isMutated: isMutatedMetadata))
} else if isMutatedMetadata {
mutations.insert(sourceOffset)
}
}
Expand Down
135 changes: 116 additions & 19 deletions FlexibleDiffTests/SectionedChangesetSpec.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class SectionedChangesetSpec: QuickSpec {
}

describe("mutation") {
it("should reflect a mutated section") {
it("should only reflect a mutation in the section's items") {
diffTest(
previous: [
Section(identifier: "A", items: [])
Expand All @@ -140,6 +140,44 @@ class SectionedChangesetSpec: QuickSpec {
Section.Item(identifier: "A0", value: .max)
])
],
expected: SectionedChangeset(
sections: Changeset(),
mutatedSections: [
SectionedChangeset.MutatedSection(
source: 0,
destination: 0,
changeset: Changeset(inserts: [0])
)
]
)
)
}

it("should only reflect a mutation in the section") {
diffTest(
previous: [
Section(identifier: "A", metadata: "old", items: [])
],
current: [
Section(identifier: "A", metadata: "new", items: [])
],
expected: SectionedChangeset(
sections: Changeset(mutations: [0]),
mutatedSections: []
)
)
}

it("should reflect a mutation in the section and its items") {
diffTest(
previous: [
Section(identifier: "A", metadata: "old", items: [])
],
current: [
Section(identifier: "A", metadata: "new", items: [
Section.Item(identifier: "A0", value: .max)
])
],
expected: SectionedChangeset(
sections: Changeset(mutations: [0]),
mutatedSections: [
Expand Down Expand Up @@ -177,7 +215,30 @@ class SectionedChangesetSpec: QuickSpec {
)
}

it("should reflect a section moved with mutations made") {
it("should reflect a section moved with one section mutation") {
diffTest(
previous: [
Section(identifier: "A", metadata: "old", items: []),
Section(identifier: "B", metadata: "old", items: []),
Section(identifier: "C", metadata: "old", items: [])
],
current: [
Section(identifier: "C", metadata: "old", items: []),
Section(identifier: "B", metadata: "old", items: []),
Section(identifier: "A", metadata: "new", items: [])
],
expected: SectionedChangeset(
sections: Changeset(moves: [
Changeset.Move(source: 0, destination: 2, isMutated: true),
Changeset.Move(source: 2, destination: 0, isMutated: false)
]),
mutatedSections: []
)
)
}


it("should reflect a section moved with item mutations made") {
diffTest(
previous: [
Section(identifier: "A", items: [
Expand All @@ -200,8 +261,8 @@ class SectionedChangesetSpec: QuickSpec {
],
expected: SectionedChangeset(
sections: Changeset(moves: [
Changeset.Move(source: 0, destination: 2, isMutated: true),
Changeset.Move(source: 2, destination: 0, isMutated: true)
Changeset.Move(source: 0, destination: 2, isMutated: false),
Changeset.Move(source: 2, destination: 0, isMutated: false)
]),
mutatedSections: [
SectionedChangeset.MutatedSection(
Expand All @@ -219,7 +280,49 @@ class SectionedChangesetSpec: QuickSpec {
)
}

it("should reflect a section moved with mutations made to replace a removal") {
it("should reflect a section moved with section and item mutations") {
diffTest(
previous: [
Section(identifier: "A", metadata: "old", items: [
Section.Item(identifier: "A0", value: .max)
]),
Section(identifier: "B", metadata: "old", items: []),
Section(identifier: "C", metadata: "old", items: [
Section.Item(identifier: "C0", value: .min)
])
],
current: [
Section(identifier: "C", metadata: "old", items: [
Section.Item(identifier: "C1", value: .max),
Section.Item(identifier: "C0", value: .min)
]),
Section(identifier: "B", metadata: "old", items: []),
Section(identifier: "A", metadata: "new", items: [
Section.Item(identifier: "A1", value: .min)
])
],
expected: SectionedChangeset(
sections: Changeset(moves: [
Changeset.Move(source: 0, destination: 2, isMutated: true),
Changeset.Move(source: 2, destination: 0, isMutated: false)
]),
mutatedSections: [
SectionedChangeset.MutatedSection(
source: 0,
destination: 2,
changeset: Changeset(inserts: [0], removals: [0])
),
SectionedChangeset.MutatedSection(
source: 2,
destination: 0,
changeset: Changeset(inserts: [0])
)
]
)
)
}

it("should reflect a section moved with item mutations made to replace a removal") {
diffTest(
previous: [
Section(identifier: "A", items: [
Expand All @@ -241,7 +344,7 @@ class SectionedChangesetSpec: QuickSpec {
sections: Changeset(
removals: [0],
moves: [
Changeset.Move(source: 2, destination: 0, isMutated: true)
Changeset.Move(source: 2, destination: 0, isMutated: false)
]
),
mutatedSections: [
Expand Down Expand Up @@ -308,8 +411,10 @@ private func reproducibilityTest(

let allRemovals = changeset.sections.removals
.union(IndexSet(changeset.sections.moves.map { $0.source }))
let allInsertions: [(Int?, Int)] = [changeset.sections.inserts.map { (nil, $0) },
changeset.sections.moves.map { ($0.source as Int?, $0.destination) }]
let allInsertions: [(Int?, Int)] = [
changeset.sections.inserts.map { (nil, $0) },
changeset.sections.moves.map { ($0.isMutated ? nil : $0.source, $0.destination) }
]
.flatMap { $0 }
.sorted { lhs, rhs in lhs.1 < rhs.1 }

Expand All @@ -325,19 +430,11 @@ private func reproducibilityTest(
}
}

let mutatedMoves = Set(changeset.sections.moves.lazy
.filter { $0.isMutated }
.compactMap { Tuple2($0.source, $0.destination) })
let mutatedSections = Set(changeset.sections.mutations.map { Tuple2($0, $0) })
.union(mutatedMoves)
let records = Set(changeset.mutatedSections.map { Tuple2($0.source, $0.destination) })

// [BUG] It seems occasionally `Set.==` is not being picked up.
// Nimble 7.3.1.
expect(records).to(equal(mutatedSections))
for index in changeset.sections.mutations {
values[index].metadata = current[index].metadata
}

for record in changeset.mutatedSections {
values[record.destination].metadata = current[record.destination].metadata
values[record.destination].items = reproduce(applying: record.changeset,
to: values[record.destination].items,
expecting: current[record.destination].items,
Expand Down