Commit Graph

210 Commits

Author SHA1 Message Date
Sean McGivern 6cd7f679d0 Only cache highlight results for latest MR diffs
Previously, we kept them all in the cache. We don't need the highlight results
for older diffs - if someone does view that (which is rare), we can do the
highlighting on the fly.
2018-03-15 11:49:53 +00:00
Sean McGivern 4bc58ca210 Merge branch 'osw-stop-recalculating-merge-base-on-mr-loading' into 'master'
Avoid re-fetching merge-base SHA from Gitaly unnecessarily

See merge request gitlab-org/gitlab-ce!17630
2018-03-12 13:22:26 +00:00
Oswaldo Ferreira ea81c27612 Submit a single batch blob RPC to Gitaly per HTTP request when viewing diffs 2018-03-09 12:36:46 -03:00
Oswaldo Ferreira c6273ec50c Avoid re-fetching merge-base SHA from Gitaly unnecessarily 2018-03-07 22:46:29 -03:00
Sean McGivern cdf3ae04f8 Fix 500 error when diff context line has broken encoding
Rugged sometimes chops a context line in between bytes, resulting in the context
line having an invalid encoding: https://github.com/libgit2/rugged/issues/716

When that happens, we will try to detect the encoding for the diff, and
sometimes we'll get it wrong. If that difference in encoding results in a
difference in string lengths between the diff and the underlying blobs, we'd
fail to highlight any inline diffs, and return a 500 status to the user.

As we're using the underlying blobs, the encoding is 'correct' anyway, so if the
string range is invalid, we can just discard the inline diff highlighting. We
still report to Sentry to ensure that we can investigate further in future.
2018-02-22 12:26:23 +00:00
🙈 jacopo beschi 🙉 729f05f0e3 Adds Rubocop rule for line break around conditionals 2018-01-11 16:34:01 +00:00
Sean McGivern 528b5eeb76 Fix error when viewing diffs without blobs
Old merge requests can have diffs without corresponding blobs. (This also may be
possible for commit diffs in corrupt repositories.)

We can't use the `&.` operator on the blobs, because the blob objects are never
nil, but `BatchLoader` instances that delegate to `Blob`. We can't use
`Object#try`, because `Blob` doesn't inherit from `Object`.

`BatchLoader` provides a `__sync` method that returns the delegated object, but
using `itself` also works because it's forwarded, and will work for
non-`BatchLoader` instances too. So the simplest solution is to just use that
with the `&.` operator.
2018-01-04 14:33:12 +00:00
Douwe Maan 771bf9527f Improve performance of DiffDiscussion#truncated_diff_lines and DiffNote#diff_line by removing expensive diff position calculation and comparison 2017-12-22 18:07:15 +01:00
Sean McGivern 7838317a14 Merge branch 'fix_39233' into 'master'
fix #39233 - 500 in merge request

Closes #39233

See merge request gitlab-org/gitlab-ce!15774
2017-12-07 20:35:39 +00:00
Martin Nowak b7a5125f02 fix #39233 - 500 in merge request
- handle unchanged empty lines in inline diff
2017-12-07 20:19:37 +01:00
micael.bergeron cb6f51ec9b add support for the commit reference filter 2017-12-07 09:01:37 -05:00
Zeger-Jan van de Weg f9565e3039
Batchload blobs for diff generation
After installing a new gem, batch-loader, a construct can be used to
queue data to be fetched in bulk. The gem was also introduced in both
gitlab-org/gitlab-ce!14680 and gitlab-org/gitlab-ce!14846, but those mrs
are not merged yet.

For the generation of diffs, both the old blob and the new blob need to
be loaded. This for every file in the diff, too. Now we collect all
these so we do 1 fetch. Three `.allow_n_plus_1_calls` have been removed,
which I expect to be valid, but this needs to be confirmed by a full CI
run.

Possibly closes:
- https://gitlab.com/gitlab-org/gitlab-ce/issues/37445
- https://gitlab.com/gitlab-org/gitlab-ce/issues/37599
- https://gitlab.com/gitlab-org/gitlab-ce/issues/37431
2017-11-21 13:53:26 +01:00
Jacopo 181cd299f9 Adds Rubocop rule for line break after guard clause
Adds a rubocop rule (with autocorrect) to ensure line break after guard clauses.
2017-11-16 17:58:29 +01:00
AlexWayfer 7ba7fa5048 Fix 500 error for old (somewhat) MRs 2017-10-30 12:30:31 +00:00
Sean McGivern b2553840e8 Merge branch 'conflict-resolution-refactor' into 'master'
Conflict resolution refactor

See merge request gitlab-org/gitlab-ce!14747
2017-10-16 10:36:06 +00:00
Valery Sizov d09895a0de Fix diff parser so it tolerates to diff special markers in the content 2017-10-15 17:00:36 +03:00
Alejandro Rodríguez 9fdde3693b Move line code generation into Gitlab::Git
Having a distinct class just for that was a bit overkill
2017-10-12 22:13:05 -03:00
Alejandro Rodríguez faa9bd402d Create a Gitlab::Git submodule for conlict-related files
Rename classes to (hopefully) clearer names while we're doing that.
2017-10-12 22:03:15 -03:00
Alejandro Rodríguez f72598b659 Move Gitlab::Diff::LineCode to module Gitlab::Git 2017-10-12 21:45:16 -03:00
Felipe Artur b54203f0ad Commenting on image diffs 2017-10-07 04:25:17 +00:00
Douwe Maan 19a30595d9 Remove unnecessary comments 2017-09-25 10:23:43 +02:00
Douwe Maan b9857d8b66 Properly compare diff refs and diff positions when shas are truncated 2017-09-25 10:22:28 +02:00
Ahmad Sherif eb36fa17a6 Migrate Gitlab::Git::Repository#diff to Gitaly
Closes gitaly#524
2017-09-21 11:05:06 +02:00
Andrew Newdigate 64d7ec0a9e Detect n+1 issues involving Gitaly 2017-09-19 10:55:37 +00:00
micael.bergeron 46e4e8f4dc changed InlineDiffMarker to make it html_safe its output
updated the spec
2017-09-12 13:42:40 -04:00
Sean McGivern e8525e107d Show un-highlighted diffs when blobs are the same
For some old merge requests, we don't have enough information to figure out the
old blob and the new blob for the file. This means that we can't highlight the
diff correctly, but we can still display it without highlighting.
2017-08-24 11:11:18 +01:00
Sean McGivern b0f09406f5 Always return a simple diff viewer
We didn't have a fallback case before, because we believed the conditions were
exhaustive. They weren't, so we can always fallback to not previewing.
2017-08-22 14:04:54 +01:00
Robert Speicher 260c8da060 Whitelist or fix additional `Gitlab/PublicSend` cop violations
An upcoming update to rubocop-gitlab-security added additional
violations.
2017-08-14 12:14:11 -04:00
Brian Neel 9770c57fab Re-enable SqlInjection and CommandInjection 2017-08-08 10:50:54 -04:00
Jacob Vosmaer 7b18c42464 Remove unused (?) code 2017-07-31 17:19:25 +02:00
Douwe Maan 5e8aca2152 Don't display comment on unchanged line on both sides in parallel diff 2017-06-19 11:50:46 -05:00
Douwe Maan 7944254563 Implement diff viewers 2017-06-14 10:12:21 -05:00
Sean McGivern e9002222a0 Merge branch 'dm-diff-file-diffable' into 'master'
Move diffable? method from Repository to Diff::File

See merge request !11980
2017-06-08 16:14:56 +00:00
Douwe Maan e6e29f9220 Use Diff::File blob methods from diff highlighter 2017-06-08 09:39:54 -05:00
Douwe Maan ffbbd4112e Move diffable? method from Repository to Diff::File 2017-06-08 09:32:57 -05:00
Douwe Maan 3a5d375e49 Fix Diff::Position#diff_file for positions on straight diffs 2017-06-06 14:48:05 -05:00
Sean McGivern c1ee45953e Merge branch 'fix_diff_line_comments' into 'master'
Fix: A diff comment on a change at last line of a file shows as two comments in discussion

Closes #32353

See merge request !11802
2017-06-01 10:30:46 +00:00
Valery Sizov 563c1ca01c Fix: A diff comment on a change at last line of a file shows as two comments in discussion 2017-05-31 18:38:10 +03:00
Douwe Maan aed0387f97 Consistent diff and blob size limit names 2017-05-29 17:02:02 -05:00
Robert Speicher 8e2fefc6c4 Merge branch 'dm-diff-cleanup' into 'master'
Clean up diff rendering

See merge request !11390
2017-05-25 22:15:57 +00:00
Douwe Maan d88a5c1db1 Address feedback 2017-05-25 15:17:57 -05:00
Douwe Maan 0bf339f0ed Address review 2017-05-24 10:10:10 -05:00
Douwe Maan 6e698b254e Fix specs 2017-05-23 16:27:30 -05:00
Douwe Maan ab91f76e8b Add system note with link to diff comparison when MR discussion becomes outdated 2017-05-23 16:27:30 -05:00
Douwe Maan 7c479d88a9 Pass fallback_diff_refs to Diff::File instead of using view helpers 2017-05-23 15:37:05 -05:00
Douwe Maan db40c668de Rename Diff::File {old,new}_ref to {old,new}_sha 2017-05-23 15:37:04 -05:00
Douwe Maan d9a0188d2f Add question mark to Gitlab::Diff::File predicate methods 2017-05-23 15:37:04 -05:00
Douwe Maan 7a7e9288d4 Stop MR conflict code from blowing up when branches are missing 2017-05-18 15:48:42 -05:00
Douwe Maan e179707844 Extract generic parts of Gitlab::Diff::InlineDiffMarker 2017-05-10 08:26:21 -05:00
Sean McGivern a0979c05fd Show correct size when MR diff overflows
The problem is that we often go via a diff object constructed from the diffs
stored in the DB. Those diffs, by definition, don't overflow, so we don't have
access to the 'correct' `real_size` - that is stored on the MR diff object
iself.
2017-04-25 16:54:10 +01:00
Douwe Maan 63d38a303b Fix commenting on an existing discussion on an unchanged line that is no longer in the diff 2017-04-24 11:17:15 -05:00
Douwe Maan 2c0de7aaaf Cache MR diffs by diff refs 2017-04-08 14:37:46 -05:00
Douwe Maan afa53810de
Fix specs 2017-04-05 17:44:14 +01:00
Douwe Maan bb8cc94668
Don't use original_discussion_id 2017-04-05 17:44:14 +01:00
Douwe Maan f6f6aaf593
Better notification emails for notes and (diff) discussions 2017-04-05 17:44:14 +01:00
mhasbini 985af1a670 take nonewline context into account in diff parser 2017-03-13 22:09:43 +02:00
Douwe Maan 5c7f9d69e3 Fix code for cops 2017-02-23 09:31:57 -06:00
Douwe Maan 8a4d68c53e Enable Style/ConditionalAssignment 2017-02-23 09:31:57 -06:00
Douwe Maan ccbebbdfeb Enable Performance/RedundantMatch 2017-02-23 09:31:57 -06:00
Douwe Maan b7d8df503c Enable Style/MutableConstant 2017-02-23 09:31:56 -06:00
Robert Speicher 17c8d15b14 Enable `Lint/EmptyWhen` cop and correct offense 2017-02-13 11:00:20 -05:00
Sean McGivern 1e5e56c698 Fix MR with files hidden by .gitattributes
Don't try to highlight and cache files hidden by .gitattributes entries.
2016-12-26 16:25:55 +00:00
Valery Sizov f5d7a61760 Fixes ActionView::Template::Error: undefined method `text?` for nil:NilClass 2016-12-02 14:03:30 +02:00
Valery Sizov 847ada36c4 Fix: Timeout creating and viewing merge request for binary file 2016-11-25 15:25:01 +02:00
Stan Hu 0f61bb2dc5
Fix Error 500 when creating a merge request that contains an image that was deleted and added
Steps to reproduce:

1. Start with a repo with an image
2. Add a commit to delete the image
3. Add another commit to replace the image with another image

In a diff comparison, we really just compare about what the image was before the diff, not
the direct parent of the last commit. This MR fixes that.

Closes #3893, gitlab-org/gitlab-ee#678

Signed-off-by: Rémy Coutable <remy@rymai.me>
2016-11-14 15:57:01 +01:00
Adam Niedzielski 3fa265d195 Fix expanding a collapsed diff when converting a symlink to a regular file
In this case comparing old_path and new_path is not enough because there
are two entires that match.
2016-11-10 18:24:12 +01:00
Adam Niedzielski 317e48193f Fix the diff in the merge request view when converting a symlink to a regular file.
In this specific case using file_path as a cache key is not enough,
because there are two entries with the same path.
Closes #21610.
2016-10-17 18:09:34 +02:00
Dmitriy Zaporozhets 9329436deb
Merge branch 'master' into dz-merge-request-version
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
2016-08-22 11:34:41 +03:00
Sean McGivern 4a4c1765be Fix line commenting for the initial commit
The initial commit doesn't have a parent, so explicitly pass the blank
SHA and handle that when calculating the position.
2016-08-19 16:35:44 +01:00
Dmitriy Zaporozhets 6db65143db
Merge branch 'master' into dz-merge-request-version
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
2016-08-19 13:01:58 +03:00
Dmitriy Zaporozhets 643a368fa4
Make merge request diff works with new FileCollection logic
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
2016-08-15 17:57:19 +03:00
Sean McGivern 261d47bce9 Fix specs
- Add match line header to expected result for `File#sections`.
- Lowercase CSS colours.
- Remove unused `diff_refs` keyword argument.
- Rename `parent` -> `parent_file`, to be more explicit.
- Skip an iteration when highlighting.
2016-08-12 23:24:46 +03:00
Sean McGivern 7af277f683 Auto-highlight conflict when rich_text is called 2016-08-12 23:24:43 +03:00
Sean McGivern df2ed097b7 Add backend for merge conflicts reading 2016-08-12 23:24:43 +03:00
Paco Guzman c86c1905b5 switch from diff_file_collection to diffs
So we have raw_diffs too
2016-08-03 07:00:20 +02:00
Paco Guzman 1d0c7b7492 Introduce Compare model in the codebase.
This object will manage Gitlab::Git::Compare instances
2016-08-03 07:00:20 +02:00
Paco Guzman 8f359ea917 Move to Gitlab::Diff::FileCollection
Instead calling diff_collection.count use diff_collection.size which is cache on the diff_collection
2016-08-03 07:00:20 +02:00
Paco Guzman cd7c2cb6dd Cache highlighted diff lines for merge requests
Introducing the concept of SafeDiffs which relates 
diffs with UI highlighting.
2016-08-03 07:00:19 +02:00
Robert Speicher 34c083a184 Merge branch 'rubocop/enable-access-modifiers-cops' into 'master'
Enable Rubocop cops that check access modifiers

## What does this MR do?

This MR enables Rubocop cops that detect methods that should be restricted but are the part of public API because of access modifiers used improperly.

This also fixes existing offenses.

## Why was this MR needed?

Some method in our codebase are public instead of being private because it is sometimes difficult to get it right without static analysis.

## What are the relevant issue numbers?

See #17478  
Closes #17372 

See merge request !5014
2016-07-29 15:58:09 +00:00
Douwe Maan 79214be727 Add Discussion model to represent MR/diff discussion 2016-07-20 16:18:18 -06:00
Sean McGivern 4f0780cc04 Ensure to_json methods take optional argument 2016-07-20 11:14:06 +01:00
Grzegorz Bizon 20a5033d79 Fix method visibility in inline diff class 2016-07-19 15:12:14 +02:00
Paco Guzman a404ab380d Collapsed diffs lines/size don't accumulate to overflow diffs. 2016-07-18 14:43:28 -06:00
Robert Speicher 9b0ef1559f Merge branch 'better-parallel-diff' into 'master'
Actually render old and new sections of parallel diff next to each other

See merge request !5173
2016-07-13 18:10:01 +00:00
Douwe Maan 5fea640e90 Merge branch 'multi-line-inline-diff' into 'master'
Render inline diffs for multiple changed lines following eachother

Before:

![Screen_Shot_2016-07-11_at_00.08.27](/uploads/b14664211e0f5cef6e77a78eadfcbcdf/Screen_Shot_2016-07-11_at_00.08.27.png)

After:

![Screen_Shot_2016-07-11_at_00.07.34](/uploads/567be631869a4867a2edf6ff7eda6369/Screen_Shot_2016-07-11_at_00.07.34.png)

See merge request !5174
2016-07-13 05:29:49 +00:00
Douwe Maan 545a85dc64 Actually render old and new sections of parallel diff next to each other 2016-07-13 00:04:07 -05:00
Douwe Maan 489e193704 Rename constant to be more descriptive 2016-07-13 00:02:20 -05:00
Douwe Maan e981d6cd0d Extended regexes ignore whitespace, so use \s 2016-07-12 11:25:39 -05:00
Douwe Maan f5cc3f63a8 Render inline diffs for multiple changed lines following eachother 2016-07-11 18:58:15 -05:00
Sean McGivern b6b26692ea Collapse large diffs by default
When rendering a list of diff files, skip those where the diff is over
10 KB and provide an endpoint to render individually instead.
2016-07-08 10:56:55 +01:00
Douwe Maan ac26b23712 Remove duplication, unused methods, and some other style things 2016-07-06 19:29:41 -04:00
Douwe Maan 228d2a4cb1 Add some more code comments. 2016-07-06 19:28:13 -04:00
Douwe Maan 29d574868a Display new diff notes and allow creation through the web interface 2016-07-06 18:51:00 -04:00
Douwe Maan db65954d78 Add Gitlab::Git::PositionTracer 2016-07-06 18:50:59 -04:00
Douwe Maan e9e06ca627 Add Gitlab::Diff::LineMapper 2016-07-06 18:50:59 -04:00
Douwe Maan 9abcc0a98f Add Gitlab::Git::Position 2016-07-06 18:50:59 -04:00
Douwe Maan a27462a5c6 Extract parts of LegacyDiffNote into DiffOnNote concern and move part of responsibility to other classes 2016-07-06 18:50:59 -04:00
Douwe Maan 375193455a Style diff and blob file headers the same way 2016-07-06 18:50:59 -04:00
Douwe Maan 9fc0e11e0d Add DiffFile#blob and #old_blob 2016-07-06 18:50:58 -04:00
Douwe Maan a9fa45f09e Represent DiffRefs as proper class instead of tuple array 2016-07-06 18:50:58 -04:00